-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Clang][AMDGPU][Driver] Add avail-extern-gv-in-addrspace-to-local option when ThinTLO is enabled
#144914
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
[Clang][AMDGPU][Driver] Add avail-extern-gv-in-addrspace-to-local option when ThinTLO is enabled
#144914
Conversation
…ption when ThinTLO is enabled On AMDGPU, we need an extra argument `-avail-extern-gv-in-addrspace-to-local=3` to privatize LDS global variables when ThinLTO is enabled.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-clang Author: Shilei Tian (shiltian) ChangesOn AMDGPU, we need an extra argument Full diff: https://github.com/llvm/llvm-project/pull/144914.diff 4 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a78a1c8978183..3314cdf96a081 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -9186,6 +9186,9 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back(
Args.MakeArgString("--device-linker=" + TC->getTripleString() +
"=-plugin-opt=-avail-extern-to-local"));
+ CmdArgs.push_back(Args.MakeArgString(
+ "--device-linker=" + TC->getTripleString() +
+ "=-plugin-opt=-avail-extern-gv-in-addrspace-to-local=3"));
if (Kind == Action::OFK_OpenMP) {
CmdArgs.push_back(
Args.MakeArgString("--device-linker=" + TC->getTripleString() +
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index 74ac8306e7cc1..b8f3a70ee827e 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -102,6 +102,8 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA,
if (IsThinLTO) {
LldArgs.push_back(Args.MakeArgString("-plugin-opt=-force-import-all"));
LldArgs.push_back(Args.MakeArgString("-plugin-opt=-avail-extern-to-local"));
+ LldArgs.push_back(Args.MakeArgString(
+ "-plugin-opt=-avail-extern-gv-in-addrspace-to-local=3"));
}
for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
diff --git a/clang/test/Driver/hip-thinlto.hip b/clang/test/Driver/hip-thinlto.hip
index 4227cd3f2e9f9..bcb7d4e6cb52e 100644
--- a/clang/test/Driver/hip-thinlto.hip
+++ b/clang/test/Driver/hip-thinlto.hip
@@ -3,6 +3,7 @@
// CHECK: -plugin-opt=thinlto
// CHECK-SAME: -plugin-opt=-force-import-all
// CHECK-SAME: -plugin-opt=-avail-extern-to-local
+// CHECK-SAME: -plugin-opt=-avail-extern-gv-in-addrspace-to-local=3
int main(int, char *[]) {
return 0;
}
diff --git a/clang/test/Driver/openmp-offload-gpu.c b/clang/test/Driver/openmp-offload-gpu.c
index f67c2173cb144..2af3e2da3b21c 100644
--- a/clang/test/Driver/openmp-offload-gpu.c
+++ b/clang/test/Driver/openmp-offload-gpu.c
@@ -388,6 +388,7 @@
// 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=-avail-extern-gv-in-addrspace-to-local=3
// THINLTO-GFX906-SAME: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-amdgpu-internalize-symbols
//
// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp \
|
|
@llvm/pr-subscribers-clang-driver Author: Shilei Tian (shiltian) ChangesOn AMDGPU, we need an extra argument Full diff: https://github.com/llvm/llvm-project/pull/144914.diff 4 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index a78a1c8978183..3314cdf96a081 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -9186,6 +9186,9 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back(
Args.MakeArgString("--device-linker=" + TC->getTripleString() +
"=-plugin-opt=-avail-extern-to-local"));
+ CmdArgs.push_back(Args.MakeArgString(
+ "--device-linker=" + TC->getTripleString() +
+ "=-plugin-opt=-avail-extern-gv-in-addrspace-to-local=3"));
if (Kind == Action::OFK_OpenMP) {
CmdArgs.push_back(
Args.MakeArgString("--device-linker=" + TC->getTripleString() +
diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp
index 74ac8306e7cc1..b8f3a70ee827e 100644
--- a/clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -102,6 +102,8 @@ void AMDGCN::Linker::constructLldCommand(Compilation &C, const JobAction &JA,
if (IsThinLTO) {
LldArgs.push_back(Args.MakeArgString("-plugin-opt=-force-import-all"));
LldArgs.push_back(Args.MakeArgString("-plugin-opt=-avail-extern-to-local"));
+ LldArgs.push_back(Args.MakeArgString(
+ "-plugin-opt=-avail-extern-gv-in-addrspace-to-local=3"));
}
for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
diff --git a/clang/test/Driver/hip-thinlto.hip b/clang/test/Driver/hip-thinlto.hip
index 4227cd3f2e9f9..bcb7d4e6cb52e 100644
--- a/clang/test/Driver/hip-thinlto.hip
+++ b/clang/test/Driver/hip-thinlto.hip
@@ -3,6 +3,7 @@
// CHECK: -plugin-opt=thinlto
// CHECK-SAME: -plugin-opt=-force-import-all
// CHECK-SAME: -plugin-opt=-avail-extern-to-local
+// CHECK-SAME: -plugin-opt=-avail-extern-gv-in-addrspace-to-local=3
int main(int, char *[]) {
return 0;
}
diff --git a/clang/test/Driver/openmp-offload-gpu.c b/clang/test/Driver/openmp-offload-gpu.c
index f67c2173cb144..2af3e2da3b21c 100644
--- a/clang/test/Driver/openmp-offload-gpu.c
+++ b/clang/test/Driver/openmp-offload-gpu.c
@@ -388,6 +388,7 @@
// 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=-avail-extern-gv-in-addrspace-to-local=3
// THINLTO-GFX906-SAME: --device-linker=amdgcn-amd-amdhsa=-plugin-opt=-amdgpu-internalize-symbols
//
// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp=libomp \
|
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.
Bit of a hack it seems, but better than it being broken.
It is a hack (or workaround). The correct approach would be either to redesign the lowering of LDS, or create another flavor of ThinLTO. Neither of it can be done in a reasonably short amount of time. |

On AMDGPU, we need an extra argument
-avail-extern-gv-in-addrspace-to-local=3to privatize LDS global variables when ThinLTO is enabled.