[PATCH] D70749: [InstCombine] do not insert nonnull assumption for undef

[Bob Haarman via Phabricator via llvm-commits]

inglorion added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/assume-replacing-call.ll:199
+entry:
+  %foo = call i64 @func_test_nonnull(i8* undef) #2
+  br i1 %c, label %ctrue, label %cend
----------------
efriedma wrote:
> Isn't this already UB?  From LangRef: "if the parameter [...] is null, the behavior is undefined".
That's basically how the optimization works. Behavior is undefined if the parameter is null, therefore we can assume it is not null, which unlock optimizations.

The problem is that we would also do this if the parameter is undef. It may not have been undef in the original program. In the example I was looking at, deadargelim replaced an unused (but declared non-null) parameter with undef, instcombine sank the call and generated a non-null assumption for the parameter (which became call void @llvm.assume(i1 undef)), and simplifycfg then eliminated the entire branch that code was on, leading to different behavior from the original program.

We could presumably fix this in other ways, but my thinking is that not generating assumptions for undef values is a sensible thing to do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70749/new/

https://reviews.llvm.org/D70749