Functions with differing phis should not be merged.

[rudkx]

Summary

Check that the incoming blocks of phi nodes are identical, and block function merging if they are not.

rdar://problem/26255167

[aschwaighofer]

LGTM

[qcolombet]

test/Transforms/MergeFunc/phi-check-blocks.ll

2 ↗

(On Diff #57900)

Instead of relying on the debug output I guess it would be possible to check that we still have both functions.
I.e., I would:

• Get rid of the REQUIRES line
• Use FileCheck instead of grep

[rudkx]

test/Transforms/MergeFunc/phi-check-blocks.ll

2 ↗

(On Diff #57900)

I had originally written the test that way but then noticed that some of the other tests were doing this.

I'll go ahead and change it back.

[rudkx]

Update the test to ensure both functions exist rather that grepping the output of -stats.

[qcolombet]

Hi Mark,

Yeah, we used to be less strict about the use of FileCheck :).
Thanks for the updated diff.

LGTM.

Cheers,
-Quentin

llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp

if (int Res = cmpNumbers(RMWI->isVolatile(),
                              cast<AtomicRMWInst>(R)->isVolatile()))
       return Res;
     if (int Res = cmpOrderings(RMWI->getOrdering(),
                              cast<AtomicRMWInst>(R)->getOrdering()))
       return Res;
     return cmpNumbers(RMWI->getSynchScope(),
                       cast<AtomicRMWInst>(R)->getSynchScope());
   }
+  if (const PHINode *PNL = dyn_cast<PHINode>(L)) {
+    const PHINode *PNR = cast<PHINode>(R);
+    // Ensure that in addition to the incoming values being identical
+    // (checked by the caller of this function), the incoming blocks
+    // are also identical.
+    for (unsigned i = 0, e = PNL->getNumIncomingValues(); i != e; ++i) {
+      if (int Res =
+              cmpValues(PNL->getIncomingBlock(i), PNR->getIncomingBlock(i)))
+        return Res;
+    }
+  }
   return 0;
 }
 
 // Determine whether two GEP operations perform the same underlying arithmetic.
 // Read method declaration comments for more details.
 int FunctionComparator::cmpGEPs(const GEPOperator *GEPL,
                                 const GEPOperator *GEPR) const {
 

llvm/trunk/test/Transforms/MergeFunc/phi-check-blocks.ll

+; RUN: opt -S -mergefunc < %s | FileCheck %s
+
+; Ensure that we do not merge functions that are identical with the
+; exception of the order of the incoming blocks to a phi.
+
+; CHECK-LABEL: define linkonce_odr hidden i1 @first(i2)
+define linkonce_odr hidden i1 @first(i2) {
+entry:
+; CHECK: switch i2
+  switch i2 %0, label %default [
+    i2 0, label %L1
+    i2 1, label %L2
+    i2 -2, label %L3
+  ]
+default:
+  unreachable
+L1:
+  br label %done
+L2:
+  br label %done
+L3:
+  br label %done
+done:
+  %result = phi i1 [ true, %L1 ], [ false, %L2 ], [ false, %L3 ]
+; CHECK: ret i1
+  ret i1 %result
+}
+
+; CHECK-LABEL: define linkonce_odr hidden i1 @second(i2)
+define linkonce_odr hidden i1 @second(i2) {
+entry:
+; CHECK: switch i2
+  switch i2 %0, label %default [
+    i2 0, label %L1
+    i2 1, label %L2
+    i2 -2, label %L3
+  ]
+default:
+  unreachable
+L1:
+  br label %done
+L2:
+  br label %done
+L3:
+  br label %done
+done:
+  %result = phi i1 [ true, %L3 ], [ false, %L2 ], [ false, %L1 ]
+; CHECK: ret i1
+  ret i1 %result
+}