[AMDGPU] Push amdgpu-preload-kern-arg-prolog after livedebugvalues

[slinder1]

This is effectively a workaround for a bug in livedebugvalues, but seems
to potentially be a general improvement, as BB sections seems like it
could ruin the special 256-byte prelude scheme that
amdgpu-preload-kern-arg-prolog requires anyway. Moving it even later
doesn't seem to have any material impact, and just adds livedebugvalues
to the list of things which no longer have to deal with pseudo
multiple-entry functions.

AMDGPU debug-info isn't supported upstream yet, so the bug being avoided
isn't testable here. I am posting the patch upstream to avoid an
unnecessary diff with AMD's fork.

[slinder1]

• [AMDGPU] Push amdgpu-preload-kern-arg-prolog after livedebugvalues #126148 👈 (View in Graphite)
• [AMDGPU] Remove dead function metadata after amdgpu-lower-kernel-arguments #126147
• [AMDGPU] Add test for failure with function !dbg info in amdgpu-lower-kernel-arguments #126146
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Scott Linder (slinder1)

Changes

This is effectively a workaround for a bug in livedebugvalues, but seems
to potentially be a general improvement, as BB sections seems like it
could ruin the special 256-byte prelude scheme that
amdgpu-preload-kern-arg-prolog requires anyway. Moving it even later
doesn't seem to have any material impact, and just adds livedebugvalues
to the list of things which no longer have to deal with pseudo
multiple-entry functions.

AMDGPU debug-info isn't supported upstream yet, so the bug being avoided
isn't testable here. I am posting the patch upstream to avoid an
unnecessary diff with AMD's fork.

---

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

1 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp (+6)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
index fffd30b26dc1d5..67b155e4e74001 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -1147,6 +1147,7 @@ class GCNPassConfig final : public AMDGPUPassConfig {
   void addPostRegAlloc() override;
   void addPreSched2() override;
   void addPreEmitPass() override;
+  void addPostBBSections() override;
 };
 
 } // end anonymous namespace
@@ -1686,6 +1687,11 @@ void GCNPassConfig::addPreEmitPass() {
     addPass(&AMDGPUInsertDelayAluID);
 
   addPass(&BranchRelaxationPassID);
+}
+
+void GCNPassConfig::addPostBBSections() {
+  // We run this later to avoid passes like livedebugvalues and BBSections
+  // having to deal with the apparent multi-entry functions we may generate.
   addPass(createAMDGPUPreloadKernArgPrologLegacyPass());
 }
 

[arsenm]

lgtm, assuming there's a downstream test for whatever bug this is avoiding

[kerbowa]

This LGTM but I think previous PR in stack should be obsolete after #123547

[slinder1]

I do have an llvm-reduce'd test case for downstream, just waiting on things to flow down first. I can't actually add the test at all without a fix for the duplicated metadata so didn't seem worth posting the review yet

[slinder1]

Merge activity

• Feb 17, 1:26 PM EST: A user started a stack merge that includes this pull request via Graphite.
• Feb 17, 1:28 PM EST: Graphite rebased this pull request as part of a merge.
• Feb 17, 1:29 PM EST: A user merged this pull request with Graphite.