[PATCH] D51431: [WIP][IPSCCP] Add lattice value for != constant and propagate nonnull.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Transforms/Scalar/SCCP.cpp:1785
+      IPNumNonNullAdded++;
+      CS.addParamAttr(ArgNo, Attribute::NonNull);
+    }
----------------
efriedma wrote:
> fhahn wrote:
> > efriedma wrote:
> > > I'm not sure this is right: nonnull is UB if the value is null, but SCCP can merge an "Undef" into a NotConstant.
> > I think I am missing something. tryToAddNonNull is used after we finished solving and a lattice value of notconstant(Null) should mean "we proved that the value is != null".  If we  merge an undef into a NotConstant, isn't that similar to when we merge an undef into a Constant, where it is safe to use the constant after solving?
> The difference is how we're using the result.  If we take a value that's undef and replace it with a constant, that's fine; the undef might have resolved to that constant anyway.  If we take a value that's undef and assert it's nonnull, that will explode if the undef actually resolves to null.
> 
> Note this is specifically a problem with our weird handling of IR "undef" values; if we treated "undef" as overdefined or constant, the lattice represents what you want it to represent.  It's possible that nonnull optimization is important enough that it's better to just kill off all the undef special-case optimizations to make this legal.
Right, thanks Eli! I'll see if it is feasible to go to overdefined when merging undef into notconstan to start with.


https://reviews.llvm.org/D51431