-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[AMDGPU] Correctly determine the toolchain linker #89803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary: The AMDGPU toolchain simply took the short name to get the link job instead of using the common utilities that respect options like `-fuse-ld`. Any linker that isn't `ld.lld` will fail, however we should be able to override it.
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-backend-amdgpu Author: Joseph Huber (jhuber6) ChangesSummary: Full diff: https://github.com/llvm/llvm-project/pull/89803.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 4e6362a0f40632..07965b487ea79b 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -617,8 +617,7 @@ void amdgpu::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfoList &Inputs,
const ArgList &Args,
const char *LinkingOutput) const {
-
- std::string Linker = getToolChain().GetProgramPath(getShortName());
+ std::string Linker = getToolChain().GetLinkerPath();
ArgStringList CmdArgs;
CmdArgs.push_back("--no-undefined");
CmdArgs.push_back("-shared");
diff --git a/clang/test/Driver/amdgpu-toolchain.c b/clang/test/Driver/amdgpu-toolchain.c
index 4300e7e9f66705..d21ce857f3c57a 100644
--- a/clang/test/Driver/amdgpu-toolchain.c
+++ b/clang/test/Driver/amdgpu-toolchain.c
@@ -24,3 +24,9 @@
// RUN: -L. -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=MCPU %s
// LTO: clang{{.*}} "-flto=full"{{.*}}"-fconvergent-functions"
// MCPU: ld.lld{{.*}}"-L."{{.*}}"-plugin-opt=mcpu=gfx906"
+
+// We do not suppor the BFD linker, but we should be able to override the
+// default even if it will error during linking.
+// RUN: %clang -### --target=amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
+// RUN: -fuse-ld=bfd %s 2>&1 | FileCheck -check-prefixes=LD %s
+// LD: ld.bfd"
|
Note that this doesn't affect OpenMP or HIP. The former uses the |
@@ -24,3 +24,9 @@ | |||
// RUN: -L. -fconvergent-functions %s 2>&1 | FileCheck -check-prefix=MCPU %s | |||
// LTO: clang{{.*}} "-flto=full"{{.*}}"-fconvergent-functions" | |||
// MCPU: ld.lld{{.*}}"-L."{{.*}}"-plugin-opt=mcpu=gfx906" | |||
|
|||
// We do not suppor the BFD linker, but we should be able to override the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/suppor/support
Thanks for fixing this after our discord chats! |
Summary:
The AMDGPU toolchain simply took the short name to get the link job
instead of using the common utilities that respect options like
-fuse-ld
. Any linker that isn'tld.lld
will fail, however we shouldbe able to override it.