[PATCH] D75845: [ValueLattice] Add singlecrfromundef lattice value.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ValueLattice.h:31
     /// state in our local meet rules.  In this usage, it's taken to mean
     /// "nothing known yet".
     unknown,
----------------
fhahn wrote:
> efriedma wrote:
> > I think it would make sense to list out the possible state transitions for each state here.
> Done! Is the format along the lines you imagined? It does not list explicitly that we can stay in the existing state, if nothing changes.
Yes, this format looks fine.

For unknown specifically, probably you could just say "transition allowed to any state".


================
Comment at: llvm/include/llvm/Analysis/ValueLattice.h:47
     /// constantexprs can appear here.)
     notconstant,
 
----------------
fhahn wrote:
> efriedma wrote:
> > Sort of a side-question while I'm here, but how are we inferring notconstant?  Is the inference safe in the presence of undef?
> I've responded in D75120
I guess my concern is really, does "notconstant" imply that we've proven the value isn't undef?  It would probably be worth stating that explicitly.


================
Comment at: llvm/include/llvm/Analysis/ValueLattice.h:44
+    /// Transition allowed to the following states:
+    ///  constant
+    ///  singlecrfromundef
----------------
Is there actually a transition from undef -> constant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75845