[clang] [llvm] Tentative fix for not removing newly internal functions (PR #106146)

[Greg Roth via cfe-commits]

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

>From 1e39029007dee5825810965fa39f26996ec9b7aa Mon Sep 17 00:00:00 2001
From: Greg Roth <grroth at microsoft.com>
Date: Sun, 25 Aug 2024 12:00:03 -0600
Subject: [PATCH] [DirectX] Remove trivially dead functions at linkage finalize

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.

Tests both the pass in isolation and the full inlining, linkage
finalization, and function removal process.

Fixes #106139
---
 .../CodeGenHLSL/remove-internal-unused.hlsl   | 47 +++++++++++
 .../Target/DirectX/DXILFinalizeLinkage.cpp    | 16 ++--
 .../DirectX/finalize-linkage-remove-dead.ll   | 80 +++++++++++++++++++
 3 files changed, 135 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/CodeGenHLSL/remove-internal-unused.hlsl
 create mode 100644 llvm/test/CodeGen/DirectX/finalize-linkage-remove-dead.ll

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 }