llvm dev - Oct 2019 - [RFC] Use of saturating intrinsics

Hello all again, take 2.

Over in D68651 I would like to make code that attempt to saturate an value
(using higher bitwidth integers) use a saturating intrinsic instead. Something
like this:
https://godbolt.org/z/9knBnP

As can be seen, the unsigned cases are already being matched to llvm.uadd.sat
intrinsics. I am hoping to extend that to the signed cases. This has numerous
benefits including simpler vectorization, cost-modelling and matching in the
backend. The current forms of the saturating intrinsics extend into a higher
type, which can be awkward to deal with in some cases (i64's are often not
legal types, for example).

The intrinsic seems to produce as good or better code in almost all the cases I
have tried (partly thanks to patches like D68643 and the many other optimisation
over the years into the backend to efficiently optimise these saturating
intrinsics, even when they are not natively available.)

As noted in the review, this does bring up the question of what we should
consider canonical. I believe that these intrinsics should be (especially over
changing types!). Let us know if you disagree.

Cheers
Dave

On Thu, Oct 10, 2019 at 6:18 PM David Green via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Hello all again, take 2.
>
> Over in D68651 I would like to make code that attempt to saturate an value
(using higher bitwidth integers) use a saturating intrinsic instead. Something
like this:
> https://godbolt.org/z/9knBnP
>
> As can be seen, the unsigned cases are already being matched to
llvm.uadd.sat intrinsics. I am hoping to extend that to the signed cases. This
has numerous benefits including simpler vectorization, cost-modelling and
matching in the backend. The current forms of the saturating intrinsics extend
into a higher type, which can be awkward to deal with in some cases (i64's
are often not legal types, for example).
>
> The intrinsic seems to produce as good or better code in almost all the
cases I have tried (partly thanks to patches like D68643 and the many other
optimisation over the years into the backend to efficiently optimise these
saturating intrinsics, even when they are not natively available.)
>
> As noted in the review, this does bring up the question of what we should
consider canonical. I believe that these intrinsics should be (especially over
changing types!).
In this particular case for this particular intrinsic i do believe the
reasoning is sound.

In https://reviews.llvm.org/D68651 Hideki Saito does raise a point
about whether this canonicalization hinders some other optimizations.
Does it appear to regress anything?
> Let us know if you disagree.
>
> Cheers
> Dave
Roman
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

On 10/10/2019 17:29, Roman Lebedev wrote:
> On Thu, Oct 10, 2019 at 6:18 PM David Green via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>>
>> Hello all again, take 2.
>>
>> Over in D68651 I would like to make code that attempt to saturate an
value (using higher bitwidth integers) use a saturating intrinsic instead.
Something like this:
>> https://godbolt.org/z/9knBnP
>>
>> As can be seen, the unsigned cases are already being matched to
llvm.uadd.sat intrinsics. I am hoping to extend that to the signed cases. This
has numerous benefits including simpler vectorization, cost-modelling and
matching in the backend. The current forms of the saturating intrinsics extend
into a higher type, which can be awkward to deal with in some cases (i64's
are often not legal types, for example).
>>
>> The intrinsic seems to produce as good or better code in almost all the
cases I have tried (partly thanks to patches like D68643 and the many other
optimisation over the years into the backend to efficiently optimise these
saturating intrinsics, even when they are not natively available.)
>>
>> As noted in the review, this does bring up the question of what we
should consider canonical. I believe that these intrinsics should be (especially
over changing types!).
> In this particular case for this particular intrinsic i do believe the
> reasoning is sound.
> 
> In https://reviews.llvm.org/D68651 Hideki Saito does raise a point
> about whether this canonicalization hinders some other optimizations.
> Does it appear to regress anything?
I have not seen anything being hindered by these. They are likely not 
optimised as well as a standard add/sub, but can be dealt with more 
simply than an add/sub with a min, max, sext and trunc attached.

This shows that a lot of stuff is already being done well:
https://godbolt.org/z/lZzWSA
Plus the tests in llvm/test/Transforms/InstCombine/saturating-add-sub.ll

That's a lot of things like constant folding, commuting, 
canonicalization to "add" over "sub", and to "add
nsw" over a sadd_sat
that cannot overflow. There are other tests that indvar simplification 
works.

Not sure what else would be good to add to that list. Let me know if you 
think of anything obvious.

Dave
> 
>> Let us know if you disagree.
>>
>> Cheers
>> Dave
> Roman
> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev