[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:45
+; CHECK:       bb1:
+; CHECK-NEXT:    ret i8* [[V1]]
+; CHECK:       bb2:
----------------
nikic wrote:
> nlopes wrote:
> > This doesn't seem correct to be in the model where load !nonnull is not UB. Passing poison as a function argument is not UB. The function might not even use the argument, for example.
> > Anyway, we need to define the semantics !nonnull before moving on, otherwise we can't tell if this is correct or not.
> > 
> This is replacing a !nonnull load with an ordinary load, so this shouldn't be exploiting UB. As far as I can see this test corresponds to the existing behavior, it only ensures that the !nonnull in bb1 is not incorrectly propagated to the load in top.
> This doesn't seem correct to be in the model where load !nonnull is not UB. Passing poison as a function argument is not UB. The function might not even use the argument, for example.

I think I am missing something there. `@use1` is readonly, so the loaded value should not change? `@test7` is a similar test case where `@use2` might change the loaded value.


https://reviews.llvm.org/D47339