Skip to content
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

[HIP] do not link runtime for -r #85675

Merged
merged 1 commit into from
Mar 20, 2024
Merged

[HIP] do not link runtime for -r #85675

merged 1 commit into from
Mar 20, 2024

Conversation

yxsamliu
Copy link
Collaborator

@yxsamliu yxsamliu commented Mar 18, 2024

since it will cause duplicate symbols when the partially linked object is linked again.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Mar 18, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 18, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

since it will cause duplicate symbols when the partially linked object is linked again.

Change-Id: I2aea39ad0d57d3dc80b6aff395d9506ab9ebbf4d


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

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+2-1)
  • (modified) clang/test/Driver/hip-partial-link.hip (+1-1)
  • (modified) clang/test/Driver/hip-runtime-libs-linux.hip (+5)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 83015b0cb81a6e..4cecdd3168a28e 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2830,7 +2830,8 @@ void tools::addHIPRuntimeLibArgs(const ToolChain &TC, Compilation &C,
                                  llvm::opt::ArgStringList &CmdArgs) {
   if ((C.getActiveOffloadKinds() & Action::OFK_HIP) &&
       !Args.hasArg(options::OPT_nostdlib) &&
-      !Args.hasArg(options::OPT_no_hip_rt)) {
+      !Args.hasArg(options::OPT_no_hip_rt) &&
+      !Args.hasArg(options::OPT_r)) {
     TC.AddHIPRuntimeLibArgs(Args, CmdArgs);
   } else {
     // Claim "no HIP libraries" arguments if any
diff --git a/clang/test/Driver/hip-partial-link.hip b/clang/test/Driver/hip-partial-link.hip
index faa185972abc33..c8451ec81ed37e 100644
--- a/clang/test/Driver/hip-partial-link.hip
+++ b/clang/test/Driver/hip-partial-link.hip
@@ -47,7 +47,7 @@
 // OBJ:  D __hip_gpubin_handle_[[ID2]]
 
 // RUN: %clang -v --target=x86_64-unknown-linux-gnu --no-offload-new-driver \
-// RUN:   --hip-link -no-hip-rt -fgpu-rdc --offload-arch=gfx906 \
+// RUN:   --hip-link -fgpu-rdc --offload-arch=gfx906 \
 // RUN:   -fuse-ld=lld -nostdlib -r %t.main.o %t.lib.o -o %t.final.o \
 // RUN:   2>&1 | FileCheck -check-prefix=LINK-O %s
 // LINK-O-NOT: Found undefined HIP {{.*}}symbol
diff --git a/clang/test/Driver/hip-runtime-libs-linux.hip b/clang/test/Driver/hip-runtime-libs-linux.hip
index 142582963c958f..a4cd2733114b69 100644
--- a/clang/test/Driver/hip-runtime-libs-linux.hip
+++ b/clang/test/Driver/hip-runtime-libs-linux.hip
@@ -43,6 +43,11 @@
 // RUN:   --rocm-path=%S/Inputs/rocm %t.o 2>&1 \
 // RUN:   | FileCheck -check-prefixes=NOHIPRT %s
 
+// Test HIP runtime lib is not linked with -r.
+// RUN: %clang -### --hip-link -r --target=x86_64-linux-gnu \
+// RUN:   --rocm-path=%S/Inputs/rocm %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefixes=NOHIPRT %s
+
 // Test HIP runtime lib is linked without hip-link if there is HIP input file.
 // RUN: %clang -### --target=x86_64-linux-gnu -nogpuinc -nogpulib \
 // RUN:   --rocm-path=%S/Inputs/rocm %s 2>&1 \

Copy link

github-actions bot commented Mar 18, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@scchan scchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

since it will cause duplicate symbols when the partially
linked object is linked again.
@yxsamliu yxsamliu merged commit e7175b0 into llvm:main Mar 20, 2024
4 checks passed
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
since it will cause duplicate symbols when the partially linked object
is linked again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants