[Verifier] Reject PHIs using defs from own block.

Reject the following IR as malformed (assuming that %entry, %next are
not in a loop):

    next:
      %y = phi i32 [ 0, %entry ]
      %x = phi i32 [ %y, %entry ]

Such PHI nodes came up in PR26718. While there was no consensus on
whether or not this is valid IR, most opinions on that bug and in a
discussion on the llvm-dev mailing list tended towards a
"strict interpretation" (term by Joseph Tremoulet) of PHI node uses.
Also, the language reference explicitly states that "the use of each
incoming value is deemed to occur on the edge from the corresponding
predecessor block to the current block" and
`DominatorTree::dominates(Instruction*, Use&)` uses this definition as
well.

For the code mentioned in PR15384, clang does not compile to such PHIs
(anymore?). The test case still hangs when replacing `%tmp6` with `%tmp`
in revisions before r176366 (where PR15384 has been fixed). The
occurrence of %tmp6 therefore was probably unintentional. Its value is
not used except in other PHIs.

Reviewers: majnemer, reames, JosephTremoulet, bkramer, grosser, jdoerfert, kparzysz, sanjoy

Differential Revision: http://reviews.llvm.org/D18443

llvm-svn: 264528

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 9d9fe7d..0cd2463 100644 (file)
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -3397,8 +3397,14 @@ void Verifier::verifyDominatesUse(Instruction &I, unsigned i) {
       return;
   }
 
+  // Quick check whether the def has already been encountered in the same block.
+  // PHI nodes are not checked to prevent accepting preceeding PHIs, because PHI
+  // uses are defined to happen on the incoming edge, not at the instruction.
+  if (!isa<PHINode>(I) && InstsInThisBlock.count(Op))
+    return;
+
   const Use &U = I.getOperandUse(i);
-  Assert(InstsInThisBlock.count(Op) || DT.dominates(Op, U),
+  Assert(DT.dominates(Op, U),
          "Instruction does not dominate all uses!", Op, &I);
 }
 

diff --git a/llvm/test/Transforms/LoopVectorize/phi-hang.ll b/llvm/test/Transforms/LoopVectorize/phi-hang.ll
index bbce239..eb1aaef 100644 (file)
--- a/llvm/test/Transforms/LoopVectorize/phi-hang.ll
+++ b/llvm/test/Transforms/LoopVectorize/phi-hang.ll
@@ -18,7 +18,7 @@ bb4:                                              ; preds = %bb3
 
 bb5:                                              ; preds = %bb4, %bb1
   %tmp6 = phi i32 [ 0, %bb4 ], [ %tmp, %bb1 ]
-  %tmp7 = phi i32 [ 0, %bb4 ], [ %tmp6, %bb1 ]
+  %tmp7 = phi i32 [ 0, %bb4 ], [ %tmp, %bb1 ]
   %tmp8 = phi i32 [ 0, %bb4 ], [ %tmp, %bb1 ]
   %tmp9 = add nsw i32 %tmp2, 1
   %tmp10 = icmp eq i32 %tmp9, 0

diff --git a/llvm/test/Verifier/dominates.ll b/llvm/test/Verifier/dominates.ll
index c5424bb..6b0c67f 100644 (file)
--- a/llvm/test/Verifier/dominates.ll
+++ b/llvm/test/Verifier/dominates.ll
@@ -55,3 +55,16 @@ bb1:
 ; CHECK-NEXT:  %y1 = add i32 %x, 1
 ; CHECK-NEXT:  %y3 = phi i32 [ %y1, %bb0 ]
 }
+
+define void @f5() {
+entry:
+  br label %next
+
+next:
+  %y = phi i32 [ 0, %entry ]
+  %x = phi i32 [ %y, %entry ]
+  ret void
+; CHECK: Instruction does not dominate all uses!
+; CHECK-NEXT:  %y = phi i32 [ 0, %entry ]
+; CHECK-NEXT:  %x = phi i32 [ %y, %entry ]
+}