diff mbox series

[committed] match.pd: Relax rule to include POLY_INT_CSTs

Message ID mptim1lt44z.fsf@arm.com
State New
Headers show
Series [committed] match.pd: Relax rule to include POLY_INT_CSTs | expand

Commit Message

Richard Sandiford July 8, 2021, 11:51 a.m. UTC
match.pd has a rule to simplify an extension, operation and truncation
back to the original type:

 (simplify
   (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))

Currently it handles cases in which @2 is an INTEGER_CST, but it
also works for POLY_INT_CSTs.[*]

For INTEGER_CST it doesn't matter whether we test @2 or @4,
but for POLY_INT_CST it is possible to have unfolded (convert …)s.

Originally I saw this leading to some bad ivopts decisions, because
we weren't folding away redundancies from candidate iv expressions.
It's also possible to test the fold directly using the SVE ACLE.

Tested on aarch64-linux-gnu and x86_64-linux-gnu, pushed as obvious.

Richard

[*] Not all INTEGER_CST rules work for POLY_INT_CSTs, since extensions
    don't necessarily distribute over the internals of the POLY_INT_CST.
    But in this case that isn't an issue.


gcc/
	* match.pd: Simplify an extend-operate-truncate sequence involving
	a POLY_INT_CST.

gcc/testsuite/
	* gcc.target/aarch64/sve/acle/general/cntb_1.c: New test.
---
 gcc/match.pd                                       |  2 +-
 .../gcc.target/aarch64/sve/acle/general/cntb_1.c   | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c

Comments

Richard Biener July 8, 2021, 12:17 p.m. UTC | #1
On Thu, Jul 8, 2021 at 1:52 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> match.pd has a rule to simplify an extension, operation and truncation
> back to the original type:
>
>  (simplify
>    (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
>
> Currently it handles cases in which @2 is an INTEGER_CST, but it
> also works for POLY_INT_CSTs.[*]
>
> For INTEGER_CST it doesn't matter whether we test @2 or @4,
> but for POLY_INT_CST it is possible to have unfolded (convert …)s.

But if it is an unfolded conversion then @4 is the conversion and of
course not POLY_INT_CST_P, so I'm not sure what you says makes
sense.  But maybe you want to _not_ simplify the unfolded
conversion case?

> Originally I saw this leading to some bad ivopts decisions, because
> we weren't folding away redundancies from candidate iv expressions.
> It's also possible to test the fold directly using the SVE ACLE.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu, pushed as obvious.
>
> Richard
>
> [*] Not all INTEGER_CST rules work for POLY_INT_CSTs, since extensions
>     don't necessarily distribute over the internals of the POLY_INT_CST.
>     But in this case that isn't an issue.
>
>
> gcc/
>         * match.pd: Simplify an extend-operate-truncate sequence involving
>         a POLY_INT_CST.
>
> gcc/testsuite/
>         * gcc.target/aarch64/sve/acle/general/cntb_1.c: New test.
> ---
>  gcc/match.pd                                       |  2 +-
>  .../gcc.target/aarch64/sve/acle/general/cntb_1.c   | 14 ++++++++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 334e8cc0496..30680d488ab 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -6175,7 +6175,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>         && (types_match (@1, @2)
>             /* Or the second operand is const integer or converted const
>                integer from valueize.  */
> -           || TREE_CODE (@2) == INTEGER_CST))
> +           || poly_int_tree_p (@4)))
>       (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
>         (op @1 (convert @2))
>         (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
> new file mode 100644
> index 00000000000..b43fcf0ed6d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
> @@ -0,0 +1,14 @@
> +/* { dg-options "-O -fdump-tree-optimized" } */
> +
> +#include <arm_sve.h>
> +
> +unsigned int
> +foo (unsigned int x)
> +{
> +  unsigned long tmp = x;
> +  tmp += svcntb ();
> +  x = tmp;
> +  return x - svcntb ();
> +}
> +
> +/* { dg-final { scan-tree-dump-not { POLY_INT_CST } optimized } } */
> --
> 2.17.1
>
Richard Sandiford July 8, 2021, 12:30 p.m. UTC | #2
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Thu, Jul 8, 2021 at 1:52 PM Richard Sandiford via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> match.pd has a rule to simplify an extension, operation and truncation
>> back to the original type:
>>
>>  (simplify
>>    (convert (op:s@0 (convert1?@3 @1) (convert2?@4 @2)))
>>
>> Currently it handles cases in which @2 is an INTEGER_CST, but it
>> also works for POLY_INT_CSTs.[*]
>>
>> For INTEGER_CST it doesn't matter whether we test @2 or @4,
>> but for POLY_INT_CST it is possible to have unfolded (convert …)s.
>
> But if it is an unfolded conversion then @4 is the conversion and of
> course not POLY_INT_CST_P, so I'm not sure what you says makes
> sense.  But maybe you want to _not_ simplify the unfolded
> conversion case?

Yeah, exactly that.  Extensions of POLY_INT_CSTs won't be folded because
extension doesn't distribute over (modulo) +.  If an unfolded POLY_INT_CST
has the same type as @1 then the match will succeed thanks to
types_match (@1, @2).  So the new pattern handles both that case
and the case in which POLY_INT_CST occurs without a conversion.

If an unfolded POLY_INT_CST has a different type from @1 then we'd need
a more complicated check for validity.  Maybe that would be useful,
but it would no longer be a one-line change :-)

Thanks,
Richard

>
>> Originally I saw this leading to some bad ivopts decisions, because
>> we weren't folding away redundancies from candidate iv expressions.
>> It's also possible to test the fold directly using the SVE ACLE.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu, pushed as obvious.
>>
>> Richard
>>
>> [*] Not all INTEGER_CST rules work for POLY_INT_CSTs, since extensions
>>     don't necessarily distribute over the internals of the POLY_INT_CST.
>>     But in this case that isn't an issue.
>>
>>
>> gcc/
>>         * match.pd: Simplify an extend-operate-truncate sequence involving
>>         a POLY_INT_CST.
>>
>> gcc/testsuite/
>>         * gcc.target/aarch64/sve/acle/general/cntb_1.c: New test.
>> ---
>>  gcc/match.pd                                       |  2 +-
>>  .../gcc.target/aarch64/sve/acle/general/cntb_1.c   | 14 ++++++++++++++
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
>>
>> diff --git a/gcc/match.pd b/gcc/match.pd
>> index 334e8cc0496..30680d488ab 100644
>> --- a/gcc/match.pd
>> +++ b/gcc/match.pd
>> @@ -6175,7 +6175,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>>         && (types_match (@1, @2)
>>             /* Or the second operand is const integer or converted const
>>                integer from valueize.  */
>> -           || TREE_CODE (@2) == INTEGER_CST))
>> +           || poly_int_tree_p (@4)))
>>       (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
>>         (op @1 (convert @2))
>>         (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
>> new file mode 100644
>> index 00000000000..b43fcf0ed6d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
>> @@ -0,0 +1,14 @@
>> +/* { dg-options "-O -fdump-tree-optimized" } */
>> +
>> +#include <arm_sve.h>
>> +
>> +unsigned int
>> +foo (unsigned int x)
>> +{
>> +  unsigned long tmp = x;
>> +  tmp += svcntb ();
>> +  x = tmp;
>> +  return x - svcntb ();
>> +}
>> +
>> +/* { dg-final { scan-tree-dump-not { POLY_INT_CST } optimized } } */
>> --
>> 2.17.1
>>
diff mbox series

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 334e8cc0496..30680d488ab 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -6175,7 +6175,7 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	&& (types_match (@1, @2)
 	    /* Or the second operand is const integer or converted const
 	       integer from valueize.  */
-	    || TREE_CODE (@2) == INTEGER_CST))
+	    || poly_int_tree_p (@4)))
      (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
        (op @1 (convert @2))
        (with { tree utype = unsigned_type_for (TREE_TYPE (@1)); }
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
new file mode 100644
index 00000000000..b43fcf0ed6d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/cntb_1.c
@@ -0,0 +1,14 @@ 
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+#include <arm_sve.h>
+
+unsigned int
+foo (unsigned int x)
+{
+  unsigned long tmp = x;
+  tmp += svcntb ();
+  x = tmp;
+  return x - svcntb ();
+}
+
+/* { dg-final { scan-tree-dump-not { POLY_INT_CST } optimized } } */