diff mbox

Add simple sign-stripping cases to match.pd

Message ID 87twpsi87y.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford Oct. 15, 2015, 1:15 p.m. UTC
This patch makes sure that, for every simplification that uses
fold_strip_sign_ops, there are associated match.pd rules for the
leaf sign ops, i.e. abs, negate and copysign.  A follow-on patch
will add a pass to handle more complex cases.

Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
OK to install?

Thanks,
Richard


gcc/
	* match.pd: Add rules to simplify ccos, ccosh, hypot, copysign
	and x*x in cases where the operands are sign ops.  Extend these
	rules to handle copysign as a sign op (including for cos, cosh
	and pow, which already treated negate and abs as sign ops).

Comments

Marc Glisse Oct. 15, 2015, 4:27 p.m. UTC | #1
On Thu, 15 Oct 2015, Richard Sandiford wrote:

> This patch makes sure that, for every simplification that uses
> fold_strip_sign_ops, there are associated match.pd rules for the
> leaf sign ops, i.e. abs, negate and copysign.  A follow-on patch
> will add a pass to handle more complex cases.
>
> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
> OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> 	* match.pd: Add rules to simplify ccos, ccosh, hypot, copysign
> 	and x*x in cases where the operands are sign ops.  Extend these
> 	rules to handle copysign as a sign op (including for cos, cosh
> 	and pow, which already treated negate and abs as sign ops).
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 83c48cd..4331df6 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -61,6 +61,12 @@ along with GCC; see the file COPYING3.  If not see
> (define_operator_list TAN BUILT_IN_TANF BUILT_IN_TAN BUILT_IN_TANL)
> (define_operator_list COSH BUILT_IN_COSHF BUILT_IN_COSH BUILT_IN_COSHL)
> (define_operator_list CEXPI BUILT_IN_CEXPIF BUILT_IN_CEXPI BUILT_IN_CEXPIL)
> +(define_operator_list CCOS BUILT_IN_CCOSF BUILT_IN_CCOS BUILT_IN_CCOSL)
> +(define_operator_list CCOSH BUILT_IN_CCOSHF BUILT_IN_CCOSH BUILT_IN_CCOSHL)
> +(define_operator_list HYPOT BUILT_IN_HYPOTF BUILT_IN_HYPOT BUILT_IN_HYPOTL)
> +(define_operator_list COPYSIGN BUILT_IN_COPYSIGNF
> +			       BUILT_IN_COPYSIGN
> +			       BUILT_IN_COPYSIGNL)
>
> /* Simplifications of operations with one constant operand and
>    simplifications to constants or single values.  */
> @@ -321,7 +327,69 @@ along with GCC; see the file COPYING3.  If not see
>    (pows (op @0) REAL_CST@1)
>    (with { HOST_WIDE_INT n; }
>     (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
> -     (pows @0 @1))))))
> +     (pows @0 @1)))))
> + /* Strip negate and abs from both operands of hypot.  */
> + (for hypots (HYPOT)
> +  (simplify
> +   (hypots (op @0) @1)
> +   (hypots @0 @1))
> +  (simplify
> +   (hypots @0 (op @1))
> +   (hypots @0 @1)))

Out of curiosity, would hypots:c have worked? (it is probably not worth 
gratuitously swapping the operands to save 3 lines though)

> + /* copysign(-x, y) and copysign(abs(x), y) -> copysign(x, y).  */
> + (for copysigns (COPYSIGN)
> +  (simplify
> +   (copysigns (op @0) @1)
> +   (copysigns @0 @1)))
> + /* -x*-x and abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
> + (simplify
> +  (mult (op @0) (op @1))
> +  (mult @0 @0)))

Typo @1 -> @0 ?
This will partially duplicate Naveen's patch "Move some bit and binary 
optimizations in simplify and match".

> +/* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
> +(for coss (COS COSH)
> +     copysigns (COPYSIGN)
> + (simplify
> +  (coss (copysigns @0 @1))
> +   (coss @0)))
> +
> +/* pow(copysign(x, y), z) -> pow(x, z) if z is an even integer.  */
> +(for pows (POW)
> +     copysigns (COPYSIGN)
> + (simplify
> +  (pows (copysigns @0 @1) REAL_CST@1)
> +  (with { HOST_WIDE_INT n; }
> +   (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
> +    (pows @0 @1)))))
> +
> +(for hypots (HYPOT)
> +     copysigns (COPYSIGN)
> + /* hypot(copysign(x, y), z) -> hypot(x, z).  */
> + (simplify
> +  (hypots (copysigns @0 @1) @2)
> +  (hypots @0 @2))
> + /* hypot(x, copysign(y, z)) -> hypot(x, y).  */
> + (simplify
> +  (hypots @0 (copysigns @1 @2))
> +  (hypots @0 @1)))
> +
> +/* copysign(copysign(x, y), z) -> copysign(x, z).  */
> +(for copysigns (COPYSIGN)
> + (simplify
> +  (copysigns (copysigns @0 @1) @2)
> +  (copysigns @0 @2)))
> +
> +/* copysign(x,y)*copysign(x,y) -> x*x.  */
> +(for copysigns (COPYSIGN)
> + (simplify
> +  (mult (copysigns @0 @1) (copysigns @0 @1))

(mult (copysigns@2 @0 @1) @2)
? Or is there some reason not to rely on CSE? (I don't think copysign has 
any errno issue)

> +  (mult @0 @0)))

Would copysign(x,y)*copysign(z,y) -> abs(x*z) make sense?

> +/* ccos(-x) -> ccos(x).  Similarly for ccosh.  */
> +(for ccoss (CCOS CCOSH)
> + (simplify
> +  (ccoss (negate @0))
> +   (ccoss @0)))
>
> /* X % Y is smaller than Y.  */
> (for cmp (lt ge)
Richard Sandiford Oct. 15, 2015, 7:36 p.m. UTC | #2
Marc Glisse <marc.glisse@inria.fr> writes:
> On Thu, 15 Oct 2015, Richard Sandiford wrote:
>
>> This patch makes sure that, for every simplification that uses
>> fold_strip_sign_ops, there are associated match.pd rules for the
>> leaf sign ops, i.e. abs, negate and copysign.  A follow-on patch
>> will add a pass to handle more complex cases.
>>
>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>> OK to install?
>>
>> Thanks,
>> Richard
>>
>>
>> gcc/
>> 	* match.pd: Add rules to simplify ccos, ccosh, hypot, copysign
>> 	and x*x in cases where the operands are sign ops.  Extend these
>> 	rules to handle copysign as a sign op (including for cos, cosh
>> 	and pow, which already treated negate and abs as sign ops).
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index 83c48cd..4331df6 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -61,6 +61,12 @@ along with GCC; see the file COPYING3.  If not see
>> (define_operator_list TAN BUILT_IN_TANF BUILT_IN_TAN BUILT_IN_TANL)
>> (define_operator_list COSH BUILT_IN_COSHF BUILT_IN_COSH BUILT_IN_COSHL)
>> (define_operator_list CEXPI BUILT_IN_CEXPIF BUILT_IN_CEXPI BUILT_IN_CEXPIL)
>> +(define_operator_list CCOS BUILT_IN_CCOSF BUILT_IN_CCOS BUILT_IN_CCOSL)
>> +(define_operator_list CCOSH BUILT_IN_CCOSHF BUILT_IN_CCOSH BUILT_IN_CCOSHL)
>> +(define_operator_list HYPOT BUILT_IN_HYPOTF BUILT_IN_HYPOT BUILT_IN_HYPOTL)
>> +(define_operator_list COPYSIGN BUILT_IN_COPYSIGNF
>> +			       BUILT_IN_COPYSIGN
>> +			       BUILT_IN_COPYSIGNL)
>>
>> /* Simplifications of operations with one constant operand and
>>    simplifications to constants or single values.  */
>> @@ -321,7 +327,69 @@ along with GCC; see the file COPYING3.  If not see
>>    (pows (op @0) REAL_CST@1)
>>    (with { HOST_WIDE_INT n; }
>>     (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
>> -     (pows @0 @1))))))
>> +     (pows @0 @1)))))
>> + /* Strip negate and abs from both operands of hypot.  */
>> + (for hypots (HYPOT)
>> +  (simplify
>> +   (hypots (op @0) @1)
>> +   (hypots @0 @1))
>> +  (simplify
>> +   (hypots @0 (op @1))
>> +   (hypots @0 @1)))
>
> Out of curiosity, would hypots:c have worked? (it is probably not worth 
> gratuitously swapping the operands to save 3 lines though)

Yeah, I think I'd prefer to keep it like it is if that's OK.

>> + /* copysign(-x, y) and copysign(abs(x), y) -> copysign(x, y).  */
>> + (for copysigns (COPYSIGN)
>> +  (simplify
>> +   (copysigns (op @0) @1)
>> +   (copysigns @0 @1)))
>> + /* -x*-x and abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
>> + (simplify
>> +  (mult (op @0) (op @1))
>> +  (mult @0 @0)))
>
> Typo @1 -> @0 ?

Argh!  Thanks for catching that.  Wonder how many proof-reads that
escaped :-(

> This will partially duplicate Naveen's patch "Move some bit and binary 
> optimizations in simplify and match".

OK.  Should I just limit it to the abs case?

>> +/* copysign(x,y)*copysign(x,y) -> x*x.  */
>> +(for copysigns (COPYSIGN)
>> + (simplify
>> +  (mult (copysigns @0 @1) (copysigns @0 @1))
>
> (mult (copysigns@2 @0 @1) @2)
> ? Or is there some reason not to rely on CSE? (I don't think copysign has 
> any errno issue)

No, simply didn't know about that trick.  I'll use it for the
(mult (op @0) (op @0)) case as well.

> Would copysign(x,y)*copysign(z,y) -> abs(x*z) make sense?

Possibly :-)  TBH though I'd rather just stick to moving the existing
rules for now.  Moving builtins.c folds to match.pd is really only a
prerequisite for the change that I actually want to make, namely having
internal functions for all the libm optabs.  The idea is that something
like vector sqrt will get picked up simply by implementing the sqrt
optab for that mode, rather than using optabs for scalar modes and
TARGET_VECTORIZE_BUILTIN_VECTORIZED_FUNCTION for the equivalent vector
operations.  That in turn should unblock Dave Sherwood's patch to add
"strict" fmin and fmax.  I'm just worried that we're running out
of time to get everything in by the close of stage 1.

Thanks,
Richard
Richard Sandiford Oct. 19, 2015, 10:46 a.m. UTC | #3
Richard Sandiford <richard.sandiford@arm.com> writes:
> Marc Glisse <marc.glisse@inria.fr> writes:
>> On Thu, 15 Oct 2015, Richard Sandiford wrote:
>>
>>> This patch makes sure that, for every simplification that uses
>>> fold_strip_sign_ops, there are associated match.pd rules for the
>>> leaf sign ops, i.e. abs, negate and copysign.  A follow-on patch
>>> will add a pass to handle more complex cases.
>>>
>>> Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.
>>> OK to install?
>>>
>>> Thanks,
>>> Richard
>>>
>>>
>>> gcc/
>>> 	* match.pd: Add rules to simplify ccos, ccosh, hypot, copysign
>>> 	and x*x in cases where the operands are sign ops.  Extend these
>>> 	rules to handle copysign as a sign op (including for cos, cosh
>>> 	and pow, which already treated negate and abs as sign ops).
>>>
>>> diff --git a/gcc/match.pd b/gcc/match.pd
>>> index 83c48cd..4331df6 100644
>>> --- a/gcc/match.pd
>>> +++ b/gcc/match.pd
>>> @@ -61,6 +61,12 @@ along with GCC; see the file COPYING3.  If not see
>>> (define_operator_list TAN BUILT_IN_TANF BUILT_IN_TAN BUILT_IN_TANL)
>>> (define_operator_list COSH BUILT_IN_COSHF BUILT_IN_COSH BUILT_IN_COSHL)
>>> (define_operator_list CEXPI BUILT_IN_CEXPIF BUILT_IN_CEXPI BUILT_IN_CEXPIL)
>>> +(define_operator_list CCOS BUILT_IN_CCOSF BUILT_IN_CCOS BUILT_IN_CCOSL)
>>> +(define_operator_list CCOSH BUILT_IN_CCOSHF BUILT_IN_CCOSH BUILT_IN_CCOSHL)
>>> +(define_operator_list HYPOT BUILT_IN_HYPOTF BUILT_IN_HYPOT BUILT_IN_HYPOTL)
>>> +(define_operator_list COPYSIGN BUILT_IN_COPYSIGNF
>>> +			       BUILT_IN_COPYSIGN
>>> +			       BUILT_IN_COPYSIGNL)
>>>
>>> /* Simplifications of operations with one constant operand and
>>>    simplifications to constants or single values.  */
>>> @@ -321,7 +327,69 @@ along with GCC; see the file COPYING3.  If not see
>>>    (pows (op @0) REAL_CST@1)
>>>    (with { HOST_WIDE_INT n; }
>>>     (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
>>> -     (pows @0 @1))))))
>>> +     (pows @0 @1)))))
>>> + /* Strip negate and abs from both operands of hypot.  */
>>> + (for hypots (HYPOT)
>>> +  (simplify
>>> +   (hypots (op @0) @1)
>>> +   (hypots @0 @1))
>>> +  (simplify
>>> +   (hypots @0 (op @1))
>>> +   (hypots @0 @1)))
>>
>> Out of curiosity, would hypots:c have worked? (it is probably not worth 
>> gratuitously swapping the operands to save 3 lines though)
>
> Yeah, I think I'd prefer to keep it like it is if that's OK.
>
>>> + /* copysign(-x, y) and copysign(abs(x), y) -> copysign(x, y).  */
>>> + (for copysigns (COPYSIGN)
>>> +  (simplify
>>> +   (copysigns (op @0) @1)
>>> +   (copysigns @0 @1)))
>>> + /* -x*-x and abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
>>> + (simplify
>>> +  (mult (op @0) (op @1))
>>> +  (mult @0 @0)))
>>
>> Typo @1 -> @0 ?
>
> Argh!  Thanks for catching that.  Wonder how many proof-reads that
> escaped :-(
>
>> This will partially duplicate Naveen's patch "Move some bit and binary 
>> optimizations in simplify and match".
>
> OK.  Should I just limit it to the abs case?
>
>>> +/* copysign(x,y)*copysign(x,y) -> x*x.  */
>>> +(for copysigns (COPYSIGN)
>>> + (simplify
>>> +  (mult (copysigns @0 @1) (copysigns @0 @1))
>>
>> (mult (copysigns@2 @0 @1) @2)
>> ? Or is there some reason not to rely on CSE? (I don't think copysign has 
>> any errno issue)
>
> No, simply didn't know about that trick.  I'll use it for the
> (mult (op @0) (op @0)) case as well.

Here's the updated patch.  I've kept the (mult (negate@1 @0) @1)
pattern for now, but can limit it to abs as necessary when
Naveen's patch goes in.

Tested on x86_64-linux-gnu, aarch64-linux-gnu and arm-linux-gnueabi.

Thanks,
Richard
diff mbox

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 83c48cd..4331df6 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -61,6 +61,12 @@  along with GCC; see the file COPYING3.  If not see
 (define_operator_list TAN BUILT_IN_TANF BUILT_IN_TAN BUILT_IN_TANL)
 (define_operator_list COSH BUILT_IN_COSHF BUILT_IN_COSH BUILT_IN_COSHL)
 (define_operator_list CEXPI BUILT_IN_CEXPIF BUILT_IN_CEXPI BUILT_IN_CEXPIL)
+(define_operator_list CCOS BUILT_IN_CCOSF BUILT_IN_CCOS BUILT_IN_CCOSL)
+(define_operator_list CCOSH BUILT_IN_CCOSHF BUILT_IN_CCOSH BUILT_IN_CCOSHL)
+(define_operator_list HYPOT BUILT_IN_HYPOTF BUILT_IN_HYPOT BUILT_IN_HYPOTL)
+(define_operator_list COPYSIGN BUILT_IN_COPYSIGNF
+			       BUILT_IN_COPYSIGN
+			       BUILT_IN_COPYSIGNL)
 
 /* Simplifications of operations with one constant operand and
    simplifications to constants or single values.  */
@@ -321,7 +327,69 @@  along with GCC; see the file COPYING3.  If not see
    (pows (op @0) REAL_CST@1)
    (with { HOST_WIDE_INT n; }
     (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
-     (pows @0 @1))))))
+     (pows @0 @1)))))
+ /* Strip negate and abs from both operands of hypot.  */
+ (for hypots (HYPOT)
+  (simplify
+   (hypots (op @0) @1)
+   (hypots @0 @1))
+  (simplify
+   (hypots @0 (op @1))
+   (hypots @0 @1)))
+ /* copysign(-x, y) and copysign(abs(x), y) -> copysign(x, y).  */
+ (for copysigns (COPYSIGN)
+  (simplify
+   (copysigns (op @0) @1)
+   (copysigns @0 @1)))
+ /* -x*-x and abs(x)*abs(x) -> x*x.  Should be valid for all types.  */
+ (simplify
+  (mult (op @0) (op @1))
+  (mult @0 @0)))
+
+/* cos(copysign(x, y)) -> cos(x).  Similarly for cosh.  */
+(for coss (COS COSH)
+     copysigns (COPYSIGN)
+ (simplify
+  (coss (copysigns @0 @1))
+   (coss @0)))
+
+/* pow(copysign(x, y), z) -> pow(x, z) if z is an even integer.  */
+(for pows (POW)
+     copysigns (COPYSIGN)
+ (simplify
+  (pows (copysigns @0 @1) REAL_CST@1)
+  (with { HOST_WIDE_INT n; }
+   (if (real_isinteger (&TREE_REAL_CST (@1), &n) && (n & 1) == 0)
+    (pows @0 @1)))))
+
+(for hypots (HYPOT)
+     copysigns (COPYSIGN)
+ /* hypot(copysign(x, y), z) -> hypot(x, z).  */
+ (simplify
+  (hypots (copysigns @0 @1) @2)
+  (hypots @0 @2))
+ /* hypot(x, copysign(y, z)) -> hypot(x, y).  */
+ (simplify
+  (hypots @0 (copysigns @1 @2))
+  (hypots @0 @1)))
+
+/* copysign(copysign(x, y), z) -> copysign(x, z).  */
+(for copysigns (COPYSIGN)
+ (simplify
+  (copysigns (copysigns @0 @1) @2)
+  (copysigns @0 @2)))
+
+/* copysign(x,y)*copysign(x,y) -> x*x.  */
+(for copysigns (COPYSIGN)
+ (simplify
+  (mult (copysigns @0 @1) (copysigns @0 @1))
+  (mult @0 @0)))
+
+/* ccos(-x) -> ccos(x).  Similarly for ccosh.  */
+(for ccoss (CCOS CCOSH)
+ (simplify
+  (ccoss (negate @0))
+   (ccoss @0)))
 
 /* X % Y is smaller than Y.  */
 (for cmp (lt ge)