[llvm] r264528 - [Verifier] Reject PHIs using defs from own block.

[Michael Kruse via llvm-commits]

Author: meinersbur
Date: Sat Mar 26 18:32:57 2016
New Revision: 264528

URL: http://llvm.org/viewvc/llvm-project?rev=264528&view=rev
Log:
[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

Modified:
    llvm/trunk/lib/IR/Verifier.cpp
    llvm/trunk/test/Transforms/LoopVectorize/phi-hang.ll
    llvm/trunk/test/Verifier/dominates.ll

Modified: llvm/trunk/lib/IR/Verifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=264528&r1=264527&r2=264528&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Verifier.cpp (original)
+++ llvm/trunk/lib/IR/Verifier.cpp Sat Mar 26 18:32:57 2016
@@ -3397,8 +3397,14 @@ void Verifier::verifyDominatesUse(Instru
       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);
 }
 

Modified: llvm/trunk/test/Transforms/LoopVectorize/phi-hang.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopVectorize/phi-hang.ll?rev=264528&r1=264527&r2=264528&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/LoopVectorize/phi-hang.ll (original)
+++ llvm/trunk/test/Transforms/LoopVectorize/phi-hang.ll Sat Mar 26 18:32:57 2016
@@ -18,7 +18,7 @@ bb4:
 
 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

Modified: llvm/trunk/test/Verifier/dominates.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Verifier/dominates.ll?rev=264528&r1=264527&r2=264528&view=diff
==============================================================================
--- llvm/trunk/test/Verifier/dominates.ll (original)
+++ llvm/trunk/test/Verifier/dominates.ll Sat Mar 26 18:32:57 2016
@@ -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 ]
+}