[PATCH] D102966: [CVP] Guard against poison in common phji value transform (PR50399)

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM - need to fix the miscompile regardless of whether we could salvage the existing test.



================
Comment at: llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll:142
 entry:
   %sub = sub nsw i32 0, %arg
   %cmp = icmp eq i32 %arg, -2147483648
----------------
nikic wrote:
> spatel wrote:
> > Check my understanding (add this test?): if the `sub` does not have `nsw`, then we still allow this transform because the plain `sub` can not create poison and `arg` is not poison?
> Yes, that's right. I've added the additional test in https://github.com/llvm/llvm-project/commit/e42636d3c1a41a9b7c5d8095ae5ef6682e26d4a2 and it does still fold for the reason you described.
Thanks. So it's one of those awkward cases where improving wrapping analysis could actually inhibit an optimization. I don't know if we have any plans for adjusting for that sort of thing, but might be worth a code or test comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102966