[llvm-diff] Implement diff of PHI nodes

[void]

Summary

Implement diff of PHI nodes

[dblaikie]

llvm/test/tools/llvm-diff/phinode.ll

2

I'm not super faimiliar with llvm-diff - but I'm guessing this test passes before/after the change, so isn't testing the new functionality, is it? I guess some negative test cases should be tested instead to demonstrate that more parts of the IR are being tested?

[void]

llvm/test/tools/llvm-diff/phinode.ll

2

I was a bit dumbfounded that this tool didn't have "negative" tests (except for the callbr for some reason). This test is almost entirely a smoke test—can it run without barfing on a PHI node? I'll see about creating a negative test.

[void]

Improve testcase to show an actual difference.

[dblaikie]

Looks good - optional test change.

llvm/test/tools/llvm-diff/phinode.ll

2

To avoid all the escaping which maybe obscures the purpose of the replacement, maybe this could be s/ 0,/ 1,/ - yeah, means it /could/ match other parts of the IR, but it doesn't currently at least.

llvm/test/tools/llvm-diff/phinode.ll

+; RUN: rm -f %t.ll
+; RUN: cat %s | sed -e 's/%7 = phi i32 \[ 0, %2 \], \[ -1, %1 \]/%7 = phi i32 [ 1, %2 ], [ -1, %1 ]/' > %t.ll
+; RUN: not llvm-diff %s %t.ll 2>&1 | FileCheck %s
+
+; CHECK:       in function foo:
+; CHECK-NEXT:   in block %6 / %6:
+; CHECK-NEXT:    >   %7 = phi i32 [ 1, %2 ], [ -1, %1 ]
+; CHECK-NEXT:    >   ret i32 %7
+; CHECK-NEXT:    <   %7 = phi i32 [ 0, %2 ], [ -1, %1 ]
+; CHECK-NEXT:    <   ret i32 %7
+define i32 @foo(i32 %0) #0 {
+  callbr void asm sideeffect "", "X,~{dirflag},~{fpsr},~{flags}"(i8* blockaddress(@foo, %6))
+          to label %2 [label %6]
+
+2:
+  %3 = icmp eq i32 %0, 0
+  br i1 %3, label %6, label %4
+
+4:
+  br label %5
+
+5:
+  br label %5
+
+6:
+  %7 = phi i32 [ 0, %2 ], [ -1, %1 ]
+  ret i32 %7
+}

llvm/tools/llvm-diff/lib/DifferenceEngine.cpp

if (isa<CmpInst>(L)) {
       if (cast<CmpInst>(L)->getPredicate()
             != cast<CmpInst>(R)->getPredicate()) {
         if (Complain) Engine.log("different predicates");
         return true;
       }
     } else if (isa<CallInst>(L)) {
       return diffCallSites(cast<CallInst>(*L), cast<CallInst>(*R), Complain);
     } else if (isa<PHINode>(L)) {
-      // FIXME: implement.
+      const PHINode &LI = cast<PHINode>(*L);
+      const PHINode &RI = cast<PHINode>(*R);
 
       // This is really weird;  type uniquing is broken?
-      if (L->getType() != R->getType()) {
-        if (!L->getType()->isPointerTy() || !R->getType()->isPointerTy()) {
+      if (LI.getType() != RI.getType()) {
+        if (!LI.getType()->isPointerTy() || !RI.getType()->isPointerTy()) {
           if (Complain) Engine.log("different phi types");
           return true;
         }
       }
+
+      if (LI.getNumIncomingValues() != RI.getNumIncomingValues()) {
+        if (Complain)
+          Engine.log("PHI node # of incoming values differ");
+        return true;
+      }
+
+      for (unsigned I = 0; I < LI.getNumIncomingValues(); ++I) {
+        if (TryUnify)
+          tryUnify(LI.getIncomingBlock(I), RI.getIncomingBlock(I));
+
+        if (!equivalentAsOperands(LI.getIncomingValue(I),
+                                  RI.getIncomingValue(I))) {
+          if (Complain)
+            Engine.log("PHI node incoming values differ");
+          return true;
+        }
+      }
+
       return false;
 
     // Terminators.
     } else if (isa<InvokeInst>(L)) {
       const InvokeInst &LI = cast<InvokeInst>(*L);
       const InvokeInst &RI = cast<InvokeInst>(*R);
       if (diffCallSites(LI, RI, Complain))
         return true;