[PATCH] D154565: [InstCombine] Fold icmps comparing uadd_sat with a constant

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3587
+  assert(isa<SaturatingInst>(II) &&
+         "This function can only be called with usub_sat and uadd_sat!");
+
----------------
Directly accept SaturingInst (moving the cast to the caller)?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:3625
+  //    Y = (X < ~C) ?  ((X + C) pred C2) : false
+  // => Y = (X < ~C) && ((X + C) pred C2)
   Value *Op0 = II->getOperand(0);
----------------
Now that makeExactNoWrapRegion is used, I think we can combine both cases to something like this:
```
  // Let Y = add/sub_sat(X, C) pred C2
  //  => Y = (no_overflow ? sat_val : (X binop C)) pred C2
  //  => Y = no_overflow ? (sat_val pred C2) : ((X binop C) pred C2)
  //
  // When (sat_val pred C2) is true, then
  //    Y = no_overflow ? true : ((X binop C) pred C2)
  // => Y = no_overflow || ((X binop C) pred C2)
  // else
  //    Y = no_overflow ? false : ((X binop C) pred C2)
  // => Y = !no_overflow && ((X binop C) pred C2)
```
This is true for all saturating intrinsics and matches the implementation more closely. Extension to the signed case at this point would only involve determining the right `sat_val`.

It would also be possible to replace the last line with:
```
  // => Y = !(no_overflow || !((X - C) pred C2))
```
Which might make the implementation a bit simpler because you only have two optional inverts, and can always use exactUnionWith. Not sure whether this is better or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154565