On Sun, Jun 25, 2017 at 11:18 AM, Andrew Pinski <pins...@gmail.com> wrote:
> On Sun, Jun 25, 2017 at 1:28 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
>> +(for cmp (gt ge lt le)
>> + outp (convert convert negate negate)
>> + outn (negate negate convert convert)
>> + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
>> + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
>> + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
>> + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
>> + (simplify
>> + (cond (cmp @0 real_zerop) real_onep real_minus_onep)
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
>> + && types_match (type, TREE_TYPE (@0)))
>> + (switch
>> + (if (types_match (type, float_type_node))
>> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0)))
>> + (if (types_match (type, double_type_node))
>> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0)))
>> + (if (types_match (type, long_double_type_node))
>> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0))))))
>>
>> There is already a 1.0 of the right type in the input, it would be easier to
>> reuse it in the output than build a new one.
>
> Right. Fixed.
>
>>
>> Non-generic builtins like copysign are such a pain... We also end up missing
>> the 128-bit case that way (pre-existing problem, not your patch). We seem to
>> have a corresponding internal function, but apparently it is not used until
>> expansion (well, maybe during vectorization).
>
> Yes I noticed that while working on a different patch related to
> copysign; The generic version of a*copysign(1.0, b) [see the other
> thread where the ARM folks started a patch for it; yes it was by pure
> accident that I was working on this and really did not notice that
> thread until yesterday].
> I was looking into a nice way of creating copysign without having to
> do the switch but I could not find one. In the end I copied was done
> already in a different location in match.pd; this is also the reason
> why I had the build_one_cst there.
>
>>
>> + /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
>> + /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
>> + /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */
>> + /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */
>> + (simplify
>> + (cond (cmp @0 real_zerop) real_minus_onep real_onep)
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
>> + && types_match (type, TREE_TYPE (@0)))
>> + (switch
>> + (if (types_match (type, float_type_node))
>> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outn @0)))
>> + (if (types_match (type, double_type_node))
>> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0)))
>> + (if (types_match (type, long_double_type_node))
>> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0)))))))
>> +
>> +/* Transform X * copysign (1.0, X) into abs(X). */
>> +(simplify
>> + (mult:c @0 (COPYSIGN real_onep @0))
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
>> + (abs @0)))
>>
>> I would have expected it do to the right thing for signed zero and qNaN. Can
>> you describe a case where it would give the wrong result, or are the
>> conditions just conservative?
>
> I was just being conservative; maybe too conservative but I was a bit
> worried I could get it incorrect.
>
>>
>> +/* Transform X * copysign (1.0, -X) into -abs(X). */
>> +(simplify
>> + (mult:c @0 (COPYSIGN real_onep (negate @0)))
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
>> + (negate (abs @0))))
>> +
>> +/* Transform copysign (-1.0, X) into copysign (1.0, X). */
>> +(simplify
>> + (COPYSIGN real_minus_onep @0)
>> + (COPYSIGN { build_one_cst (type); } @0))
>>
>> (simplify
>> (COPYSIGN REAL_CST@0 @1)
>> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
>> (COPYSIGN (negate @0) @1)))
>> ? Or does that create trouble with sNaN and only the 1.0 case is worth
>> the trouble?
>
> No that is the correct way; I Noticed the other thread about copysign
> had something similar as what should be done too.
>
> I will send out a new patch after testing soon.
New patch.
OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions.
Thanks,
Andrew Pinski
ChangeLog:
* match.pd (X >/>=/</<= 0 ? 1.0 : -1.0): New patterns.
(X * copysign (1.0, X)): New pattern.
(X * copysign (1.0, -X)): New pattern.
(copysign (-1.0, CST)): New pattern.
testsuite/ChangeLog:
* gcc.dg/tree-ssa/copy-sign-1.c: New testcase.
* gcc.dg/tree-ssa/copy-sign-2.c: New testcase.
* gcc.dg/tree-ssa/mult-abs-2.c: New testcase.
>
> Thanks,
> Andrew
>
>>
>> --
>> Marc Glisse
Index: match.pd
===================================================================
--- match.pd (revision 249626)
+++ match.pd (working copy)
@@ -155,6 +155,58 @@
|| !COMPLEX_FLOAT_TYPE_P (type)))
(negate @0)))
+(for cmp (gt ge lt le)
+ outp (convert convert negate negate)
+ outn (negate negate convert convert)
+ /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */
+ /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */
+ /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
+ /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */
+ (simplify
+ (cond (cmp @0 real_zerop) real_onep@1 real_minus_onep)
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
+ && types_match (type, TREE_TYPE (@0)))
+ (switch
+ (if (types_match (type, float_type_node))
+ (BUILT_IN_COPYSIGNF @1 (outp @0)))
+ (if (types_match (type, double_type_node))
+ (BUILT_IN_COPYSIGN @1 (outp @0)))
+ (if (types_match (type, long_double_type_node))
+ (BUILT_IN_COPYSIGNL @1 (outp @0))))))
+ /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
+ /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */
+ /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */
+ /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */
+ (simplify
+ (cond (cmp @0 real_zerop) real_minus_onep real_onep@1)
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)
+ && types_match (type, TREE_TYPE (@0)))
+ (switch
+ (if (types_match (type, float_type_node))
+ (BUILT_IN_COPYSIGNF @1 (outn @0)))
+ (if (types_match (type, double_type_node))
+ (BUILT_IN_COPYSIGN @1 (outn @0)))
+ (if (types_match (type, long_double_type_node))
+ (BUILT_IN_COPYSIGNL @1 (outn @0)))))))
+
+/* Transform X * copysign (1.0, X) into abs(X). */
+(simplify
+ (mult:c @0 (COPYSIGN real_onep @0))
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
+ (abs @0)))
+
+/* Transform X * copysign (1.0, -X) into -abs(X). */
+(simplify
+ (mult:c @0 (COPYSIGN real_onep (negate @0)))
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
+ (negate (abs @0))))
+
+/* Transform copysign (CST, X) into copysign (ABS(CST), X). */
+(simplify
+ (COPYSIGN REAL_CST@0 @1)
+ (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0)))
+ (COPYSIGN (negate @0) @1)))
+
/* 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/copy-sign-1.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/copy-sign-1.c (nonexistent)
+++ testsuite/gcc.dg/tree-ssa/copy-sign-1.c (working copy)
@@ -0,0 +1,35 @@
+/* { dg-options "-O2 -ffast-math -fdump-tree-gimple" } */
+/* { dg-do compile } */
+float f(float x)
+{
+ return (x > 0.f ? -1.f : 1.f);
+}
+float f1(float x)
+{
+ return (x > 0.f ? 1.f : -1.f);
+}
+float g(float x)
+{
+ return (x >= 0.f ? -1.f : 1.f);
+}
+float g1(float x)
+{
+ return (x >= 0.f ? 1.f : -1.f);
+}
+float h(float x)
+{
+ return (x < 0.f ? -1.f : 1.f);
+}
+float h1(float x)
+{
+ return (x < 0.f ? 1.f : -1.f);
+}
+float i(float x)
+{
+ return (x <= 0.f ? -1.f : 1.f);
+}
+float i1(float x)
+{
+ return (x <= 0.f ? 1.f : -1.f);
+}
+/* { dg-final { scan-tree-dump-times "copysign" 8 "gimple"} } */
Index: testsuite/gcc.dg/tree-ssa/copy-sign-2.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/copy-sign-2.c (nonexistent)
+++ testsuite/gcc.dg/tree-ssa/copy-sign-2.c (working copy)
@@ -0,0 +1,13 @@
+/* { dg-options "-O2 -ffast-math -fdump-tree-optimized" } */
+/* { dg-do compile } */
+float f(float x)
+{
+ float t = __builtin_copysignf (1.0f, x);
+ return x * t;
+}
+float f1(float x)
+{
+ float t = __builtin_copysignf (1.0f, -x);
+ return x * t;
+}
+/* { dg-final { scan-tree-dump-times "ABS" 2 "optimized"} } */
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"} } */
