[AMDGPU] Remove dead function metadata after amdgpu-lower-kernel-arguments

[slinder1]

The verifier ensures function !dbg metadata is unique across the module,
so ensure the old nameless function we leave behind doesn't violate
this invariant.

Removing the function via e.g. eraseFromParent seems like a better
option, but doesn't seem to be legal from a FunctionPass.

[slinder1]

• [AMDGPU] Push amdgpu-preload-kern-arg-prolog after livedebugvalues #126148
• [AMDGPU] Remove dead function metadata after amdgpu-lower-kernel-arguments #126147 👈 (View in Graphite)
• [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

The verifier ensures function !dbg metadata is unique across the module,
so ensure the old nameless function we leave behind doesn't violate
this invariant.

Removing the function via e.g. eraseFromParent seems like a better
option, but doesn't seem to be legal from a FunctionPass.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp (+1)
• (modified) llvm/test/CodeGen/AMDGPU/preload-implicit-kernargs-debug-info.ll (+3-3)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
index e9d009baa20af2..09412d1b0f1cc9 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
@@ -132,6 +132,7 @@ class PreloadKernelArgInfo {
     NF->setAttributes(AL);
     F.replaceAllUsesWith(NF);
     F.setCallingConv(CallingConv::C);
+    F.clearMetadata();
 
     return NF;
   }
diff --git a/llvm/test/CodeGen/AMDGPU/preload-implicit-kernargs-debug-info.ll b/llvm/test/CodeGen/AMDGPU/preload-implicit-kernargs-debug-info.ll
index fe110cbcafc465..4b47a218f1be45 100644
--- a/llvm/test/CodeGen/AMDGPU/preload-implicit-kernargs-debug-info.ll
+++ b/llvm/test/CodeGen/AMDGPU/preload-implicit-kernargs-debug-info.ll
@@ -1,7 +1,7 @@
-; RUN: not --crash opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx940 -passes='amdgpu-attributor,function(amdgpu-lower-kernel-arguments)' -amdgpu-kernarg-preload-count=16 -S < %s 2>&1 | FileCheck %s
+; RUN: opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx940 -passes='amdgpu-attributor,function(amdgpu-lower-kernel-arguments)' -amdgpu-kernarg-preload-count=16 -S < %s 2>&1 | FileCheck %s
 
-; CHECK: function declaration may only have a unique !dbg attachment
-; CHECK-NEXT: ptr @0
+; CHECK: define amdgpu_kernel void @preload_block_count_x{{.*}} !dbg ![[#]]
+; CHECK-NOT: declare void @0{{.*}} !dbg ![[#]]
 
 define amdgpu_kernel void @preload_block_count_x(ptr addrspace(1) %out) !dbg !4 {
   %imp_arg_ptr = call ptr addrspace(4) @llvm.amdgcn.implicitarg.ptr()

[kerbowa]

This shouldn't be needed after #123547, there are other bugs besides this caused by the leftover declarations.

[arsenm]

this is still a big aggressive for check-not, and I'm not sure it supports regex. Can you simplify to just check-not=declare and explicitly check the few declares that are expected?

[slinder1]

I had already double-checked that --implicit-check-not supports regex (just --implicit-check-not='{{.*}}' will fail on the first line)

I went that route to avoid the awkward CHECKs for the irrelevant declares of llvm.amdgcn.implicitarg.ptr and llvm.amdgcn.kernarg.segment.ptr, but I posted a version with --implicit-check-not=declare and the extra CHECKs; let me know what you think

[slinder1]

For some reason pushing didn't clear your approval, but I wanted to double check your thoughts on the test change so I re-requested

[slinder1]

I am not certain I understand the github interface, but it seems to be saying you approved the latest patch, so I will land and anything else can be resolved post-commit

[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:27 PM EST: A user merged this pull request with Graphite.