[clang] [llvm] [DirectX] Remove trivially dead functions at linkage finalize (PR #106146)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-hlsl

Author: Greg Roth (pow2clk)

<details>
<summary>Changes</summary>

Functions are not removed even when made internal by DXILFinalizeLinkage The removal code is called from alwaysinliner and globalopt, which are invoked too early to remove functions made internal by this pass.

This adds a check similar to that in alwaysinliner that removes trivially dead functions after being marked internal. It refactors that code a bit to make it simpler including reversing what is stored in the work queue.

Not sure how to test this. To test all the interactions between alwaysinliner, DXILfinalizelinkage and any other optimization passes, it kinda needs to be end-to-end.

Fixes #<!-- -->106139

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


3 Files Affected:

- (added) clang/test/CodeGenHLSL/remove-internal-unused.hlsl (+47) 
- (modified) llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp (+8-8) 
- (added) llvm/test/CodeGen/DirectX/finalize-linkage-remove-dead.ll (+80) 


``````````diff
diff --git a/clang/test/CodeGenHLSL/remove-internal-unused.hlsl b/clang/test/CodeGenHLSL/remove-internal-unused.hlsl
new file mode 100644
index 00000000000000..85c114618a1e0e
--- /dev/null
+++ b/clang/test/CodeGenHLSL/remove-internal-unused.hlsl
@@ -0,0 +1,47 @@
+// RUN: %clang -target dxil-pc-shadermodel6.0-compute -S -o - %s | FileCheck %s
+// RUN: %clang -target dxil-pc-shadermodel6.3-library -S -o - %s | FileCheck %s
+
+// Verify that internal linkage unused functions are removed
+
+RWBuffer<unsigned> buf;
+
+// Never called functions should be removed.
+// CHECK-NOT: define{{.*}}uncalledFor
+void uncalledFor() {
+     buf[1] = 1;
+}
+
+// Never called but exported functions should remain.
+// CHECK: define void @"?exported@@YAXXZ"()
+export void exported() {
+     buf[1] = 1;
+}
+
+// Never called but noinlined functions should remain.
+// CHECK: define internal void @"?noinlined@@YAXXZ"()
+__attribute__((noinline)) void noinlined() {
+     buf[1] = 1;
+}
+
+// Called functions marked noinline should remain.
+// CHECK: define internal void @"?calledAndNoinlined@@YAXXZ"()
+__attribute__((noinline)) void calledAndNoinlined() {
+     buf[1] = 1;
+}
+
+// Called functions that get inlined by default should be removed.
+// CHECK-NOT: define{{.*}}calledAndInlined
+void calledAndInlined() {
+     buf[1] = 1;
+}
+
+
+// Entry point functions should remain.
+// CHECK: define{{.*}}main
+[numthreads(1,1,1)]
+[shader("compute")]
+void main() {
+     calledAndInlined();
+     calledAndNoinlined();
+     buf[0] = 0;
+}
\ No newline at end of file
diff --git a/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp b/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
index d315d9bd16f439..59b30f965bf951 100644
--- a/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
+++ b/llvm/lib/Target/DirectX/DXILFinalizeLinkage.cpp
@@ -19,20 +19,20 @@
 using namespace llvm;
 
 static bool finalizeLinkage(Module &M) {
-  SmallPtrSet<Function *, 8> EntriesAndExports;
+  SmallPtrSet<Function *, 8> Funcs;
 
   // Find all entry points and export functions
   for (Function &EF : M.functions()) {
-    if (!EF.hasFnAttribute("hlsl.shader") && !EF.hasFnAttribute("hlsl.export"))
+    if (EF.hasFnAttribute("hlsl.shader") || EF.hasFnAttribute("hlsl.export"))
       continue;
-    EntriesAndExports.insert(&EF);
+    Funcs.insert(&EF);
   }
 
-  for (Function &F : M.functions()) {
-    if (F.getLinkage() == GlobalValue::ExternalLinkage &&
-        !EntriesAndExports.contains(&F)) {
-      F.setLinkage(GlobalValue::InternalLinkage);
-    }
+  for (Function *F : Funcs) {
+    if (F->getLinkage() == GlobalValue::ExternalLinkage)
+      F->setLinkage(GlobalValue::InternalLinkage);
+    if (F->hasFnAttribute(Attribute::AlwaysInline) && F->isDefTriviallyDead())
+      M.getFunctionList().erase(F);
   }
 
   return false;
diff --git a/llvm/test/CodeGen/DirectX/finalize-linkage-remove-dead.ll b/llvm/test/CodeGen/DirectX/finalize-linkage-remove-dead.ll
new file mode 100644
index 00000000000000..df5934355664d1
--- /dev/null
+++ b/llvm/test/CodeGen/DirectX/finalize-linkage-remove-dead.ll
@@ -0,0 +1,80 @@
+; RUN: opt -S -dxil-finalize-linkage -mtriple=dxil-unknown-shadermodel6.5-compute %s | FileCheck %s
+; RUN: llc %s --filetype=asm -o - | FileCheck %s
+
+target triple = "dxilv1.5-pc-shadermodel6.5-compute"
+
+; Confirm that DXILFinalizeLinkage will remove functions that have compatible
+; linkage and are not called from anywhere. This should be any function that
+; is not explicitly marked noinline or export and is not an entry point.
+
+; Not called nor marked with any linking or inlining attributes.
+; CHECK-NOT: define {{.*}}doNothingNothing
+define void @"?doNothingNothing@@YAXXZ"() #0 {
+entry:
+  ret void
+}
+
+; Marked internal, this should be removed.
+; CHECK-NOT: define {{.*}}doNothingInternally
+define internal void @"?doNothingInternally@@YAXXZ"() #0 {
+entry:
+  ret void
+}
+
+; Marked external, which should become internal and be removed.
+; CHECK-NOT: define {{.*}}doNothingExternally
+define external void @"?doNothingExternally@@YAXXZ"() #0 {
+entry:
+  ret void
+}
+
+; Not called nor marked with any linking or inlining attributes.
+; CHECK: define internal void @"?doSomethingSomething@@YAXXZ"() #0
+define void @"?doSomethingSomething@@YAXXZ"() #0 {
+entry:
+  ret void
+}
+
+; Marked internal, this should be removed.
+; CHECK: define internal void @"?doSomethingInternally@@YAXXZ"() #0
+define internal void @"?doSomethingInternally@@YAXXZ"() #0 {
+entry:
+  ret void
+}
+
+; Marked external, which should become internal and be removed.
+; CHECK: define internal void @"?doSomethingExternally@@YAXXZ"() #0
+define external void @"?doSomethingExternally@@YAXXZ"() #0 {
+entry:
+  ret void
+}
+
+; Lacks alwaysinline attribute. Should remain.
+; CHECK: define internal void @"?doNothingDefault@@YAXXZ"() #1
+define void @"?doNothingDefault@@YAXXZ"() #1 {
+entry:
+  ret void
+}
+
+; Has noinline attribute. Should remain.
+; CHECK: define {{.*}}doNothingNoinline
+define void @"?doNothingNoinline@@YAXXZ"() #2 {
+entry:
+  ret void
+}
+
+; Entry point function should stay.
+; CHECK: define void @main() #3
+define void @main() #3 {
+entry:
+  call void @"?doSomethingSomething@@YAXXZ"() #4
+  call void @"?doSomethingInternally@@YAXXZ"() #4
+  call void @"?doSomethingExternally@@YAXXZ"() #4
+  ret void
+}
+
+attributes #0 = { alwaysinline convergent norecurse nounwind }
+attributes #1 = { convergent norecurse nounwind }
+attributes #2 = { convergent noinline norecurse nounwind }
+attributes #3 = { convergent noinline norecurse "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }
+attributes #4 = { convergent }

``````````

</details>


https://github.com/llvm/llvm-project/pull/106146