[PATCH] D10903: [LICM] Replace incoming values with undef if the incoming BB is unreachable

[David Majnemer]

majnemer added inline comments.

================
Comment at: lib/Transforms/Scalar/LICM.cpp:614-621
@@ -612,1 +613,10 @@
 
+    // Replace incoming values with undef if the incoming BB is unreachable.
+    Use &U = UI.getUse();
+    BasicBlock *BB = PN->getIncomingBlock(U);
+    if (!DT->isReachableFromEntry(BB)) {
+      // Replace the use with an undef value.
+      U = UndefValue::get(I.getType());
+      continue;
+    }
+
----------------
chandlerc wrote:
> Why do this here rather than in InstSimplify? Do we form these too late for that to clean it up? If so, should we add it to InstSimplify and call that here?
I don't know what we could do in InstructionSimplify instead of here.

Running InstructionSimplify on the PHI won't help because it cannot create new instructions, it wouldn't be able to give us a new PHI which had the incoming value replaced with undef.

Running InstructionSimplify on the incoming value won't help because it might not be simplifiable.  My trivial test case has a trivial incoming value but we cannot, in the general case, rely on simplification yielding a `Constant`.  For example:
  define i32 @f(i1 zeroext %p1) {
  entry:
    br label %lbl

  lbl.loopexit:                                     ; No predecessors!
    br label %lbl

  lbl:                                              ; preds = %lbl.loopexit, %entry
    %phi = phi i32 [ %conv, %lbl.loopexit ], [ undef, %entry ]
    br i1 %p1, label %if.then.5, label %out

  if.then.5:                                        ; preds = %if.then.5, %lbl
    %conv = zext i1 %p1 to i32
    br label %if.then.5

  out:                                              ; preds = %lbl
    ret i32 %phi
  }


http://reviews.llvm.org/D10903