Merge pull request #8705 from eeckstein/fix-merge-funcs

MergeFunctions: now really handle self recursions correctly.

Author: eeckstein

lib/LLVMPasses/LLVMMergeFunctions.cpp

@@ -782,31 +782,20 @@ bool SwiftMergeFunctions::deriveParams(ParamInfos &Params,
 
 /// Returns true if the \p OpIdx's constant operand in the current instruction
 /// does differ in any of the functions in \p FInfos.
-///
-/// But it accepts the case where all operands refer to their containing
-/// functions (in case of self recursive functions).
 bool SwiftMergeFunctions::constsDiffer(const FunctionInfos &FInfos,
                                        unsigned OpIdx) {
   Constant *CommonConst = nullptr;
-  bool matching = true;
-  bool selfReferencing = true;
 
   for (const FunctionInfo &FI : FInfos) {
     Value *Op = FI.CurrentInst->getOperand(OpIdx);
     if (Constant *C = dyn_cast<Constant>(Op)) {
-      if (C != FI.F)
-        selfReferencing = false;
-
       if (!CommonConst) {
         CommonConst = C;
       } else if (C != CommonConst) {
-        matching = false;
-      }
-      if (!selfReferencing && !matching)
         return true;
+      }
     }
   }
-  assert(selfReferencing || matching);
   return false;
 }
 
@@ -895,18 +884,29 @@ void SwiftMergeFunctions::mergeWithParams(const FunctionInfos &FInfos,
     OrigArg.replaceAllUsesWith(&NewArg);
   }
 
+  SmallPtrSet<Function *, 8> SelfReferencingFunctions;
+
   // Replace all differing operands with a parameter.
   for (const ParamInfo &PI : Params) {
     Argument *NewArg = &*NewArgIter++;
     for (const OpLocation &OL : PI.Uses) {
       OL.I->setOperand(OL.OpIndex, NewArg);
     }
     ParamTypes.push_back(PI.Values[0]->getType());
+
+    // Collect all functions which are referenced by any parameter.
+    for (Value *V : PI.Values) {
+      if (Function *F = dyn_cast<Function>(V))
+        SelfReferencingFunctions.insert(F);
+    }
   }
 
   for (unsigned FIdx = 0, NumFuncs = FInfos.size(); FIdx < NumFuncs; ++FIdx) {
     Function *OrigFunc = FInfos[FIdx].F;
-    if (replaceDirectCallers(OrigFunc, NewFunction, Params, FIdx)) {
+    // Don't try to replace all callers of functions which are used as
+    // parameters because we must not delete such functions.
+    if (SelfReferencingFunctions.count(OrigFunc) == 0 &&
+        replaceDirectCallers(OrigFunc, NewFunction, Params, FIdx)) {
       // We could replace all uses (and the function is not externally visible),
       // so we can delete the original function.
       auto Iter = FuncEntries.find(OrigFunc);
@@ -1075,12 +1075,8 @@ bool SwiftMergeFunctions::replaceDirectCallers(Function *Old, Function *New,
     for (const ParamInfo &PI : Params) {
       assert(ParamIdx < NewFuncTy->getNumParams());
       Constant *ArgValue = PI.Values[FuncIdx];
-
-      // Check if it's a self referencing function. We must not use the old
-      // function pointer as argument.
-      if (ArgValue == Old)
-        ArgValue = New;
-
+      assert(ArgValue != Old &&
+        "should not try to replace all callers of self referencing functions");
       NewArgs.push_back(ArgValue);
       OldParamTypes.push_back(ArgValue->getType());
       ++ParamIdx;

test/LLVMPasses/merge_func.ll

@@ -326,63 +326,79 @@ done:
 
 ; Check self recursive functions
 
-; CHECK-NOT: @recursive1(
+; CHECK-LABEL: define internal void @recursive1(i32 %x, i32 %y)
+; CHECK: tail call void @recursive1_merged(i32 %x, i32 %y, i32* @g1, void (i32, i32)* @recursive1)
+; CHECK: ret void
 define internal void @recursive1(i32 %x, i32 %y) {
   br i1 undef, label %bb1, label %bb2
 
 bb1:
+  %l = load i32, i32* @g1, align 4
   call void @recursive1(i32 %x, i32 %y)
   br label %bb2
 
 bb2:
   ret void
 }
 
-; CHECK-LABEL: define internal void @recursive1_merged(i32, i32)
-; CHECK: call void @recursive1_merged(i32 %0, i32 %1)
+; CHECK-LABEL: define internal void @recursive2(i32 %x, i32 %y)
+; CHECK: tail call void @recursive1_merged(i32 %x, i32 %y, i32* @g2, void (i32, i32)* @recursive2)
 ; CHECK: ret void
 define internal void @recursive2(i32 %x, i32 %y) {
   br i1 undef, label %bb1, label %bb2
 
 bb1:
+  %l = load i32, i32* @g2, align 4
   call void @recursive2(i32 %x, i32 %y)
   br label %bb2
 
 bb2:
   ret void
 }
+; CHECK-LABEL: define internal void @recursive1_merged(i32, i32, i32*, void (i32, i32)*)
+; CHECK: load i32, i32* %2
+; CHECK: call void %3(i32 %0, i32 %1)
+; CHECK: ret void
+
 
-; CHECK-LABEL: define internal void @another_recursive_func_merged(i32, void (i32)*)
-; CHECK: call void %1(i32 %0)
+; CHECK-LABEL: define internal void @another_recursive_func(i32 %x)
+; CHECK: tail call void @another_recursive_func_merged(i32 %x, i32* @g1, void (i32)* @another_recursive_func)
 ; CHECK: ret void
 define internal void @another_recursive_func(i32 %x) {
   br i1 undef, label %bb1, label %bb2
 
 bb1:
+  store i32 %x, i32 *@g1, align 4
   call void @another_recursive_func(i32 %x)
   br label %bb2
 
 bb2:
   ret void
 }
-
 ; CHECK-NOT: @not_really_recursive(
+
+; CHECK-LABEL: define internal void @another_recursive_func_merged(i32, i32*, void (i32)*)
+; CHECK: store i32 %0, i32* %1
+; CHECK: call void %2(i32 %0)
+; CHECK: ret void
 define internal void @not_really_recursive(i32 %x) {
   br i1 undef, label %bb1, label %bb2
 
 bb1:
+  store i32 %x, i32 *@g2, align 4
   call void @callee1(i32 %x)
   br label %bb2
 
 bb2:
   ret void
 }
+; CHECK-NOT: @not_really_recursive(
 
 ; CHECK-LABEL: define void @call_recursive_funcs(i32 %x)
-; CHECK: call void @recursive1_merged(i32 %x, i32 %x)
-; CHECK: call void @recursive1_merged(i32 %x, i32 %x)
-; CHECK: call void bitcast (void (i32, void (i32)*)* @another_recursive_func_merged to void (i32, void (i32, void (i32)*)*)*)(i32 %x, void (i32, void (i32)*)* @another_recursive_func_merged)
-; CHECK: call void @another_recursive_func_merged(i32 %x, void (i32)* @callee1)
+; CHECK: call void @recursive1(i32 %x, i32 %x)
+; CHECK: call void @recursive2(i32 %x, i32 %x)
+; CHECK: call void @another_recursive_func(i32 %x)
+; CHECK: call void @another_recursive_func_merged(i32 %x, i32* @g2, void (i32)* @callee1)
 ; CHECK: ret void
 define void @call_recursive_funcs(i32 %x) {
   call void @recursive1(i32 %x, i32 %x)