[PATCH] D152068: [InstCombine] add overflow checking on AddSub `C-(X+C2) --> (C-C2)-X`

[Kohei Asano via Phabricator via llvm-commits]

khei4 added a comment.

Thank you for the review!

> I don't think this proof is correct, because the sub nsw i8 %c1, %c2 will already be poison on overflow,

Seems so! The original assumption seemed to allow all c1 and c2. Thanks!



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2004
+      Res->setHasNoSignedWrap(I.hasNoSignedWrap() &&
+                              OBO1->hasNoUnsignedWrap() &&
+                              willNotOverflowSignedSub(C, C2, *Res));
----------------
nikic wrote:
> hasNoUnsignedWrap -> hasNoSignedWrap
Ah... Thanks!


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2005
+                              OBO1->hasNoUnsignedWrap() &&
+                              willNotOverflowSignedSub(C, C2, *Res));
+      return Res;
----------------
nikic wrote:
> `*Res` should be `I` here.
Seems reasonable!


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

https://reviews.llvm.org/D152068