Message ID | mptim1lt44z.fsf@arm.com |
---|---|
State | New |
Headers | show |
Series | [committed] match.pd: Relax rule to include POLY_INT_CSTs | expand |
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 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 --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 } } */