[PATCH] D27114: Preserve nonnull metadata on Loads through SROA & mem2reg.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: lib/Transforms/Utils/PromoteMemoryToRegister.cpp:392
+    // We're replacing this Load with another Load, so let's preserve metadata
+    if (isa<LoadInst>(ReplVal)) {
+      SmallVector<std::pair<unsigned, MDNode *>, 8> MD;
----------------
luqmana wrote:
> efriedma wrote:
> > This isn't a legal transform: you're assigning "nonnull" to a completely unrelated instruction (which could have other users) just because it happens to be a LoadInst.  I guess you could get away with this if you can prove there aren't any other uses of the value, though.
> Hmmm, I could just add a check for it being the only use but is it illegal? Just walking through this code:
> 
> - We have an alloca with a single store (OnlyStore)
> - The value stored was from a Load (ReplVal)
> - Now, we have a Load (LI) from this alloca that's dominated by this store and it has !nonnull.
> 
> Adding !nonnull to ReplVal seems reasonable since (assuming) the !nonnull on LI was valid, we know that value must be non null. So, can we not just propagate it back to ReplVal?
> (assuming) the !nonnull on LI was valid, we know that value must be non null

!nonnull basically means "if this load would return null, we can treat it as poison".  So you can't propagate backwards unless you can prove that the load will actually execute (LI post-dominates ReplVal), and that the loaded value will be used in a way that causes undefined behavior (a branch or call which uses LI post-dominates ReplVal).  In practice, this is essentially impossible to check in LLVM.


https://reviews.llvm.org/D27114