[PATCH] D51929: [DAGCombiner] use UADDO to optimize saturated unsigned add

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D51929#1242231, @lebedev.ri wrote:

> In https://reviews.llvm.org/D51929#1242158, @spatel wrote:
>
> > In https://reviews.llvm.org/D51929#1241507, @lebedev.ri wrote:
> >
> > > Do we somehow enforce that in `%r = select i1 %c, i32 -1, i32 %a`, `-1` is in the middle?
> > >  If not, we miss at least one case i think:
> >
> >
> > That's correct. I think there are also 4 swapped variants for the pattern with a variable and a 'not' op (online version of Alive looks dead? cc'ing @nlopes):
> > ...
> >
> > My plan is to canonicalize all of the patterns in IR. If I'm seeing it correctly, there shouldn't be anything blocking those canonicalizations because we only try to form the uaddo here when the cmp (setcc) has one use (the select). That should let us get by with just the basic matching here in the backend.
>
>
> Ok, sounds good. At worst, this can be extended.
>  I'm not sure why there is no x86 test coverage.
>  Won't `ADC` <https://www.felixcloutier.com/x86/ADC.html> work for this purpose?
>  `ADD` evaluates the result for both signed and unsigned integer operands and sets the CF and OF flags to indicate a carry (overflow) in the signed or unsigned result, respectively <https://www.felixcloutier.com/x86/ADD.html#description>, too.
>  Some plumbing missing?
>
> I think this looks good, but maybe wait a bit just in case someone else wants to comment.


Thanks! I'm not following the 'adc' suggestion though. That uses the carry from a previous op in the addition. What would that code sequence look like?


https://reviews.llvm.org/D51929