Merge pull request #131 from nikic/some-fixes

Misc

Author: alexcrichton

include/llvm/IR/Instruction.h

@@ -586,14 +586,6 @@ class Instruction : public User,
         static_cast<const Instruction *>(this)->getNextNonDebugInstruction());
   }
 
-  /// Return a pointer to the previous non-debug instruction in the same basic
-  /// block as 'this', or nullptr if no such instruction exists.
-  const Instruction *getPrevNonDebugInstruction() const;
-  Instruction *getPrevNonDebugInstruction() {
-    return const_cast<Instruction *>(
-        static_cast<const Instruction *>(this)->getPrevNonDebugInstruction());
-  }
-
   /// Create a copy of 'this' instruction that is identical in all ways except
   /// the following:
   ///   * The instruction has no parent

lib/IR/Instruction.cpp

@@ -602,13 +602,6 @@ const Instruction *Instruction::getNextNonDebugInstruction() const {
   return nullptr;
 }
 
-const Instruction *Instruction::getPrevNonDebugInstruction() const {
-  for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode())
-    if (!isa<DbgInfoIntrinsic>(I))
-      return I;
-  return nullptr;
-}
-
 bool Instruction::isAssociative() const {
   unsigned Opcode = getOpcode();
   if (isAssociative(Opcode))

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;
   }
 }
 

lib/Transforms/Utils/SimplifyCFG.cpp

@@ -1372,14 +1372,6 @@ static bool HoistThenElseCodeToIf(BranchInst *BI,
     }
   }
 
-  // As the parent basic block terminator is a branch instruction which is
-  // removed at the end of the current transformation, use its previous
-  // non-debug instruction, as the reference insertion point, which will
-  // provide the debug location for the instruction being hoisted. For BBs
-  // with only debug instructions, use an empty debug location.
-  Instruction *InsertPt =
-      BIParent->getTerminator()->getPrevNonDebugInstruction();
-
   // Okay, it is safe to hoist the terminator.
   Instruction *NT = I1->clone();
   BIParent->getInstList().insert(BI->getIterator(), NT);
@@ -1389,14 +1381,6 @@ static bool HoistThenElseCodeToIf(BranchInst *BI,
     NT->takeName(I1);
   }
 
-  // The instruction NT being hoisted, is the terminator for the true branch,
-  // with debug location (DILocation) within that branch. We can't retain
-  // its original debug location value, otherwise 'select' instructions that
-  // are created from any PHI nodes, will take its debug location, giving
-  // the impression that those 'select' instructions are in the true branch,
-  // causing incorrect stepping, affecting the debug experience.
-  NT->setDebugLoc(InsertPt ? InsertPt->getDebugLoc() : DebugLoc());
-
   IRBuilder<NoFolder> Builder(NT);
   // Hoisting one of the terminators from our successor is a great thing.
   // Unfortunately, the successors of the if/else blocks may have PHI nodes in

test/CodeGen/X86/pr39187-g.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
+}