Skip to content

Conversation

@shiltian
Copy link
Contributor

@shiltian shiltian commented May 5, 2025

No description provided.

@shiltian shiltian requested a review from jhuber6 May 5, 2025 15:53
Copy link
Contributor Author

shiltian commented May 5, 2025

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels May 5, 2025
@llvmbot
Copy link
Member

llvmbot commented May 5, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Shilei Tian (shiltian)

Changes

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

3 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+5-6)
  • (modified) clang/test/Driver/hip-options.hip (-6)
  • (modified) clang/test/Driver/openmp-offload-gpu.c (+1-7)
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 093f6fe9d5c77..b2e0e5c857228 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -9284,12 +9284,6 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
         CmdArgs.push_back(Args.MakeArgString(
             "--device-linker=" + TC->getTripleString() + "=" + Arg));
 
-      // Enable internalization for AMDGPU.
-      if (TC->getTriple().isAMDGPU())
-        CmdArgs.push_back(
-            Args.MakeArgString("--device-linker=" + TC->getTripleString() +
-                               "=-plugin-opt=-amdgpu-internalize-symbols"));
-
       // Forward the LTO mode relying on the Driver's parsing.
       if (C.getDriver().getOffloadLTOMode() == LTOK_Full)
         CmdArgs.push_back(Args.MakeArgString(
@@ -9304,6 +9298,11 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
           CmdArgs.push_back(
               Args.MakeArgString("--device-linker=" + TC->getTripleString() +
                                  "=-plugin-opt=-avail-extern-to-local"));
+          if (Kind == Action::OFK_OpenMP) {
+            CmdArgs.push_back(
+                Args.MakeArgString("--device-linker=" + TC->getTripleString() +
+                                   "=-plugin-opt=-amdgpu-internalize-symbols"));
+          }
         }
       }
     }
diff --git a/clang/test/Driver/hip-options.hip b/clang/test/Driver/hip-options.hip
index 2a4d7d2cfd8bb..a07dca3638565 100644
--- a/clang/test/Driver/hip-options.hip
+++ b/clang/test/Driver/hip-options.hip
@@ -259,9 +259,3 @@
 // RUN:   --offload-arch=gfx1100 --offload-new-driver --offload-jobs=0x4 %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVJOBS %s
 // INVJOBS: clang: error: invalid integral value '0x4' in '--offload-jobs=0x4'
-
-// Check that internalization is enabled for offloading on AMDGPU.
-// RUN: %clang -### -Werror --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
-// RUN:   --offload-arch=gfx1100 --offload-new-driver %s 2>&1 | \
-// RUN:   FileCheck -check-prefix=INTERNALIZATION %s
-// INTERNALIZATION: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-amdgpu-internalize-symbols
diff --git a/clang/test/Driver/openmp-offload-gpu.c b/clang/test/Driver/openmp-offload-gpu.c
index 73dc5a7f75d5b..f67c2173cb144 100644
--- a/clang/test/Driver/openmp-offload-gpu.c
+++ b/clang/test/Driver/openmp-offload-gpu.c
@@ -388,15 +388,9 @@
 // THINLTO-GFX906: --device-compiler=amdgcn-amd-amdhsa=-flto=thin
 // THINLTO-GFX906-SAME: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-force-import-all
 // THINLTO-GFX906-SAME: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-avail-extern-to-local
+// THINLTO-GFX906-SAME: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-amdgpu-internalize-symbols
 //
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp \
 // RUN:     --offload-arch=sm_52 -foffload-lto=thin -nogpulib -nogpuinc %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=THINLTO-SM52 %s
 // THINLTO-SM52: --device-compiler=nvptx64-nvidia-cuda=-flto=thin
-
-// Check that internalization is enabled for OpenMP offloading on AMDGPU.
-//
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp \
-// RUN:     --offload-arch=gfx906 -nogpulib -nogpuinc %s 2>&1 \
-// RUN:   | FileCheck --check-prefix=INTERNALIZATION-GFX906 %s
-// INTERNALIZATION-GFX906: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-amdgpu-internalize-symbols

@shiltian shiltian merged commit 8ae9a20 into main May 5, 2025
14 checks passed
@shiltian shiltian deleted the users/shiltian/only-internalize-for-openmp-with-thinlto branch May 5, 2025 17:10
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
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.

4 participants