Merge pull request #18622 from eeckstein/fix-func-merge-4.2

[4.2] Fix a miscompile in the swift function merging pass.

Author: eeckstein

lib/LLVMPasses/LLVMMergeFunctions.cpp

@@ -402,6 +402,8 @@ class SwiftMergeFunctions : public ModulePass {
 
   bool deriveParams(ParamInfos &Params, FunctionInfos &FInfos);
 
+  bool numOperandsDiffer(FunctionInfos &FInfos);
+
   bool constsDiffer(const FunctionInfos &FInfos, unsigned OpIdx);
 
   bool tryMapToParameter(FunctionInfos &FInfos, unsigned OpIdx,
@@ -762,6 +764,20 @@ bool SwiftMergeFunctions::deriveParams(ParamInfos &Params,
   // Iterate over all instructions synchronously in all functions.
   do {
     if (isEligibleForConstantSharing(FirstFI.CurrentInst)) {
+
+      // Here we handle a rare corner case which needs to be explained:
+      // Usually the number of operands match, because otherwise the functions
+      // in FInfos would not be in the same equivalence class. There is only one
+      // exception to that: If the current instruction is a call to a function,
+      // which was merged in the previous iteration (in tryMergeEquivalenceClass)
+      // then the call could be replaced and has more arguments than the
+      // original call.
+      if (numOperandsDiffer(FInfos)) {
+        assert(isa<CallInst>(FirstFI.CurrentInst) &&
+               "only calls are expected to differ in number of operands");
+        return false;
+      }
+
       for (unsigned OpIdx = 0, NumOps = FirstFI.CurrentInst->getNumOperands();
            OpIdx != NumOps; ++OpIdx) {
 
@@ -783,6 +799,16 @@ bool SwiftMergeFunctions::deriveParams(ParamInfos &Params,
   return true;
 }
 
+/// Returns true if the number of operands of the current instruction differs.
+bool SwiftMergeFunctions::numOperandsDiffer(FunctionInfos &FInfos) {
+  unsigned numOps = FInfos[0].CurrentInst->getNumOperands();
+  for (const FunctionInfo &FI : ArrayRef<FunctionInfo>(FInfos).drop_front(1)) {
+    if (FI.CurrentInst->getNumOperands() != numOps)
+      return true;
+  }
+  return false;
+}
+
 /// Returns true if the \p OpIdx's constant operand in the current instruction
 /// does differ in any of the functions in \p FInfos.
 bool SwiftMergeFunctions::constsDiffer(const FunctionInfos &FInfos,

test/LLVMPasses/merge_func.ll

@@ -200,6 +200,100 @@ define i32 @func4_merged_with2(i32 %x) {
 }
 
 
+; The same example as above, but we cannot merge func2 with func4, because
+; func4 calls func1 (which is merged with func2 in the first iteration).
+
+declare i32 @get_int(i32 %x)
+
+; CHECK-LABEL: define i32 @Function1_merged_with_3(i32 %x)
+; CHECK: %1 = tail call i32 @Function1_merged_with_3Tm(i32 %x, i32* @g1)
+; CHECK: ret i32 %1
+define i32 @Function1_merged_with_3(i32 %x) {
+  %l1 = load i32, i32* @g1, align 4
+  %sum = add i32 %x, %l1
+  %l2 = load i32, i32* @g2, align 4
+  %sum2 = add i32 %sum, %l2
+  %l3 = load i32, i32* @g3, align 4
+  %sum3 = add i32 %sum2, %l2
+  %l4 = load i32, i32* @g4, align 4
+  %sum4 = add i32 %sum3, %l2
+  %l5 = load i32, i32* @g5, align 4
+  %sum5 = add i32 %sum4, %l2
+  %c = call fastcc i32 @get_int(i32 %sum5)
+  ret i32 %c
+}
+
+; CHECK-LABEL: define i32 @Function2_not_merged(i32 %x)
+; CHECK: load
+; CHECK: load
+; CHECK: load
+; CHECK: load
+; CHECK: %c = call fastcc i32 @get_int
+; CHECK: ret i32 %c
+define i32 @Function2_not_merged(i32 %x) {
+  %l1 = load i32, i32* @g2, align 4
+  %sum = add i32 %x, %l1
+  %l2 = load i32, i32* @g3, align 4
+  %sum2 = add i32 %sum, %l2
+  %l3 = load i32, i32* @g4, align 4
+  %sum3 = add i32 %sum2, %l2
+  %l4 = load i32, i32* @g5, align 4
+  %sum4 = add i32 %sum3, %l2
+  %l5 = load i32, i32* @g1, align 4
+  %sum5 = add i32 %sum4, %l2
+  %c = call fastcc i32 @get_int(i32 %sum5)
+  ret i32 %c
+}
+
+; CHECK-LABEL: define i32 @Function3_merged_with_1(i32 %x)
+; CHECK: %1 = tail call i32 @Function1_merged_with_3Tm(i32 %x, i32* @g2)
+; CHECK: ret i32 %1
+define i32 @Function3_merged_with_1(i32 %x) {
+  %l1 = load i32, i32* @g2, align 4
+  %sum = add i32 %x, %l1
+  %l2 = load i32, i32* @g2, align 4
+  %sum2 = add i32 %sum, %l2
+  %l3 = load i32, i32* @g3, align 4
+  %sum3 = add i32 %sum2, %l2
+  %l4 = load i32, i32* @g4, align 4
+  %sum4 = add i32 %sum3, %l2
+  %l5 = load i32, i32* @g5, align 4
+  %sum5 = add i32 %sum4, %l2
+  %c = call fastcc i32 @get_int(i32 %sum5)
+  ret i32 %c
+}
+
+; CHECK-LABEL: define internal i32 @Function1_merged_with_3Tm(i32, i32*)
+; CHECK: load
+; CHECK: load
+; CHECK: load
+; CHECK: load
+; CHECK: %c = call fastcc i32 @get_int
+; CHECK: ret i32 %c
+
+; CHECK-LABEL: define i32 @Function4_not_merged(i32 %x) {
+; CHECK: load
+; CHECK: load
+; CHECK: load
+; CHECK: load
+; CHECK: %1 = call fastcc i32 @Function1_merged_with_3Tm(i32 %sum5, i32* @g1)
+; CHECK: ret i32 %1
+define i32 @Function4_not_merged(i32 %x) {
+  %l1 = load i32, i32* @g1, align 4
+  %sum = add i32 %x, %l1
+  %l2 = load i32, i32* @g3, align 4
+  %sum2 = add i32 %sum, %l2
+  %l3 = load i32, i32* @g4, align 4
+  %sum3 = add i32 %sum2, %l2
+  %l4 = load i32, i32* @g5, align 4
+  %sum4 = add i32 %sum3, %l2
+  %l5 = load i32, i32* @g1, align 4
+  %sum5 = add i32 %sum4, %l2
+  %c = call fastcc i32 @Function1_merged_with_3(i32 %sum5)
+  ret i32 %c
+}
+
+
 ; Test a call chain: caller -> callee1 -> callee2.
 ; Functions should be merged in bottom-up order: callee2, callee1, caller.
 ; Also check that the calling convention is preserved.