[PATCH] D54532: [InstructionSimplify] Add support for saturating add/sub

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: nlopes.
spatel added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:4925-4928
+    if (match(Op1, m_Zero()) || match(Op1, m_Undef()))
+      return Op0;
+    // 0 + X -> X, undef + X -> X
+    if (match(Op0, m_Zero()) || match(Op0, m_Undef()))
----------------
nikic wrote:
> spatel wrote:
> > It's slightly better to return a constant when one of the operands is undef because that eliminates the use of a variable. That also matches the behavior of the subtracts. 
> > 
> > For sadd_sat, we can always return 0, and for uadd_sat, we can always return -1?
> The question of undef handling is also discussed here: https://reviews.llvm.org/D54237#1294571
> 
> The problem is that we can't return 0 for sadd_sat, due to the asymmetric range for signed integers. If one operand is -128, the largest number you can reach is -128 + 127 = -1, not 0. That's why I'm returning the other operand here, so that handling is consistent for uadd/sadd.
> 
> But ... now that I think about it, shouldn't it be legal to return -1 for *both* the signed and the unsigned case?
Return -1 for both sounds good to me. cc'ing @nlopes in case he sees an alternative.

In the earlier discussion, you said (uadd_sat X, undef) could return 0, but I don't think that's possible. Say for example that X=42. There's no undef value that we can add to get us to 0 (or any value <42). uadd_sat must go to all-ones or choose "0" as the undef and return X.


https://reviews.llvm.org/D54532