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

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

[Andrew Pinski]

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.

OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.

Thanks,
Andrew Pinski

ChangeLog:
* match.pd ( X * (X >/>=/</<= 0 ? 1 : -1)): New patterns.

Testsuite/ChangeLog:
* testsuite/gcc.dg/tree-ssa/mult-abs-1.c: New testcase.
* testsuite/gcc.dg/tree-ssa/mult-abs-2.c: New testcase.

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

[Andrew Pinski]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

Forgot the patch

On Fri, Jun 23, 2017 at 8:59 PM, Andrew Pinski <pinskia@gmail.com> 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.
>
> OK?  Bootstrapped and tested on aarch64-linux-gnu with no regressions.
>
> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * match.pd ( X * (X >/>=/</<= 0 ? 1 : -1)): New patterns.
>
> Testsuite/ChangeLog:
> * testsuite/gcc.dg/tree-ssa/mult-abs-1.c: New testcase.
> * testsuite/gcc.dg/tree-ssa/mult-abs-2.c: New testcase.

[-- Attachment #2: mult-abs.diff.txt --]
[-- Type: text/plain, Size: 4029 bytes --]

Index: match.pd
===================================================================
--- match.pd	(revision 249613)
+++ match.pd	(working copy)
@@ -155,6 +155,55 @@
            || !COMPLEX_FLOAT_TYPE_P (type)))
    (negate @0)))
 
+(for cmp (gt ge)
+ /* Transform X * (X > 0 ? 1 : -1) into ABS(X). */
+ /* Transform X * (X >= 0 ? 1 : -1) into ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 integer_zerop) integer_onep integer_all_onesp))
+  (abs @0))
+ /* Transform X * (X > 0.0 ? 1.0 : -1.0) into ABS(X). */
+ /* Transform X * (X >= 0.0 ? 1.0 : -1.0) into ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 real_zerop) real_onep real_minus_onep))
+  (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type))
+   (abs @0)))
+ /* Transform X * (X > 0 ? -1 : 1) into -ABS(X). */
+ /* Transform X * (X >= 0 ? -1 : 1) into -ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 integer_zerop) integer_all_onesp integer_onep))
+  (negate (abs @0)))
+ /* Transform X * (X > 0.0 ? -1.0 : 1.0) into -ABS(X). */
+ /* Transform X * (X >= 0.0 ? -1.0 : 1.0) into -ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 real_zerop) real_minus_onep real_onep))
+  (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type))
+   (negate (abs @0)))))
+
+(for cmp (lt le)
+ /* Transform X * (X < 0 ? 1 : -1) into -ABS(X). */
+ /* Transform X * (X <= 0 ? 1 : -1) into -ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 integer_zerop) integer_onep integer_all_onesp))
+  (negate (abs @0)))
+ /* Transform X * (X < 0.0 ? 1.0 : -1.0) into -ABS(X). */
+ /* Transform X * (X <= 0.0 ? 1.0 : -1.0) into -ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 real_zerop) real_onep real_minus_onep))
+  (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type))
+   (negate (abs @0))))
+ /* Transform X * (X < 0 ? -1 : 1) into ABS(X). */
+ /* Transform X * (X <= 0 ? -1 : 1) into ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 integer_zerop) integer_all_onesp integer_onep))
+  (abs @0))
+ /* Transform X * (X < 0.0 ? -1.0 : 1.0) into ABS(X). */
+ /* Transform X * (X <= 0.0 ? -1.0 : 1.0) into ABS(X). */
+ (simplify
+  (mult:c @0 (cond (cmp @0 real_zerop) real_minus_onep real_onep))
+  (if (!HONOR_SNANS (type) && !HONOR_SIGNED_ZEROS (type))
+   (abs @0))))
+
+
 /* X * 1, X / 1 -> X.  */
 (for op (mult trunc_div ceil_div floor_div round_div exact_div)
   (simplify
Index: testsuite/gcc.dg/tree-ssa/mult-abs-1.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/mult-abs-1.c	(nonexistent)
+++ testsuite/gcc.dg/tree-ssa/mult-abs-1.c	(working copy)
@@ -0,0 +1,35 @@
+/* { dg-options "-O2 -fdump-tree-gimple" } */
+/* { dg-do compile } */
+int f(int x)
+{
+  return x * (x > 0 ? -1 : 1);
+}
+int f1(int x)
+{
+  return x * (x > 0 ? 1 : -1);
+}
+int g(int x)
+{
+  return x * (x >= 0 ? -1 : 1);
+}
+int g1(int x)
+{
+  return x * (x >= 0 ? 1 : -1);
+}
+int h(int x)
+{
+  return x * (x < 0 ? -1 : 1);
+}
+int h1(int x)
+{
+  return x * (x < 0 ? 1 : -1);
+}
+int i(int x)
+{
+  return x * (x <= 0 ? -1 : 1);
+}
+int i1(int x)
+{
+  return x * (x <= 0 ? 1 : -1);
+}
+/* { dg-final { scan-tree-dump-times "ABS" 8 "gimple"} } */
Index: testsuite/gcc.dg/tree-ssa/mult-abs-2.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/mult-abs-2.c	(nonexistent)
+++ testsuite/gcc.dg/tree-ssa/mult-abs-2.c	(working copy)
@@ -0,0 +1,35 @@
+/* { dg-options "-O2 -ffast-math -fdump-tree-gimple" } */
+/* { dg-do compile } */
+float f(float x)
+{
+  return x * (x > 0.f ? -1.f : 1.f);
+}
+float f1(float x)
+{
+  return x * (x > 0.f ? 1.f : -1.f);
+}
+float g(float x)
+{
+  return x * (x >= 0.f ? -1.f : 1.f);
+}
+float g1(float x)
+{
+  return x * (x >= 0.f ? 1.f : -1.f);
+}
+float h(float x)
+{
+  return x * (x < 0.f ? -1.f : 1.f);
+}
+float h1(float x)
+{
+  return x * (x < 0.f ? 1.f : -1.f);
+}
+float i(float x)
+{
+  return x * (x <= 0.f ? -1.f : 1.f);
+}
+float i1(float x)
+{
+  return x * (x <= 0.f ? 1.f : -1.f);
+}
+/* { dg-final { scan-tree-dump-times "ABS" 8 "gimple"} } */

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

[Marc Glisse]

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...

* 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).

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

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

* 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.

* 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 ;-)

-- 
Marc Glisse

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

[Andrew Pinski]

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

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

[Marc Glisse]

On Sat, 24 Jun 2017, Andrew Pinski wrote:

>> * 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.

Note that I have no idea what guarantees we give in gcc. It is quite 
possible that your patch is fine without this change, I only wanted to 
raise the question in case someone knows.

-- 
Marc Glisse

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

[Andrew Pinski]

On Sat, Jun 24, 2017 at 12:47 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Sat, 24 Jun 2017, Andrew Pinski wrote:
>
>>> * 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.
>
>
> Note that I have no idea what guarantees we give in gcc. It is quite
> possible that your patch is fine without this change, I only wanted to raise
> the question in case someone knows.

So looking through, we do guarantee the sign of the NaNs except when
not honoring NaNs in the first place.  So the conversion from
a>0?1.0:-1.0 to copysign will be conditional on honoring NaNs.  But
the x*copysign(1.0,x) will only conditional on not honoring sNaNs.

>
> --
> Marc Glisse

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

[Joseph Myers]

On Sat, 24 Jun 2017, Marc Glisse wrote:

> * if X is NaN, we may get a qNaN with the wrong sign bit. We probably don't
> care much though...

The sign bit from a multiplication involving a NaN is not specified.  
*But* making any of these transformations with a qNaN loses the "invalid" 
exception from an ordered comparison involving a qNaN, so isn't valid in 
the case of (qNaNs respected and trapping-math).

-- 
Joseph S. Myers
joseph@codesourcery.com