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

[Luqman Aden via Phabricator via llvm-commits]

luqmana 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;
----------------
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?


https://reviews.llvm.org/D27114