[PATCH] D47339: [GVN,NewGVN] Propagate nonnull if K dominates J.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: test/Transforms/NewGVN/metadata-nonnull.ll:98
+; CHECK:       bb1:
+; CHECK-NEXT:    ret i8* [[V1]]
+; CHECK:       bb2:
----------------
nlopes wrote:
> Somehow the line numbers changed. Getting back to my point: here "ret %v2" is replaced with "ret %v1". So it replaces a normal load with a nonnull load.
> This is correct only if the nonnull load is UB. The use by "@use1" call does turn into UB if %v1 is poison. So we need to define if the load is UB or if it returns poison. I don't have any insight to have a preferred option, tough.
> 
Ah right, thanks! So if the nonnull load would be poison, then it would not work, as the rational behind preserving the !nonnull metadata here would be that %v1 is guaranteed to be nonnull, so the later loads of the same location must also be nonnull.

Eli's clarification of the LangRef in D47747 make loading null from a nonnull load UB, which would make this behaviour OK IIUC, assuming it goes in :)


https://reviews.llvm.org/D47339