[MergeFuncs] Generate alias instead of thunk if possible

The MergeFunctions pass was originally intended to emit aliases instead of thunks where possible (unnamed_addr). However, for a long time this functionality was behind a flag hardcoded to false, bitrotted and was eventually removed in r309313. Originally the functionality was first disabled in r108417 due to lack of support for aliases in Mach-O. I believe that this is no longer the case nowadays, but not really familiar with this area. In the interest of being conservative, this patch reintroduces the aliasing functionality behind a default disabled -mergefunc-use-aliases flag. Differential Revision: https://reviews.llvm.org/D53285 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@347407 91177308-0d34-0410-b5e6-96231b3b80d8
rust-lang · Nov 21, 2018 · 16ca3e1 · 16ca3e1
1 parent 940eac6
commit 16ca3e1
Show file tree

Hide file tree

Showing 2 changed files with 189 additions and 14 deletions.
diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp
@@ -136,6 +136,7 @@ using namespace llvm;
 
 STATISTIC(NumFunctionsMerged, "Number of functions merged");
 STATISTIC(NumThunksWritten, "Number of thunks generated");
+STATISTIC(NumAliasesWritten, "Number of aliases generated");
 STATISTIC(NumDoubleWeak, "Number of new functions created");
 
 static cl::opt<unsigned> NumFunctionsForSanityCheck(
@@ -165,6 +166,11 @@ static cl::opt<bool>
                       cl::desc("Preserve debug info in thunk when mergefunc "
                                "transformations are made."));
 
+static cl::opt<bool>
+    MergeFunctionsAliases("mergefunc-use-aliases", cl::Hidden,
+                          cl::init(false),
+                          cl::desc("Allow mergefunc to create aliases"));
+
 namespace {
 
 class FunctionNode {
@@ -272,6 +278,13 @@ class MergeFunctions : public ModulePass {
   /// delete G.
   void writeThunk(Function *F, Function *G);
 
+  // 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
+  // profitable. Returns false if neither is the case.
+  bool writeThunkOrAlias(Function *F, Function *G);
+
   /// Replace function F with function G in the function tree.
   void replaceFunctionInTree(const FunctionNode &FN, Function *G);
 
@@ -735,27 +748,76 @@ void MergeFunctions::writeThunk(Function *F, Function *G) {
   ++NumThunksWritten;
 }
 
+// Whether this function may be replaced by an alias
+static bool canCreateAliasFor(Function *F) {
+  if (!MergeFunctionsAliases || !F->hasGlobalUnnamedAddr())
+    return false;
+
+  // We should only see linkages supported by aliases here
+  assert(F->hasLocalLinkage() || F->hasExternalLinkage()
+      || F->hasWeakLinkage() || F->hasLinkOnceLinkage());
+  return true;
+}
+
+// Replace G with an alias to F (deleting function G)
+void MergeFunctions::writeAlias(Function *F, Function *G) {
+  Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType());
+  PointerType *PtrType = G->getType();
+  auto *GA = GlobalAlias::create(
+      PtrType->getElementType(), PtrType->getAddressSpace(),
+      G->getLinkage(), "", BitcastF, G->getParent());
+
+  F->setAlignment(std::max(F->getAlignment(), G->getAlignment()));
+  GA->takeName(G);
+  GA->setVisibility(G->getVisibility());
+  GA->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
+
+  removeUsers(G);
+  G->replaceAllUsesWith(GA);
+  G->eraseFromParent();
+
+  LLVM_DEBUG(dbgs() << "writeAlias: " << GA->getName() << '\n');
+  ++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 (canCreateAliasFor(G)) {
+    writeAlias(F, G);
+    return true;
+  }
+  if (isThunkProfitable(F)) {
+    writeThunk(F, G);
+    return true;
+  }
+  return false;
+}
+
 // Merge two equivalent functions. Upon completion, Function G is deleted.
 void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
   if (F->isInterposable()) {
     assert(G->isInterposable());
 
-    if (!isThunkProfitable(F)) {
+    // 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.
+    if (!isThunkProfitable(F) && (!canCreateAliasFor(F) || !canCreateAliasFor(G))) {
       return;
     }
 
     // Make them both thunks to the same internal function.
-    Function *H = Function::Create(F->getFunctionType(), F->getLinkage(), "",
-                                   F->getParent());
-    H->copyAttributesFrom(F);
-    H->takeName(F);
+    Function *NewF = Function::Create(F->getFunctionType(), F->getLinkage(), "",
+                                      F->getParent());
+    NewF->copyAttributesFrom(F);
+    NewF->takeName(F);
     removeUsers(F);
-    F->replaceAllUsesWith(H);
+    F->replaceAllUsesWith(NewF);
 
-    unsigned MaxAlignment = std::max(G->getAlignment(), H->getAlignment());
+    unsigned MaxAlignment = std::max(G->getAlignment(), NewF->getAlignment());
 
-    writeThunk(F, G);
-    writeThunk(F, H);
+    writeThunkOrAlias(F, G);
+    writeThunkOrAlias(F, NewF);
 
     F->setAlignment(MaxAlignment);
     F->setLinkage(GlobalValue::PrivateLinkage);
@@ -789,12 +851,9 @@ void MergeFunctions::mergeTwoFunctions(Function *F, Function *G) {
       return;
     }
 
-    if (!isThunkProfitable(F)) {
-      return;
+    if (writeThunkOrAlias(F, G)) {
+      ++NumFunctionsMerged;
     }
-
-    writeThunk(F, G);
-    ++NumFunctionsMerged;
   }
 }
 

diff --git a/test/Transforms/MergeFunc/alias.ll b/test/Transforms/MergeFunc/alias.ll
@@ -0,0 +1,116 @@
+; RUN: opt -S -mergefunc -mergefunc-use-aliases < %s | FileCheck %s
+
+; Aliases should always be created for the weak functions, and
+; for external functions if there is no local function
+
+; CHECK: @external_external_2 = unnamed_addr alias void (float*), bitcast (void (i32*)* @external_external_1 to void (float*)*)
+; CHECK: @weak_weak_2 = weak unnamed_addr alias void (float*), bitcast (void (i32*)* @0 to void (float*)*)
+; CHECK: @weak_weak_1 = weak unnamed_addr alias void (i32*), void (i32*)* @0
+; CHECK: @weak_external_1 = weak unnamed_addr alias void (i32*), bitcast (void (float*)* @weak_external_2 to void (i32*)*)
+; CHECK: @external_weak_2 = weak unnamed_addr alias void (float*), bitcast (void (i32*)* @external_weak_1 to void (float*)*)
+; CHECK: @weak_internal_1 = weak unnamed_addr alias void (i32*), bitcast (void (float*)* @weak_internal_2 to void (i32*)*)
+; CHECK: @internal_weak_2 = weak unnamed_addr alias void (float*), bitcast (void (i32*)* @internal_weak_1 to void (float*)*)
+
+; A strong backing function had to be created for the weak-weak pair
+
+; CHECK: define private void @0(i32* %a) unnamed_addr
+; CHECK_NEXT: call void @dummy4()
+
+; These internal functions are dropped in favor of the external ones
+
+; CHECK-NOT: define internal void @external_internal_2(float *%a) unnamed_addr
+; CHECK-NOT: define internal void @internal_external_1(i32 *%a) unnamed_addr
+; CHECK-NOT: define internal void @internal_external_1(i32 *%a) unnamed_addr
+; CHECK-NOT: define internal void @internal_external_2(float *%a) unnamed_addr
+
+; Only used to mark which functions should be merged.
+declare void @dummy1()
+declare void @dummy2()
+declare void @dummy3()
+declare void @dummy4()
+declare void @dummy5()
+declare void @dummy6()
+declare void @dummy7()
+declare void @dummy8()
+declare void @dummy9()
+
+define void @external_external_1(i32 *%a) unnamed_addr {
+  call void @dummy1()
+  ret void
+}
+define void @external_external_2(float *%a) unnamed_addr {
+  call void @dummy1()
+  ret void
+}
+
+define void @external_internal_1(i32 *%a) unnamed_addr {
+  call void @dummy2()
+  ret void
+}
+define internal void @external_internal_2(float *%a) unnamed_addr {
+  call void @dummy2()
+  ret void
+}
+
+define internal void @internal_external_1(i32 *%a) unnamed_addr {
+  call void @dummy3()
+  ret void
+}
+define void @internal_external_2(float *%a) unnamed_addr {
+  call void @dummy3()
+  ret void
+}
+
+define weak void @weak_weak_1(i32 *%a) unnamed_addr {
+  call void @dummy4()
+  ret void
+}
+define weak void @weak_weak_2(float *%a) unnamed_addr {
+  call void @dummy4()
+  ret void
+}
+
+define weak void @weak_external_1(i32 *%a) unnamed_addr {
+  call void @dummy5()
+  ret void
+}
+define external void @weak_external_2(float *%a) unnamed_addr {
+  call void @dummy5()
+  ret void
+}
+
+define external void @external_weak_1(i32 *%a) unnamed_addr {
+  call void @dummy6()
+  ret void
+}
+define weak void @external_weak_2(float *%a) unnamed_addr {
+  call void @dummy6()
+  ret void
+}
+
+define weak void @weak_internal_1(i32 *%a) unnamed_addr {
+  call void @dummy7()
+  ret void
+}
+define internal void @weak_internal_2(float *%a) unnamed_addr {
+  call void @dummy7()
+  ret void
+}
+
+define internal void @internal_weak_1(i32 *%a) unnamed_addr {
+  call void @dummy8()
+  ret void
+}
+define weak void @internal_weak_2(float *%a) unnamed_addr {
+  call void @dummy8()
+  ret void
+}
+
+define internal void @internal_internal_1(i32 *%a) unnamed_addr {
+  call void @dummy9()
+  ret void
+}
+define internal void @internal_internal_2(float *%a) unnamed_addr {
+  call void @dummy9()
+  ret void
+}