[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[Joseph Huber via cfe-commits]

https://github.com/jhuber6 closed 
https://github.com/llvm/llvm-project/pull/89803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[Madhur Amilkanthwar via cfe-commits]

madhur13490 wrote:

Thanks for fixing this after our discord chats!

https://github.com/llvm/llvm-project/pull/89803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[Madhur Amilkanthwar via cfe-commits]

@@ -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

madhur13490 wrote:

s/suppor/support

https://github.com/llvm/llvm-project/pull/89803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[Matt Arsenault via cfe-commits]

https://github.com/arsenm approved this pull request.


https://github.com/llvm/llvm-project/pull/89803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[Joseph Huber via cfe-commits]

jhuber6 wrote:

Note that this doesn't affect OpenMP or HIP. The former uses the 
`clang-linker-wrapper` while the latter calls `lld` directly.

https://github.com/llvm/llvm-project/pull/89803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[via cfe-commits]

llvmbot wrote:



@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-amdgpu

Author: Joseph Huber (jhuber6)


Changes

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.


---
Full diff: https://github.com/llvm/llvm-project/pull/89803.diff


2 Files Affected:

- (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+1-2) 
- (modified) clang/test/Driver/amdgpu-toolchain.c (+6) 


``diff
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 , const 
JobAction ,
   const InputInfoList ,
   const ArgList ,
   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"

``




https://github.com/llvm/llvm-project/pull/89803
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [AMDGPU] Correctly determine the toolchain linker (PR #89803)

[Joseph Huber via cfe-commits]

https://github.com/jhuber6 created 
https://github.com/llvm/llvm-project/pull/89803

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.


>From 9389f7831f89e201b09d13bda4e3852968718e32 Mon Sep 17 00:00:00 2001
From: Joseph Huber 
Date: Tue, 23 Apr 2024 12:46:09 -0500
Subject: [PATCH] [AMDGPU] Correctly determine the toolchain linker

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.
---
 clang/lib/Driver/ToolChains/AMDGPU.cpp | 3 +--
 clang/test/Driver/amdgpu-toolchain.c   | 6 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

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 , const 
JobAction ,
   const InputInfoList ,
   const ArgList ,
   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"

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits