[PATCH] D126962: [SCCP] Don't mark edges feasible when resolving undefs

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SCCPSolver.cpp:1450
+/// (e.g. for a phi node with one undef and one constant value, which becomes
+/// feasible later).
 ///
----------------
nikic wrote:
> efriedma wrote:
> > I don't think the example given in the comment is accurate. The lattice distinguishes between unknown and undef, so PHI nodes should propagate correctly.
> > 
> > The problem is other operations: for example, visitCastInst doesn't propagate undef values.  (This is an optimization: if the operand of a cast transitions from undef to a non-undef constant, we don't want to force the result of the cast to overdefined.  We might want to consider revisiting this once we're using poison more consistently.)
> Yes, allowing the lattice value transition from undef -> constant is the point. We do make use of that for phi nodes as well, because we merge undef and C into C, not overdefined.
The undef->constant transition, by itself, doesn't imply we need resolvedUndefsIn, I think.  The way the transition is defined, a "constant" just means "a specific value, or an undef we can force to that value".  Given the way SCCPInstVisitor::visitPHINode works, that actually just works.

The issues come up when you try to mix in other operations; SCCPInstVisitor visitors for arithmetic/casts/etc. with an undef input produce unknown, which is different from what you'd otherwise expect for ValueLattice solving.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126962/new/

https://reviews.llvm.org/D126962