[PATCH] D74228: [PatternMatch] Match XOR variant of unsigned-add overflow check.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D74228#1878189 <https://reviews.llvm.org/D74228#1878189>, @nikic wrote:

> > The transform makes sense in IR because it eliminates a use of a variable and trades an 'add' for a 'not' - both of those are better for analysis and enabling subsequent folding. If we are questioning that transform, then we have to be ok with reversing that transform: turn 'not' into 'add' and create an extra use of a variable. To do that, we'd need to show that that is at least not harmful in codegen. Given the history of regressions using overflow intrinsics, I'm skeptical that we want to reverse that.
>
> Okay, that makes sense. I'm not even sure we could do the reverse fold, thanks to the magic of undef. `(A < ~B) => (A + B < B)` for `A = -1, B = undef`, the former is guaranteed `false`, while the latter could be `true` or `false` as the two uses of B can be chosen independently.


We almost can now, it would be `C = freeze(B); (A + C < C)`
Almost, because i think codegen patch D29014 <https://reviews.llvm.org/D29014> is stuck :/

>> For similar reasons, I don't think we want to canonicalize to overflow intrinsics if we're not using the math part of the op - that would be harmful to IR-level transforms and codegen unless we teach every pass to undo those?
> 
> Yes, definitely. We can't even canonicalize to (unsigned) overflow intrinsics even if the result is used, because they are so badly optimized right now.
> 
> So overall, I agree as well that the approach in this patch is fine :)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74228