diff mbox

Clean-ups in match.pd

Message ID alpine.DEB.2.02.1507041612060.16424@laptop-mg.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse July 4, 2015, 2:34 p.m. UTC
Hello,

these are just some minor changes. I believe I had already promised a 
build_ function to match integer_each_onep.

Bootstrap+testsuite on powerpc64le-unknown-linux-gnu (it looks like
*-match.c takes about 10 minutes to compile in stage2 these days).

2015-07-06  Marc Glisse  <marc.glisse@inria.fr>

 	* match.pd: Remove element_mode inside HONOR_*.
 	(~ (-A) -> A - 1, ~ (A - 1) -> -A): Handle complex types.
 	(~X | X -> -1, ~X ^ X -> -1): Merge.
 	* tree.c (build_each_one_cst): New function.
 	* tree.h (build_each_one_cst): Likewise.

Comments

Richard Biener July 6, 2015, 2:08 p.m. UTC | #1
On Sat, Jul 4, 2015 at 4:34 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hello,
>
> these are just some minor changes. I believe I had already promised a build_
> function to match integer_each_onep.
>
> Bootstrap+testsuite on powerpc64le-unknown-linux-gnu (it looks like
> *-match.c takes about 10 minutes to compile in stage2 these days).

Ouch.  I have some changes to the code generation in the queue which
also supports a more natural "if" structure (else and elif).  Eventually
that helps a bit but I suppose the main issue is simply from the large
functions.  They can be split quite easily I think, but passing down
all relevant state might turn out to be tricky unless we start using
nested functions here ... (and IIRC those are not supported in C++)



Richard.

> 2015-07-06  Marc Glisse  <marc.glisse@inria.fr>
>
>         * match.pd: Remove element_mode inside HONOR_*.
>         (~ (-A) -> A - 1, ~ (A - 1) -> -A): Handle complex types.
>         (~X | X -> -1, ~X ^ X -> -1): Merge.
>         * tree.c (build_each_one_cst): New function.
>         * tree.h (build_each_one_cst): Likewise.
>
> --
> Marc Glisse
> Index: match.pd
> ===================================================================
> --- match.pd    (revision 225411)
> +++ match.pd    (working copy)
> @@ -101,7 +101,7 @@
>     negative value by 0 gives -0, not +0.  */
>  (simplify
>   (mult @0 real_zerop@1)
> - (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (element_mode (type)))
> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
>    @1))
>
>  /* In IEEE floating point, x*1 is not equivalent to x for snans.
> @@ -108,8 +108,8 @@
>     Likewise for complex arithmetic with signed zeros.  */
>  (simplify
>   (mult @0 real_onep)
> - (if (!HONOR_SNANS (element_mode (type))
> -      && (!HONOR_SIGNED_ZEROS (element_mode (type))
> + (if (!HONOR_SNANS (type)
> +      && (!HONOR_SIGNED_ZEROS (type)
>            || !COMPLEX_FLOAT_TYPE_P (type)))
>    (non_lvalue @0)))
>
> @@ -116,8 +116,8 @@
>  /* Transform x * -1.0 into -x.  */
>  (simplify
>   (mult @0 real_minus_onep)
> -  (if (!HONOR_SNANS (element_mode (type))
> -       && (!HONOR_SIGNED_ZEROS (element_mode (type))
> +  (if (!HONOR_SNANS (type)
> +       && (!HONOR_SIGNED_ZEROS (type)
>             || !COMPLEX_FLOAT_TYPE_P (type)))
>     (negate @0)))
>
> @@ -165,7 +165,7 @@
>   (rdiv @0 @0)
>   (if (FLOAT_TYPE_P (type)
>        && ! HONOR_NANS (type)
> -      && ! HONOR_INFINITIES (element_mode (type)))
> +      && ! HONOR_INFINITIES (type))
>    { build_one_cst (type); }))
>
>  /* Optimize -A / A to -1.0 if we don't care about
> @@ -174,19 +174,19 @@
>   (rdiv:c @0 (negate @0))
>   (if (FLOAT_TYPE_P (type)
>        && ! HONOR_NANS (type)
> -      && ! HONOR_INFINITIES (element_mode (type)))
> +      && ! HONOR_INFINITIES (type))
>    { build_minus_one_cst (type); }))
>
>  /* In IEEE floating point, x/1 is not equivalent to x for snans.  */
>  (simplify
>   (rdiv @0 real_onep)
> - (if (!HONOR_SNANS (element_mode (type)))
> + (if (!HONOR_SNANS (type))
>    (non_lvalue @0)))
>
>  /* In IEEE floating point, x/-1 is not equivalent to -x for snans.  */
>  (simplify
>   (rdiv @0 real_minus_onep)
> - (if (!HONOR_SNANS (element_mode (type)))
> + (if (!HONOR_SNANS (type))
>    (negate @0)))
>
>  /* If ARG1 is a constant, we can convert this to a multiply by the
> @@ -297,9 +297,10 @@
>    @1)
>
>  /* ~x | x -> -1 */

Please also adjust this comment.  Ok with that change.

Thanks,
Richard.

> -(simplify
> - (bit_ior:c (convert? @0) (convert? (bit_not @0)))
> - (convert { build_all_ones_cst (TREE_TYPE (@0)); }))
> +(for op (bit_ior bit_xor plus)
> + (simplify
> +  (op:c (convert? @0) (convert? (bit_not @0)))
> +  (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
>
>  /* x ^ x -> 0 */
>  (simplify
> @@ -311,11 +312,6 @@
>    (bit_xor @0 integer_all_onesp@1)
>    (bit_not @0))
>
> -/* ~X ^ X is -1.  */
> -(simplify
> - (bit_xor:c (bit_not @0) @0)
> - { build_all_ones_cst (type); })
> -
>  /* x & ~0 -> x  */
>  (simplify
>   (bit_and @0 integer_all_onesp)
> @@ -603,11 +599,11 @@
>  (simplify
>   (bit_not (convert? (negate @0)))
>   (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
> -  (convert (minus @0 { build_one_cst (TREE_TYPE (@0)); }))))
> +  (convert (minus @0 { build_each_one_cst (TREE_TYPE (@0)); }))))
>
>  /* Convert ~ (A - 1) or ~ (A + -1) to -A.  */
>  (simplify
> - (bit_not (convert? (minus @0 integer_onep)))
> + (bit_not (convert? (minus @0 integer_each_onep)))
>   (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
>    (convert (negate @0))))
>  (simplify
> Index: tree.c
> ===================================================================
> --- tree.c      (revision 225411)
> +++ tree.c      (working copy)
> @@ -1968,6 +1968,21 @@
>    return t;
>  }
>
> +/* Return the constant 1 in type TYPE.  If TYPE has several elements, each
> +   element is set to 1.  In particular, this is 1 + i for complex types.
> */
> +
> +tree
> +build_each_one_cst (tree type)
> +{
> +  if (TREE_CODE (type) == COMPLEX_TYPE)
> +    {
> +      tree scalar = build_one_cst (TREE_TYPE (type));
> +      return build_complex (type, scalar, scalar);
> +    }
> +  else
> +    return build_one_cst (type);
> +}
> +
>  /* Return a constant of arithmetic type TYPE which is the
>     multiplicative identity of the set TYPE.  */
>
> Index: tree.h
> ===================================================================
> --- tree.h      (revision 225411)
> +++ tree.h      (working copy)
> @@ -3772,6 +3772,7 @@
>  extern tree build_constructor_va (tree, int, ...);
>  extern tree build_real_from_int_cst (tree, const_tree);
>  extern tree build_complex (tree, tree, tree);
> +extern tree build_each_one_cst (tree);
>  extern tree build_one_cst (tree);
>  extern tree build_minus_one_cst (tree);
>  extern tree build_all_ones_cst (tree);
>
Richard Biener July 7, 2015, 10:01 a.m. UTC | #2
On Mon, Jul 6, 2015 at 4:08 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Sat, Jul 4, 2015 at 4:34 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hello,
>>
>> these are just some minor changes. I believe I had already promised a build_
>> function to match integer_each_onep.
>>
>> Bootstrap+testsuite on powerpc64le-unknown-linux-gnu (it looks like
>> *-match.c takes about 10 minutes to compile in stage2 these days).
>
> Ouch.  I have some changes to the code generation in the queue which
> also supports a more natural "if" structure (else and elif).  Eventually
> that helps a bit but I suppose the main issue is simply from the large
> functions.  They can be split quite easily I think, but passing down
> all relevant state might turn out to be tricky unless we start using
> nested functions here ... (and IIRC those are not supported in C++)

Just checking in my dev tree (-O0 build with checking enabled, thus
similar to the stage2 situation) reveals nothing interesting.  The checkers
take up most of the time:

 CFG verifier            :  21.27 ( 8%) usr   0.01 ( 1%) sys  21.46 (
8%) wall       0 kB ( 0%) ggc
early inlining heuristics:  12.59 ( 5%) usr   0.03 ( 2%) sys  12.61 (
5%) wall   10826 kB ( 1%) ggc
tree SSA verifier       :  26.30 (10%) usr   0.01 ( 1%) sys  26.34
(10%) wall       0 kB ( 0%) ggc
 tree STMT verifier      :  50.44 (20%) usr   0.10 ( 6%) sys  50.27
(20%) wall       0 kB ( 0%) ggc

that's everything >= 5%

Trying to figure out if there is some gross algorithms in here (yes, we now
verify stuff quite often...)

Richard.

>
>
> Richard.
>
>> 2015-07-06  Marc Glisse  <marc.glisse@inria.fr>
>>
>>         * match.pd: Remove element_mode inside HONOR_*.
>>         (~ (-A) -> A - 1, ~ (A - 1) -> -A): Handle complex types.
>>         (~X | X -> -1, ~X ^ X -> -1): Merge.
>>         * tree.c (build_each_one_cst): New function.
>>         * tree.h (build_each_one_cst): Likewise.
>>
>> --
>> Marc Glisse
>> Index: match.pd
>> ===================================================================
>> --- match.pd    (revision 225411)
>> +++ match.pd    (working copy)
>> @@ -101,7 +101,7 @@
>>     negative value by 0 gives -0, not +0.  */
>>  (simplify
>>   (mult @0 real_zerop@1)
>> - (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (element_mode (type)))
>> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
>>    @1))
>>
>>  /* In IEEE floating point, x*1 is not equivalent to x for snans.
>> @@ -108,8 +108,8 @@
>>     Likewise for complex arithmetic with signed zeros.  */
>>  (simplify
>>   (mult @0 real_onep)
>> - (if (!HONOR_SNANS (element_mode (type))
>> -      && (!HONOR_SIGNED_ZEROS (element_mode (type))
>> + (if (!HONOR_SNANS (type)
>> +      && (!HONOR_SIGNED_ZEROS (type)
>>            || !COMPLEX_FLOAT_TYPE_P (type)))
>>    (non_lvalue @0)))
>>
>> @@ -116,8 +116,8 @@
>>  /* Transform x * -1.0 into -x.  */
>>  (simplify
>>   (mult @0 real_minus_onep)
>> -  (if (!HONOR_SNANS (element_mode (type))
>> -       && (!HONOR_SIGNED_ZEROS (element_mode (type))
>> +  (if (!HONOR_SNANS (type)
>> +       && (!HONOR_SIGNED_ZEROS (type)
>>             || !COMPLEX_FLOAT_TYPE_P (type)))
>>     (negate @0)))
>>
>> @@ -165,7 +165,7 @@
>>   (rdiv @0 @0)
>>   (if (FLOAT_TYPE_P (type)
>>        && ! HONOR_NANS (type)
>> -      && ! HONOR_INFINITIES (element_mode (type)))
>> +      && ! HONOR_INFINITIES (type))
>>    { build_one_cst (type); }))
>>
>>  /* Optimize -A / A to -1.0 if we don't care about
>> @@ -174,19 +174,19 @@
>>   (rdiv:c @0 (negate @0))
>>   (if (FLOAT_TYPE_P (type)
>>        && ! HONOR_NANS (type)
>> -      && ! HONOR_INFINITIES (element_mode (type)))
>> +      && ! HONOR_INFINITIES (type))
>>    { build_minus_one_cst (type); }))
>>
>>  /* In IEEE floating point, x/1 is not equivalent to x for snans.  */
>>  (simplify
>>   (rdiv @0 real_onep)
>> - (if (!HONOR_SNANS (element_mode (type)))
>> + (if (!HONOR_SNANS (type))
>>    (non_lvalue @0)))
>>
>>  /* In IEEE floating point, x/-1 is not equivalent to -x for snans.  */
>>  (simplify
>>   (rdiv @0 real_minus_onep)
>> - (if (!HONOR_SNANS (element_mode (type)))
>> + (if (!HONOR_SNANS (type))
>>    (negate @0)))
>>
>>  /* If ARG1 is a constant, we can convert this to a multiply by the
>> @@ -297,9 +297,10 @@
>>    @1)
>>
>>  /* ~x | x -> -1 */
>
> Please also adjust this comment.  Ok with that change.
>
> Thanks,
> Richard.
>
>> -(simplify
>> - (bit_ior:c (convert? @0) (convert? (bit_not @0)))
>> - (convert { build_all_ones_cst (TREE_TYPE (@0)); }))
>> +(for op (bit_ior bit_xor plus)
>> + (simplify
>> +  (op:c (convert? @0) (convert? (bit_not @0)))
>> +  (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
>>
>>  /* x ^ x -> 0 */
>>  (simplify
>> @@ -311,11 +312,6 @@
>>    (bit_xor @0 integer_all_onesp@1)
>>    (bit_not @0))
>>
>> -/* ~X ^ X is -1.  */
>> -(simplify
>> - (bit_xor:c (bit_not @0) @0)
>> - { build_all_ones_cst (type); })
>> -
>>  /* x & ~0 -> x  */
>>  (simplify
>>   (bit_and @0 integer_all_onesp)
>> @@ -603,11 +599,11 @@
>>  (simplify
>>   (bit_not (convert? (negate @0)))
>>   (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
>> -  (convert (minus @0 { build_one_cst (TREE_TYPE (@0)); }))))
>> +  (convert (minus @0 { build_each_one_cst (TREE_TYPE (@0)); }))))
>>
>>  /* Convert ~ (A - 1) or ~ (A + -1) to -A.  */
>>  (simplify
>> - (bit_not (convert? (minus @0 integer_onep)))
>> + (bit_not (convert? (minus @0 integer_each_onep)))
>>   (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
>>    (convert (negate @0))))
>>  (simplify
>> Index: tree.c
>> ===================================================================
>> --- tree.c      (revision 225411)
>> +++ tree.c      (working copy)
>> @@ -1968,6 +1968,21 @@
>>    return t;
>>  }
>>
>> +/* Return the constant 1 in type TYPE.  If TYPE has several elements, each
>> +   element is set to 1.  In particular, this is 1 + i for complex types.
>> */
>> +
>> +tree
>> +build_each_one_cst (tree type)
>> +{
>> +  if (TREE_CODE (type) == COMPLEX_TYPE)
>> +    {
>> +      tree scalar = build_one_cst (TREE_TYPE (type));
>> +      return build_complex (type, scalar, scalar);
>> +    }
>> +  else
>> +    return build_one_cst (type);
>> +}
>> +
>>  /* Return a constant of arithmetic type TYPE which is the
>>     multiplicative identity of the set TYPE.  */
>>
>> Index: tree.h
>> ===================================================================
>> --- tree.h      (revision 225411)
>> +++ tree.h      (working copy)
>> @@ -3772,6 +3772,7 @@
>>  extern tree build_constructor_va (tree, int, ...);
>>  extern tree build_real_from_int_cst (tree, const_tree);
>>  extern tree build_complex (tree, tree, tree);
>> +extern tree build_each_one_cst (tree);
>>  extern tree build_one_cst (tree);
>>  extern tree build_minus_one_cst (tree);
>>  extern tree build_all_ones_cst (tree);
>>
Eric Botcazou July 7, 2015, 10:48 a.m. UTC | #3
> Bootstrap+testsuite on powerpc64le-unknown-linux-gnu (it looks like
> *-match.c takes about 10 minutes to compile in stage2 these days).

Yeah, it has already taken back all the speedup brought by the rewrite of the 
RTL gen* stuff by Richard S. :-(
Richard Biener July 7, 2015, 10:54 a.m. UTC | #4
On Tue, Jul 7, 2015 at 12:48 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Bootstrap+testsuite on powerpc64le-unknown-linux-gnu (it looks like
>> *-match.c takes about 10 minutes to compile in stage2 these days).
>
> Yeah, it has already taken back all the speedup brought by the rewrite of the
> RTL gen* stuff by Richard S. :-(

And it's going to get worse (read: larger).  Looking at the
time-report data I don't
think splitting into multiple functions will help though.  Splitting
into multiple
files would allow to parallelize the build at least.

I'm gathering a profile to see where all the time in the checking stuff goes.

As said, one code generation arrangement that is on my TODO list will
remove some code duplication, but I'm not sure it will make a big
enough difference.

Richard.

> --
> Eric Botcazou
diff mbox

Patch

Index: match.pd
===================================================================
--- match.pd	(revision 225411)
+++ match.pd	(working copy)
@@ -101,7 +101,7 @@ 
    negative value by 0 gives -0, not +0.  */
 (simplify
  (mult @0 real_zerop@1)
- (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (element_mode (type)))
+ (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
   @1))
 
 /* In IEEE floating point, x*1 is not equivalent to x for snans.
@@ -108,8 +108,8 @@ 
    Likewise for complex arithmetic with signed zeros.  */
 (simplify
  (mult @0 real_onep)
- (if (!HONOR_SNANS (element_mode (type))
-      && (!HONOR_SIGNED_ZEROS (element_mode (type))
+ (if (!HONOR_SNANS (type)
+      && (!HONOR_SIGNED_ZEROS (type)
           || !COMPLEX_FLOAT_TYPE_P (type)))
   (non_lvalue @0)))
 
@@ -116,8 +116,8 @@ 
 /* Transform x * -1.0 into -x.  */
 (simplify
  (mult @0 real_minus_onep)
-  (if (!HONOR_SNANS (element_mode (type))
-       && (!HONOR_SIGNED_ZEROS (element_mode (type))
+  (if (!HONOR_SNANS (type)
+       && (!HONOR_SIGNED_ZEROS (type)
            || !COMPLEX_FLOAT_TYPE_P (type)))
    (negate @0)))
 
@@ -165,7 +165,7 @@ 
  (rdiv @0 @0)
  (if (FLOAT_TYPE_P (type)
       && ! HONOR_NANS (type)
-      && ! HONOR_INFINITIES (element_mode (type)))
+      && ! HONOR_INFINITIES (type))
   { build_one_cst (type); }))
 
 /* Optimize -A / A to -1.0 if we don't care about
@@ -174,19 +174,19 @@ 
  (rdiv:c @0 (negate @0))
  (if (FLOAT_TYPE_P (type)
       && ! HONOR_NANS (type)
-      && ! HONOR_INFINITIES (element_mode (type)))
+      && ! HONOR_INFINITIES (type))
   { build_minus_one_cst (type); }))
 
 /* In IEEE floating point, x/1 is not equivalent to x for snans.  */
 (simplify
  (rdiv @0 real_onep)
- (if (!HONOR_SNANS (element_mode (type)))
+ (if (!HONOR_SNANS (type))
   (non_lvalue @0)))
 
 /* In IEEE floating point, x/-1 is not equivalent to -x for snans.  */
 (simplify
  (rdiv @0 real_minus_onep)
- (if (!HONOR_SNANS (element_mode (type)))
+ (if (!HONOR_SNANS (type))
   (negate @0)))
 
 /* If ARG1 is a constant, we can convert this to a multiply by the
@@ -297,9 +297,10 @@ 
   @1)
 
 /* ~x | x -> -1 */
-(simplify
- (bit_ior:c (convert? @0) (convert? (bit_not @0)))
- (convert { build_all_ones_cst (TREE_TYPE (@0)); }))
+(for op (bit_ior bit_xor plus)
+ (simplify
+  (op:c (convert? @0) (convert? (bit_not @0)))
+  (convert { build_all_ones_cst (TREE_TYPE (@0)); })))
 
 /* x ^ x -> 0 */
 (simplify
@@ -311,11 +312,6 @@ 
   (bit_xor @0 integer_all_onesp@1)
   (bit_not @0))
 
-/* ~X ^ X is -1.  */
-(simplify
- (bit_xor:c (bit_not @0) @0)
- { build_all_ones_cst (type); })
-
 /* x & ~0 -> x  */
 (simplify
  (bit_and @0 integer_all_onesp)
@@ -603,11 +599,11 @@ 
 (simplify
  (bit_not (convert? (negate @0)))
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
-  (convert (minus @0 { build_one_cst (TREE_TYPE (@0)); }))))
+  (convert (minus @0 { build_each_one_cst (TREE_TYPE (@0)); }))))
 
 /* Convert ~ (A - 1) or ~ (A + -1) to -A.  */
 (simplify
- (bit_not (convert? (minus @0 integer_onep)))
+ (bit_not (convert? (minus @0 integer_each_onep)))
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
   (convert (negate @0))))
 (simplify
Index: tree.c
===================================================================
--- tree.c	(revision 225411)
+++ tree.c	(working copy)
@@ -1968,6 +1968,21 @@ 
   return t;
 }
 
+/* Return the constant 1 in type TYPE.  If TYPE has several elements, each
+   element is set to 1.  In particular, this is 1 + i for complex types.  */
+
+tree
+build_each_one_cst (tree type)
+{
+  if (TREE_CODE (type) == COMPLEX_TYPE)
+    {
+      tree scalar = build_one_cst (TREE_TYPE (type));
+      return build_complex (type, scalar, scalar);
+    }
+  else
+    return build_one_cst (type);
+}
+
 /* Return a constant of arithmetic type TYPE which is the
    multiplicative identity of the set TYPE.  */
 
Index: tree.h
===================================================================
--- tree.h	(revision 225411)
+++ tree.h	(working copy)
@@ -3772,6 +3772,7 @@ 
 extern tree build_constructor_va (tree, int, ...);
 extern tree build_real_from_int_cst (tree, const_tree);
 extern tree build_complex (tree, tree, tree);
+extern tree build_each_one_cst (tree);
 extern tree build_one_cst (tree);
 extern tree build_minus_one_cst (tree);
 extern tree build_all_ones_cst (tree);