[llvm] [MergeFunc] Remove discardables function before writing alias or thunk. (PR #128865)

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 27 04:55:57 PST 2025


https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/128865

>From 1548cd44bc1b6eacfb86614e577d43cec7f6e3d8 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Mon, 24 Feb 2025 18:54:56 +0000
Subject: [PATCH 1/4] [MergeFunc] Remove discardables function before writing
 alias or thunk.

Update writeThunkOrAlias to only create an alias or thunk if it is
actually needed. Drop discardable linkone_odr functions if they are not
used before.
---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp    | 36 +++++++++++--------
 llvm/test/Transforms/MergeFunc/linkonce.ll    |  8 +----
 .../test/Transforms/MergeFunc/linkonce_odr.ll |  8 +----
 .../MergeFunc/merge-linkonce-odr.ll           | 14 +-------
 4 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 9cc159830b17d..013dd0e04457b 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -283,9 +283,10 @@ class MergeFunctions {
   // Replace G with an alias to F (deleting function G)
   void writeAlias(Function *F, Function *G);
 
-  // Replace G with an alias to F if possible, or a thunk to F if possible.
-  // Returns false if neither is the case.
-  bool writeThunkOrAlias(Function *F, Function *G);
+  // If needed, replace G with an alias to F if possible, or a thunk to F if
+  // profitable. Returns false if neither is the case. If \p G is not needed
+  // (e.g. it is discardable and linkonce_odr), \p G is removed directly.
+  bool writeThunkOrAliasIfNeeded(Function *F, Function *G);
 
   /// Replace function F with function G in the function tree.
   void replaceFunctionInTree(const FunctionNode &FN, Function *G);
@@ -875,9 +876,15 @@ void MergeFunctions::writeAlias(Function *F, Function *G) {
   ++NumAliasesWritten;
 }
 
-// Replace G with an alias to F if possible, or a thunk to F if
-// profitable. Returns false if neither is the case.
-bool MergeFunctions::writeThunkOrAlias(Function *F, Function *G) {
+// If needed, replace G with an alias to F if possible, or a thunk to F if
+// profitable. Returns false if neither is the case. If \p G is not needed (e.g.
+// it is discardable and linkonce_odr), \p G is removed directly.
+bool MergeFunctions::writeThunkOrAliasIfNeeded(Function *F, Function *G) {
+  if (G->isDiscardableIfUnused() &&
+      G->use_empty() && !MergeFunctionsPDI) {
+    G->eraseFromParent();
+    return true;
+  }
   if (canCreateAliasFor(G)) {
     writeAlias(F, G);
     return true;
@@ -904,9 +911,10 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
     assert((!isODR(G) || isODR(F)) &&
            "if G is ODR, F must also be ODR due to ordering");
 
-    // Both writeThunkOrAlias() calls below must succeed, either because we can
-    // create aliases for G and NewF, or because a thunk for F is profitable.
-    // F here has the same signature as NewF below, so that's what we check.
+    // Both writeThunkOrAliasIfNeeded() calls below must succeed, either because
+    // we can create aliases for G and NewF, or because a thunk for F is
+    // profitable. F here has the same signature as NewF below, so that's what
+    // we check.
     if (!canCreateThunkFor(F) &&
         (!canCreateAliasFor(F) || !canCreateAliasFor(G)))
       return;
@@ -930,13 +938,13 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
     if (isODR(F))
       replaceDirectCallers(NewF, F);
 
-    // We collect alignment before writeThunkOrAlias that overwrites NewF and
-    // G's content.
+    // We collect alignment before writeThunkOrAliasIfNeeded that overwrites
+    // NewF and G's content.
     const MaybeAlign NewFAlign = NewF->getAlign();
     const MaybeAlign GAlign = G->getAlign();
 
-    writeThunkOrAlias(F, G);
-    writeThunkOrAlias(F, NewF);
+    writeThunkOrAliasIfNeeded(F, G);
+    writeThunkOrAliasIfNeeded(F, NewF);
 
     if (NewFAlign || GAlign)
       F->setAlignment(std::max(NewFAlign.valueOrOne(), GAlign.valueOrOne()));
@@ -975,7 +983,7 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
       return;
     }
 
-    if (writeThunkOrAlias(F, G)) {
+    if (writeThunkOrAliasIfNeeded(F, G)) {
       ++NumFunctionsMerged;
     }
   }
diff --git a/llvm/test/Transforms/MergeFunc/linkonce.ll b/llvm/test/Transforms/MergeFunc/linkonce.ll
index 2f648edc62408..2b65234b02cc2 100644
--- a/llvm/test/Transforms/MergeFunc/linkonce.ll
+++ b/llvm/test/Transforms/MergeFunc/linkonce.ll
@@ -43,14 +43,8 @@ define linkonce i32 @funA(i32 %x, i32 %y) {
 ; CHECK-NEXT:    ret i32 [[SUM3]]
 ;
 ;
-; CHECK-LABEL: define linkonce i32 @funC(
-; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
-; CHECK-NEXT:    ret i32 [[TMP3]]
-;
-;
 ; CHECK-LABEL: define linkonce i32 @funB(
 ; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0]](i32 [[TMP0]], i32 [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
 ; CHECK-NEXT:    ret i32 [[TMP3]]
 ;
diff --git a/llvm/test/Transforms/MergeFunc/linkonce_odr.ll b/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
index ecbe6f08ab8c2..4ff34c7cd17fe 100644
--- a/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
+++ b/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
@@ -43,14 +43,8 @@ define linkonce_odr i32 @funA(i32 %x, i32 %y) {
 ; CHECK-NEXT:    ret i32 [[SUM3]]
 ;
 ;
-; CHECK-LABEL: define linkonce_odr i32 @funC(
-; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
-; CHECK-NEXT:    ret i32 [[TMP3]]
-;
-;
 ; CHECK-LABEL: define linkonce_odr i32 @funB(
 ; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0]](i32 [[TMP0]], i32 [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
 ; CHECK-NEXT:    ret i32 [[TMP3]]
 ;
diff --git a/llvm/test/Transforms/MergeFunc/merge-linkonce-odr.ll b/llvm/test/Transforms/MergeFunc/merge-linkonce-odr.ll
index 7e815e1f50d64..1a622bfae72c4 100644
--- a/llvm/test/Transforms/MergeFunc/merge-linkonce-odr.ll
+++ b/llvm/test/Transforms/MergeFunc/merge-linkonce-odr.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
-; RUN: opt -p mergefunc -S %s | FileCheck %s
+; RUN: opt -p mergefunc -S %s | FileCheck --implicit-check-not=linkonce_odr_caller_of_foo_2 --implicit-check-not=linkonce_odr_caller_of_foo_1 %s
 
 define void @caller_of_callers(ptr %p) {
   call void @linkonce_odr_caller_of_foo_1(ptr %p)
@@ -125,15 +125,3 @@ declare void @zar(ptr)
 ; CHECK-NEXT:    tail call void @zar(ptr [[P]])
 ; CHECK-NEXT:    ret void
 ;
-;
-; CHECK-LABEL: define linkonce_odr hidden void @linkonce_odr_caller_of_foo_2(
-; CHECK-SAME: ptr [[TMP0:%.*]]) {
-; CHECK-NEXT:    tail call void @[[GLOB0]](ptr [[TMP0]])
-; CHECK-NEXT:    ret void
-;
-;
-; CHECK-LABEL: define linkonce_odr hidden void @linkonce_odr_caller_of_foo_1(
-; CHECK-SAME: ptr [[TMP0:%.*]]) {
-; CHECK-NEXT:    tail call void @[[GLOB0]](ptr [[TMP0]])
-; CHECK-NEXT:    ret void
-;

>From 9697fbf8afc06c79d4f6195a1a27c5d28fe08660 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 26 Feb 2025 11:45:05 +0000
Subject: [PATCH 2/4] !fixup fix formatting

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 013dd0e04457b..c969884bcd415 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -880,8 +880,7 @@ void MergeFunctions::writeAlias(Function *F, Function *G) {
 // profitable. Returns false if neither is the case. If \p G is not needed (e.g.
 // it is discardable and linkonce_odr), \p G is removed directly.
 bool MergeFunctions::writeThunkOrAliasIfNeeded(Function *F, Function *G) {
-  if (G->isDiscardableIfUnused() &&
-      G->use_empty() && !MergeFunctionsPDI) {
+  if (G->isDiscardableIfUnused() && G->use_empty() && !MergeFunctionsPDI) {
     G->eraseFromParent();
     return true;
   }

>From 7e0d0ebd22dfe53c2bdc1c0fe18e31c9bce004d9 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 26 Feb 2025 21:28:38 +0000
Subject: [PATCH 3/4] !fixup address latest comments, thanks

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp     | 6 +++---
 llvm/test/Transforms/MergeFunc/linkonce.ll     | 2 +-
 llvm/test/Transforms/MergeFunc/linkonce_odr.ll | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index c969884bcd415..d2443a83535e4 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -285,7 +285,7 @@ class MergeFunctions {
 
   // If needed, replace G with an alias to F if possible, or a thunk to F if
   // profitable. Returns false if neither is the case. If \p G is not needed
-  // (e.g. it is discardable and linkonce_odr), \p G is removed directly.
+  // (i.e. it is discardable and not used), \p G is removed directly.
   bool writeThunkOrAliasIfNeeded(Function *F, Function *G);
 
   /// Replace function F with function G in the function tree.
@@ -877,8 +877,8 @@ void MergeFunctions::writeAlias(Function *F, Function *G) {
 }
 
 // If needed, replace G with an alias to F if possible, or a thunk to F if
-// profitable. Returns false if neither is the case. If \p G is not needed (e.g.
-// it is discardable and linkonce_odr), \p G is removed directly.
+// profitable. Returns false if neither is the case. If \p G is not needed (i.e.
+// it is discardable and unused), \p G is removed directly.
 bool MergeFunctions::writeThunkOrAliasIfNeeded(Function *F, Function *G) {
   if (G->isDiscardableIfUnused() && G->use_empty() && !MergeFunctionsPDI) {
     G->eraseFromParent();
diff --git a/llvm/test/Transforms/MergeFunc/linkonce.ll b/llvm/test/Transforms/MergeFunc/linkonce.ll
index 2b65234b02cc2..820fb6c395791 100644
--- a/llvm/test/Transforms/MergeFunc/linkonce.ll
+++ b/llvm/test/Transforms/MergeFunc/linkonce.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
-; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funC
+; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funA -implicit-check-not=funC
 
 ; Replacments should be totally ordered on the function name.
 ; If we don't do this we  can end up with one module defining a thunk for @funA
diff --git a/llvm/test/Transforms/MergeFunc/linkonce_odr.ll b/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
index 4ff34c7cd17fe..416d2b44583dd 100644
--- a/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
+++ b/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
-; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funC
+; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funA -implicit-check-not=funC
 
 ; Replacments should be totally ordered on the function name.
 ; If we don't do this we  can end up with one module defining a thunk for @funA

>From fb08c275f7925092d4ca3915b275625f7741d57d Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Thu, 27 Feb 2025 12:54:51 +0000
Subject: [PATCH 4/4] !fixup Don't use pattern to check unnamed function in
 call.

---
 llvm/test/Transforms/MergeFunc/linkonce.ll     | 3 +--
 llvm/test/Transforms/MergeFunc/linkonce_odr.ll | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/llvm/test/Transforms/MergeFunc/linkonce.ll b/llvm/test/Transforms/MergeFunc/linkonce.ll
index 820fb6c395791..567139d97a387 100644
--- a/llvm/test/Transforms/MergeFunc/linkonce.ll
+++ b/llvm/test/Transforms/MergeFunc/linkonce.ll
@@ -1,4 +1,3 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
 ; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funA -implicit-check-not=funC
 
 ; Replacments should be totally ordered on the function name.
@@ -45,6 +44,6 @@ define linkonce i32 @funA(i32 %x, i32 %y) {
 ;
 ; CHECK-LABEL: define linkonce i32 @funB(
 ; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @0(i32 [[TMP0]], i32 [[TMP1]])
 ; CHECK-NEXT:    ret i32 [[TMP3]]
 ;
diff --git a/llvm/test/Transforms/MergeFunc/linkonce_odr.ll b/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
index 416d2b44583dd..a1cae60685da1 100644
--- a/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
+++ b/llvm/test/Transforms/MergeFunc/linkonce_odr.ll
@@ -1,4 +1,3 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --include-generated-funcs --version 5
 ; RUN: opt -S -passes=mergefunc < %s | FileCheck %s -implicit-check-not=funA -implicit-check-not=funC
 
 ; Replacments should be totally ordered on the function name.
@@ -45,6 +44,6 @@ define linkonce_odr i32 @funA(i32 %x, i32 %y) {
 ;
 ; CHECK-LABEL: define linkonce_odr i32 @funB(
 ; CHECK-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]]) {
-; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @[[GLOB0:[0-9]+]](i32 [[TMP0]], i32 [[TMP1]])
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @0(i32 [[TMP0]], i32 [[TMP1]])
 ; CHECK-NEXT:    ret i32 [[TMP3]]
 ;



More information about the llvm-commits mailing list