[PATCH] D18443: [Verifier] Reject PHIs using definitions from own block.

[Sanjoy Das via llvm-commits]

sanjoy added inline comments.

================
Comment at: lib/IR/Verifier.cpp:3401
@@ -3400,3 +3400,3 @@
   const Use &U = I.getOperandUse(i);
-  Assert(InstsInThisBlock.count(Op) || DT.dominates(Op, U),
-         "Instruction does not dominate all uses!", Op, &I);
+  if (isa<PHINode>(I))
+    Assert(DT.dominates(Op, U),
----------------
I'd phrase this check a little differently, as:

```
bool FastDominanceCheck /* perhaps use a better name here */ = !isa<PHINode> && InstsInThisBlock.count(Op)
if (!FastDominanceCheck)
  Assert(DT.dominates(...));
```

to make it explicit that the lookup in `InstsInThisBlock` is a compile-time optimization.

================
Comment at: test/Transforms/LoopVectorize/phi-hang.ll:3
@@ -2,3 +2,1 @@
 
-; PR15384
-define void @test1(i32 %arg) {
----------------
Was this test checking that loop vectorizer can deal with non-strict PHI nodes, or something else?  If the latter, perhaps we should rewrite this test to check that?


Repository:
  rL LLVM

http://reviews.llvm.org/D18443