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

[Dhruv Chawla via Phabricator via llvm-commits]

0xdc03 marked 2 inline comments as done.
0xdc03 added inline comments.


================
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);
----------------
nikic wrote:
> 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.
Okay, I have done this and cleaned up the formula, hopefully the code is better now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154565