[AMDGPU][LTO] Assume closed world after linking #105845
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-lto @llvm/pr-subscribers-clang Author: Anshil Gandhi (gandhi56) ChangesChange-Id: I7d8fa4251c80a6f815f55a0998677d18ade25b72 Full diff: https://github.com/llvm/llvm-project/pull/105845.diff 3 Files Affected:
diff --git a/clang/test/CodeGenCUDA/gpu-rdc-amdgpu-attrs.cu b/clang/test/CodeGenCUDA/gpu-rdc-amdgpu-attrs.cu
new file mode 100644
index 00000000000000..614917aecc0d60
--- /dev/null
+++ b/clang/test/CodeGenCUDA/gpu-rdc-amdgpu-attrs.cu
@@ -0,0 +1,12 @@
+// RUN: clang -x hip -O3 -fgpu-rdc %s -mllvm -debug-only=amdgpu-attributor -o - | FileCheck %s
+
+// CHECK: Module {{.*}} is not assumed to be a closed world
+// CHECK: Module ld-temp.o is assumed to be a closed world
+
+__attribute__((device)) int foo() {
+ return 1;
+}
+
+int main() {
+ return 0;
+}
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
index d65e0ae92308e6..c78fc66e41ec58 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
@@ -1066,6 +1066,9 @@ static bool runImpl(Module &M, AnalysisGetter &AG, TargetMachine &TM,
Attributor A(Functions, InfoCache, AC);
+ LLVM_DEBUG(dbgs() << "Module " << M.getName() << " is " << (AC.IsClosedWorldModule ? "" : "not ")
+ << "assumed to be a closed world\n");
+
for (Function &F : M) {
if (F.isIntrinsic())
continue;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index 7ac7b3315bb972..a4898366a21ee1 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -761,7 +761,7 @@ void AMDGPUTargetMachine::registerPassBuilderCallbacks(PassBuilder &PB) {
if (EnableLowerModuleLDS)
PM.addPass(AMDGPULowerModuleLDSPass(*this));
if (EnableAMDGPUAttributor && Level != OptimizationLevel::O0)
- PM.addPass(AMDGPUAttributorPass(*this));
+ PM.addPass(AMDGPUAttributorPass(*this, AMDGPUAttributorOptions{true} ));
});
PB.registerRegClassFilterParsingCallback(
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
gandhi56
force-pushed
the
gpu-rdc/assume-closed-world-after-linking
branch
from
8830b6f to
d4b8e5b
Compare
shiltian
reviewed
Aug 23, 2024
shiltian
requested changes
Aug 23, 2024
gandhi56
force-pushed
the
gpu-rdc/assume-closed-world-after-linking
branch
from
d4b8e5b to
35601ad
Compare
gandhi56
force-pushed
the
gpu-rdc/assume-closed-world-after-linking
branch
from
35601ad to
e01a7c8
Compare
shiltian
reviewed
Aug 23, 2024
shiltian
reviewed
Aug 23, 2024
gandhi56
force-pushed
the
gpu-rdc/assume-closed-world-after-linking
branch
from
e01a7c8 to
cec79bd
Compare
shiltian
approved these changes
Aug 23, 2024
gandhi56
force-pushed
the
gpu-rdc/assume-closed-world-after-linking
branch
from
cec79bd to
3c14ab2
Compare
Change-Id: I7d8fa4251c80a6f815f55a0998677d18ade25b72
gandhi56
force-pushed
the
gpu-rdc/assume-closed-world-after-linking
branch
from
3c14ab2 to
beed3cb
Compare
|
Rebased against trunk |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/3896 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/5430 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/3337 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/3894 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/12/builds/4442 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/3288 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/3813 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/3740 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/155/builds/1751 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/4407 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/3914 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/63/builds/1210 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/13/builds/1773 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/54/builds/1624 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/2343 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/1832 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/193/builds/2162 Here is the relevant piece of the build log for the reference |
gandhi56
added a commit
to gandhi56/llvm-project
that referenced
this pull request
Aug 25, 2024
This reverts commit 33f3ebc.
gandhi56
added a commit
that referenced
this pull request
Aug 25, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/81/builds/1751 Here is the relevant piece of the build log for the reference |
gandhi56
added a commit
to gandhi56/llvm-project
that referenced
this pull request
Aug 25, 2024
…#105845)" (llvm#106000)" This reverts commit 4b6c064. Add a requirement for an amdgpu target in the test.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/38/builds/287 Here is the relevant piece of the build log for the reference |
gandhi56
added a commit
that referenced
this pull request
Aug 25, 2024
shiltian
added a commit
that referenced
this pull request
Sep 1, 2024
We can't assume closed world even in full LTO post-link stage. It is only true if we are building a "GPU executable". However, AMDGPU does support "dyamic library". I'm not aware of any approach to tell if it is relocatable link when we create the pass. For now let's revert the patch. We can re-enable it once we can handle it correctly.
shiltian
added a commit
that referenced
this pull request
Sep 1, 2024
…106889) We can't assume closed world even in full LTO post-link stage. It is only true if we are building a "GPU executable". However, AMDGPU does support "dyamic library". I'm not aware of any approach to tell if it is relocatable link when we create the pass. For now let's revert the patch as it is currently breaking things. We can re-enable it once we can handle it correctly.
qiaojbao
pushed a commit
to GPUOpen-Drivers/llvm-project
that referenced
this pull request
Sep 30, 2024
…094d86999 Local branch amd-gfx f1e094d Merged main:1193f7d6487d2d94009f8d8d27da3907136482b9 into amd-gfx:56dd30a1ccd0 Remote branch main 33f3ebc [AMDGPU][LTO] Assume closed world after linking (llvm#105845)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.