[PATCH] D59071: [Transform] Improve saddo with mixed signs

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2060
+      if (!Overflow) {
+        if (C0->isNegative() == C1->isNegative() || C0->abs().slt(C1->abs())) {
+          return replaceInstUsesWith(
----------------
The use of abs here makes me uncomfortable... abs(INT_MIN) is INT_MIN again. So if we take `saddo(X +nsw INT_MIN, 1)`, then the check here is `abs(INT_MIN) s< abs(1)` which is `INT_MIN s< 1`, which is true. While this will ultimately give a conservatively correct result, you'd prefer this case to take the other branch.


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

https://reviews.llvm.org/D59071