[PATCH] D58881: [Transform] Improve fold of sadd.with.overflow

[Dan Robertson via Phabricator via llvm-commits]

dlrobertson marked 3 inline comments as done.
dlrobertson added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/sadd-with-overflow.ll:36-37
 ;
   %2 = add nsw i8 %0, 127
   %3 = tail call { i8, i1 } @llvm.sadd.with.overflow.i8(i8 %2, i8 127)
   ret { i8, i1 } %3
----------------
spatel wrote:
> Nit: Ideally, the positive test would use constants at the limit, and the negative test would use constants just over that limit. So the 1st test would be something like:
> 
> ```
>   %2 = add nsw i8 %0, 100
>   %3 = tail call { i8, i1 } @llvm.sadd.with.overflow.i8(i8 %2, i8 27)
>   ret { i8, i1 } %3
> ```
> and this one:
> 
> ```
>   %2 = add nsw i8 %0, 100
>   %3 = tail call { i8, i1 } @llvm.sadd.with.overflow.i8(i8 %2, i8 28)
>   ret { i8, i1 } %3
> 
> ```
> 
> That gives us just a bit more coverage in case the underlying code breaks somehow, and makes it clearer how the transform is implemented.
Good point. Added in the update.


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

https://reviews.llvm.org/D58881