This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] fold a * (a > 0 ? 1 : -1) to abs(a) and related optimizations


On Fri, Jun 23, 2017 at 11:50 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Fri, 23 Jun 2017, Andrew Pinski wrote:
>
>> Hi,
>>  I saw this on llvm's review site (https://reviews.llvm.org/D34579)
>> and I thought why not add it to GCC.  I expanded more than what was
>> done on the LLVM patch.
>>
>> I added the following optimizations:
>> Transform X * (X > 0 ? 1 : -1) into ABS(X).
>> Transform X * (X >= 0 ? 1 : -1) into ABS(X).
>> Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X).
>> Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X).
>> Transform X * (X > 0 ? -1 : 1) into -ABS(X).
>> Transform X * (X >= 0 ? -1 : 1) into -ABS(X).
>> Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X).
>> Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X).
>> Transform X * (X < 0 ? 1 : -1) into -ABS(X).
>> Transform X * (X <= 0 ? 1 : -1) into -ABS(X).
>> Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X).
>> Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X).
>> Transform X * (X < 0 ? -1 : 1) into ABS(X).
>> Transform X * (X <= 0 ? -1 : 1) into ABS(X).
>> Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X).
>> Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X).
>>
>> The floating points ones only happen when not honoring SNANS and not
>> honoring signed zeros.
>
>
> Some random comments (not a review):
>
> * if X is NaN, we may get a qNaN with the wrong sign bit. We probably don't
> care much though...

Ok, I changed it to when not honoring NANs.

>
> * I am surprised (X<0.?-1.:1.) and copysign(1., X) remain different for the
> whole optimization pipeline with -ffast-math. X*copysign(1., X) is another
> candidate to become fabs(X).

This might be a better idea because of ...

>
> * Whenever you get -ABS(X) for integers, what about the case where X is
> INT_MIN?

This.  Yes this is an issue; I guess I need to rethink the integer patterns.


>
> * I guess we can't get there with an unsigned type because X>0 would have
> become X!=0 .

No, unsigned is not an issue.

>
> * I wonder if we could use something like
>
> (for cmp (gt ge lt le)
>      outp (convert convert negate negate)
>      outn (negate negate convert convert)
> [...]
>   (outp (abs @0))
>
> to reduce duplication or if that would be less readable.


I did thought of that but I added the lt/le parts latter on.

>
> * Some of the cases are handled by PRE turning
>
>   # iftmp.0_1 = PHI <1.0e+0(5), -1.0e+0(3)>
>   _3 = iftmp.0_1 * a_2(D);
>
> into
>
>   _5 = -a_2(D);
> [...]
>   # iftmp.0_1 = PHI <1.0e+0(2), -1.0e+0(3)>
>   # prephitmp_6 = PHI <a_2(D)(2), _5(3)>
>
> which phiopt3 can handle (quite late).
>
> * With cond, this currently (?) only affects generic, so I am not sure it
> will hit very often... But it will be there if someone later writes a
> match.pd->phiopt generator ;-)

I have a start of this patch but I have not finished it yet.  Both
phiopt and ifcombine should be moved over to gimple match and
simplify.
I will submit a new patch which implements some of the above by the
end of the day; I might split up the patch into two (one for the
integer case and one for the floating point case).

Thanks,
Andrew

>
> --
> Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]