* [PATCH] match.pd: Add new abs pattern [PR94290] @ 2022-07-13 19:24 Sam Feifer 2022-07-13 19:35 ` Andrew Pinski 0 siblings, 1 reply; 11+ messages in thread From: Sam Feifer @ 2022-07-13 19:24 UTC (permalink / raw) To: gcc-patches This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. Tests are also included to be added to the testsuite. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR tree-optimization/94290 gcc/ChangeLog: * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. * match.pd (x <= 0 ? -x : 0): New Simplification. gcc/testsuite/ChangeLog: * gcc.c-torture/execute/pr94290-1.c: New test. * gcc.dg/pr94290-2.c: New test. * gcc.dg/pr94290.c: New test. --- gcc/match.pd | 15 ++++++ .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c create mode 100644 gcc/testsuite/gcc.dg/pr94290.c diff --git a/gcc/match.pd b/gcc/match.pd index 45aefd96688..55ca79d7ac9 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7848,3 +7848,18 @@ and, (if (TYPE_UNSIGNED (TREE_TYPE (@0))) (bit_and @0 @1) (cond (le @0 @1) @0 (bit_and @0 @1)))))) + +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ +(simplify + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) + (abs @0)) + +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ +(simplify + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) + (abs @0)) + +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) + (max (negate @0) @1)) diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c new file mode 100644 index 00000000000..93b80d569aa --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c @@ -0,0 +1,16 @@ +/* PR tree-optimization/94290 */ + +#include "../../gcc.dg/pr94290.c" + +int main() { + + if (foo(0) != 0 + || foo(-42) != 42 + || foo(42) != 42 + || baz(-10) != 10 + || baz(-10) != 10) { + __builtin_abort(); + } + + return 0; +} diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c new file mode 100644 index 00000000000..ea6e55755f5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94290-2.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/94290 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +/* Form from PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return x <= 0 ? -x : 0; +} + +/* Changed order. */ +__attribute__((noipa)) unsigned int bar(int x) { + return 0 >= x ? -x : 0; +} + +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c new file mode 100644 index 00000000000..47617c36c02 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94290.c @@ -0,0 +1,46 @@ +/* PR tree-optimization/94290 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + + +/* Same form as PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Signed function. */ +__attribute__((noipa)) int bar(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Commutative property. */ +__attribute__((noipa)) unsigned int baz(int x) { + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); +} + +/* Flipped order for max expressions. */ +__attribute__((noipa)) unsigned int quux(int x) { + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int waldo(int x) { + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int fred(int x) { + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) unsigned int goo(int x) { + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) int qux(int x) { + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); +} + +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ base-commit: 6af530f914801f5e561057da55c41480f28751f7 -- 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-13 19:24 [PATCH] match.pd: Add new abs pattern [PR94290] Sam Feifer @ 2022-07-13 19:35 ` Andrew Pinski 2022-07-14 6:52 ` Richard Biener 2022-07-14 14:09 ` Sam Feifer 0 siblings, 2 replies; 11+ messages in thread From: Andrew Pinski @ 2022-07-13 19:35 UTC (permalink / raw) To: Sam Feifer; +Cc: GCC Patches On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. You could use :c for the commutative property instead and that should simplify things. That is: (simplify (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) (abs @0)) Also since integer_zerop works on vectors, it seems like you should add a testcase or two for the vector case. Also would be useful if you write a testcase that uses different statements rather than one big one so it gets exercised in the forwprop case. Note also if either of the max are used more than just in this simplification, it could increase the lifetime of @0, maybe you need to add :s to the max expressions. Thanks, Andrew > > Tests are also included to be added to the testsuite. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR tree-optimization/94290 > > gcc/ChangeLog: > > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. > * match.pd (x <= 0 ? -x : 0): New Simplification. > > gcc/testsuite/ChangeLog: > > * gcc.c-torture/execute/pr94290-1.c: New test. > * gcc.dg/pr94290-2.c: New test. > * gcc.dg/pr94290.c: New test. > --- > gcc/match.pd | 15 ++++++ > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ > 4 files changed, 92 insertions(+) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 45aefd96688..55ca79d7ac9 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -7848,3 +7848,18 @@ and, > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > (bit_and @0 @1) > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > + > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > +(simplify > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > + (abs @0)) > + > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > +(simplify > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > + (abs @0)) > + > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > +(simplify > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > + (max (negate @0) @1)) > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > new file mode 100644 > index 00000000000..93b80d569aa > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > @@ -0,0 +1,16 @@ > +/* PR tree-optimization/94290 */ > + > +#include "../../gcc.dg/pr94290.c" > + > +int main() { > + > + if (foo(0) != 0 > + || foo(-42) != 42 > + || foo(42) != 42 > + || baz(-10) != 10 > + || baz(-10) != 10) { > + __builtin_abort(); > + } > + > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c > new file mode 100644 > index 00000000000..ea6e55755f5 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > @@ -0,0 +1,15 @@ > +/* PR tree-optimization/94290 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > + > +/* Form from PR. */ > +__attribute__((noipa)) unsigned int foo(int x) { > + return x <= 0 ? -x : 0; > +} > + > +/* Changed order. */ > +__attribute__((noipa)) unsigned int bar(int x) { > + return 0 >= x ? -x : 0; > +} > + > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c > new file mode 100644 > index 00000000000..47617c36c02 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr94290.c > @@ -0,0 +1,46 @@ > +/* PR tree-optimization/94290 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > + > + > +/* Same form as PR. */ > +__attribute__((noipa)) unsigned int foo(int x) { > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > +} > + > +/* Signed function. */ > +__attribute__((noipa)) int bar(int x) { > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > +} > + > +/* Commutative property. */ > +__attribute__((noipa)) unsigned int baz(int x) { > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > +} > + > +/* Flipped order for max expressions. */ > +__attribute__((noipa)) unsigned int quux(int x) { > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > +} > + > +/* Not zero so should not optimize. */ > +__attribute__((noipa)) unsigned int waldo(int x) { > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > +} > + > +/* Not zero so should not optimize. */ > +__attribute__((noipa)) unsigned int fred(int x) { > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > +} > + > +/* Incorrect pattern. */ > +__attribute__((noipa)) unsigned int goo(int x) { > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > +} > + > +/* Incorrect pattern. */ > +__attribute__((noipa)) int qux(int x) { > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > +} > + > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-13 19:35 ` Andrew Pinski @ 2022-07-14 6:52 ` Richard Biener 2022-07-14 14:09 ` Sam Feifer 1 sibling, 0 replies; 11+ messages in thread From: Richard Biener @ 2022-07-14 6:52 UTC (permalink / raw) To: Andrew Pinski; +Cc: Sam Feifer, GCC Patches On Wed, Jul 13, 2022 at 9:36 PM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. > > You could use :c for the commutative property instead and that should > simplify things. > That is: > > (simplify > (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > (abs @0)) > > Also since integer_zerop works on vectors, it seems like you should > add a testcase or two for the vector case. > Also would be useful if you write a testcase that uses different > statements rather than one big one so it gets exercised in the > forwprop case. > Note also if either of the max are used more than just in this > simplification, it could increase the lifetime of @0, maybe you need > to add :s to the max expressions. Note :s will be ineffective since the result is "simple", I think it is OK to do this simplification even when the max are not dead after the transform. > > Thanks, > Andrew > > > > > Tests are also included to be added to the testsuite. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > PR tree-optimization/94290 > > > > gcc/ChangeLog: > > > > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. > > * match.pd (x <= 0 ? -x : 0): New Simplification. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.c-torture/execute/pr94290-1.c: New test. > > * gcc.dg/pr94290-2.c: New test. > > * gcc.dg/pr94290.c: New test. > > --- > > gcc/match.pd | 15 ++++++ > > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ > > 4 files changed, 92 insertions(+) > > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 45aefd96688..55ca79d7ac9 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7848,3 +7848,18 @@ and, > > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > > (bit_and @0 @1) > > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > > + > > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > > +(simplify > > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > > +(simplify > > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > > +(simplify > > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > > + (max (negate @0) @1)) > > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > new file mode 100644 > > index 00000000000..93b80d569aa > > --- /dev/null > > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > @@ -0,0 +1,16 @@ > > +/* PR tree-optimization/94290 */ > > + > > +#include "../../gcc.dg/pr94290.c" > > + > > +int main() { > > + > > + if (foo(0) != 0 > > + || foo(-42) != 42 > > + || foo(42) != 42 > > + || baz(-10) != 10 > > + || baz(-10) != 10) { > > + __builtin_abort(); > > + } > > + > > + return 0; > > +} > > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c > > new file mode 100644 > > index 00000000000..ea6e55755f5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > > @@ -0,0 +1,15 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > +/* Form from PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return x <= 0 ? -x : 0; > > +} > > + > > +/* Changed order. */ > > +__attribute__((noipa)) unsigned int bar(int x) { > > + return 0 >= x ? -x : 0; > > +} > > + > > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c > > new file mode 100644 > > index 00000000000..47617c36c02 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290.c > > @@ -0,0 +1,46 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > + > > +/* Same form as PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Signed function. */ > > +__attribute__((noipa)) int bar(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Commutative property. */ > > +__attribute__((noipa)) unsigned int baz(int x) { > > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* Flipped order for max expressions. */ > > +__attribute__((noipa)) unsigned int quux(int x) { > > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int waldo(int x) { > > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int fred(int x) { > > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) unsigned int goo(int x) { > > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) int qux(int x) { > > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > > > > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > > -- > > 2.31.1 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-13 19:35 ` Andrew Pinski 2022-07-14 6:52 ` Richard Biener @ 2022-07-14 14:09 ` Sam Feifer 2022-07-14 17:24 ` Andrew Pinski 1 sibling, 1 reply; 11+ messages in thread From: Sam Feifer @ 2022-07-14 14:09 UTC (permalink / raw) To: Andrew Pinski; +Cc: GCC Patches On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: > On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > This patch is intended to fix a missed optimization in match.pd. It > optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > write a second simplification in match.pd to handle the commutative > property as the match was not ocurring otherwise. Additionally, the pattern > (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > other simplification rule. > > You could use :c for the commutative property instead and that should > simplify things. > That is: > > (simplify > (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > (abs @0)) > > Also since integer_zerop works on vectors, it seems like you should > add a testcase or two for the vector case. > Also would be useful if you write a testcase that uses different > statements rather than one big one so it gets exercised in the > forwprop case. > Note also if either of the max are used more than just in this > simplification, it could increase the lifetime of @0, maybe you need > to add :s to the max expressions. > > Thanks for the feedback. I'm not quite sure what a vector test case would look like for this. Could I get some guidance on that? Thanks -Sam > Thanks, > Andrew > > > > > Tests are also included to be added to the testsuite. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > PR tree-optimization/94290 > > > > gcc/ChangeLog: > > > > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > simplification. > > * match.pd (x <= 0 ? -x : 0): New Simplification. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.c-torture/execute/pr94290-1.c: New test. > > * gcc.dg/pr94290-2.c: New test. > > * gcc.dg/pr94290.c: New test. > > --- > > gcc/match.pd | 15 ++++++ > > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ > > 4 files changed, 92 insertions(+) > > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 45aefd96688..55ca79d7ac9 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7848,3 +7848,18 @@ and, > > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > > (bit_and @0 @1) > > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > > + > > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > > +(simplify > > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > > +(simplify > > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > > + (abs @0)) > > + > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > > +(simplify > > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > > + (max (negate @0) @1)) > > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > new file mode 100644 > > index 00000000000..93b80d569aa > > --- /dev/null > > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > @@ -0,0 +1,16 @@ > > +/* PR tree-optimization/94290 */ > > + > > +#include "../../gcc.dg/pr94290.c" > > + > > +int main() { > > + > > + if (foo(0) != 0 > > + || foo(-42) != 42 > > + || foo(42) != 42 > > + || baz(-10) != 10 > > + || baz(-10) != 10) { > > + __builtin_abort(); > > + } > > + > > + return 0; > > +} > > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > b/gcc/testsuite/gcc.dg/pr94290-2.c > > new file mode 100644 > > index 00000000000..ea6e55755f5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > > @@ -0,0 +1,15 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > +/* Form from PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return x <= 0 ? -x : 0; > > +} > > + > > +/* Changed order. */ > > +__attribute__((noipa)) unsigned int bar(int x) { > > + return 0 >= x ? -x : 0; > > +} > > + > > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > b/gcc/testsuite/gcc.dg/pr94290.c > > new file mode 100644 > > index 00000000000..47617c36c02 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr94290.c > > @@ -0,0 +1,46 @@ > > +/* PR tree-optimization/94290 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > + > > + > > +/* Same form as PR. */ > > +__attribute__((noipa)) unsigned int foo(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Signed function. */ > > +__attribute__((noipa)) int bar(int x) { > > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > +} > > + > > +/* Commutative property. */ > > +__attribute__((noipa)) unsigned int baz(int x) { > > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* Flipped order for max expressions. */ > > +__attribute__((noipa)) unsigned int quux(int x) { > > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int waldo(int x) { > > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > > +} > > + > > +/* Not zero so should not optimize. */ > > +__attribute__((noipa)) unsigned int fred(int x) { > > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) unsigned int goo(int x) { > > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > > +} > > + > > +/* Incorrect pattern. */ > > +__attribute__((noipa)) int qux(int x) { > > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > > +} > > + > > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > > > > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > > -- > > 2.31.1 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-14 14:09 ` Sam Feifer @ 2022-07-14 17:24 ` Andrew Pinski 2022-07-14 19:38 ` Sam Feifer 0 siblings, 1 reply; 11+ messages in thread From: Andrew Pinski @ 2022-07-14 17:24 UTC (permalink / raw) To: Sam Feifer; +Cc: GCC Patches On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches >> <gcc-patches@gcc.gnu.org> wrote: >> > >> > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. >> >> You could use :c for the commutative property instead and that should >> simplify things. >> That is: >> >> (simplify >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> (abs @0)) >> >> Also since integer_zerop works on vectors, it seems like you should >> add a testcase or two for the vector case. >> Also would be useful if you write a testcase that uses different >> statements rather than one big one so it gets exercised in the >> forwprop case. >> Note also if either of the max are used more than just in this >> simplification, it could increase the lifetime of @0, maybe you need >> to add :s to the max expressions. >> > > Thanks for the feedback. I'm not quite sure what a vector test case would look like for this. Could I get some guidance on that? Yes this should produce the pattern at forwprop1 time (with the C++ front-end, the C front-end does not support vector selects): typedef int __attribute__((vector_size(4*sizeof(int)))) vint; vint foo(vint x) { vint t = (x >= 0 ? x : 0) ; vint xx = -x; vint t1 = (xx >= 0 ? xx : 0); return t + t1; } int foo(int x) { int t = (x >= 0 ? x : 0) ; int xx = -x; int t1 = (xx >= 0 ? xx : 0); return t + t1; } Thanks, Andrew Pinski > > Thanks > -Sam > >> >> Thanks, >> Andrew >> >> > >> > Tests are also included to be added to the testsuite. >> > >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> > >> > PR tree-optimization/94290 >> > >> > gcc/ChangeLog: >> > >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. >> > * match.pd (x <= 0 ? -x : 0): New Simplification. >> > >> > gcc/testsuite/ChangeLog: >> > >> > * gcc.c-torture/execute/pr94290-1.c: New test. >> > * gcc.dg/pr94290-2.c: New test. >> > * gcc.dg/pr94290.c: New test. >> > --- >> > gcc/match.pd | 15 ++++++ >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ >> > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ >> > 4 files changed, 92 insertions(+) >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c >> > >> > diff --git a/gcc/match.pd b/gcc/match.pd >> > index 45aefd96688..55ca79d7ac9 100644 >> > --- a/gcc/match.pd >> > +++ b/gcc/match.pd >> > @@ -7848,3 +7848,18 @@ and, >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) >> > (bit_and @0 @1) >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) >> > + >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ >> > +(simplify >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> > + (abs @0)) >> > + >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ >> > +(simplify >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) >> > + (abs @0)) >> > + >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ >> > +(simplify >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) >> > + (max (negate @0) @1)) >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> > new file mode 100644 >> > index 00000000000..93b80d569aa >> > --- /dev/null >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> > @@ -0,0 +1,16 @@ >> > +/* PR tree-optimization/94290 */ >> > + >> > +#include "../../gcc.dg/pr94290.c" >> > + >> > +int main() { >> > + >> > + if (foo(0) != 0 >> > + || foo(-42) != 42 >> > + || foo(42) != 42 >> > + || baz(-10) != 10 >> > + || baz(-10) != 10) { >> > + __builtin_abort(); >> > + } >> > + >> > + return 0; >> > +} >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c >> > new file mode 100644 >> > index 00000000000..ea6e55755f5 >> > --- /dev/null >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c >> > @@ -0,0 +1,15 @@ >> > +/* PR tree-optimization/94290 */ >> > +/* { dg-do compile } */ >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> > + >> > +/* Form from PR. */ >> > +__attribute__((noipa)) unsigned int foo(int x) { >> > + return x <= 0 ? -x : 0; >> > +} >> > + >> > +/* Changed order. */ >> > +__attribute__((noipa)) unsigned int bar(int x) { >> > + return 0 >= x ? -x : 0; >> > +} >> > + >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c >> > new file mode 100644 >> > index 00000000000..47617c36c02 >> > --- /dev/null >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c >> > @@ -0,0 +1,46 @@ >> > +/* PR tree-optimization/94290 */ >> > +/* { dg-do compile } */ >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> > + >> > + >> > +/* Same form as PR. */ >> > +__attribute__((noipa)) unsigned int foo(int x) { >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> > +} >> > + >> > +/* Signed function. */ >> > +__attribute__((noipa)) int bar(int x) { >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> > +} >> > + >> > +/* Commutative property. */ >> > +__attribute__((noipa)) unsigned int baz(int x) { >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); >> > +} >> > + >> > +/* Flipped order for max expressions. */ >> > +__attribute__((noipa)) unsigned int quux(int x) { >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); >> > +} >> > + >> > +/* Not zero so should not optimize. */ >> > +__attribute__((noipa)) unsigned int waldo(int x) { >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); >> > +} >> > + >> > +/* Not zero so should not optimize. */ >> > +__attribute__((noipa)) unsigned int fred(int x) { >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); >> > +} >> > + >> > +/* Incorrect pattern. */ >> > +__attribute__((noipa)) unsigned int goo(int x) { >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); >> > +} >> > + >> > +/* Incorrect pattern. */ >> > +__attribute__((noipa)) int qux(int x) { >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); >> > +} >> > + >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ >> > >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 >> > -- >> > 2.31.1 >> > >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-14 17:24 ` Andrew Pinski @ 2022-07-14 19:38 ` Sam Feifer 2022-07-14 19:53 ` Andrew Pinski 0 siblings, 1 reply; 11+ messages in thread From: Sam Feifer @ 2022-07-14 19:38 UTC (permalink / raw) To: Andrew Pinski; +Cc: GCC Patches On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > > > > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: > >> > >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > >> <gcc-patches@gcc.gnu.org> wrote: > >> > > >> > This patch is intended to fix a missed optimization in match.pd. It > optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > write a second simplification in match.pd to handle the commutative > property as the match was not ocurring otherwise. Additionally, the pattern > (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > other simplification rule. > >> > >> You could use :c for the commutative property instead and that should > >> simplify things. > >> That is: > >> > >> (simplify > >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> (abs @0)) > >> > >> Also since integer_zerop works on vectors, it seems like you should > >> add a testcase or two for the vector case. > >> Also would be useful if you write a testcase that uses different > >> statements rather than one big one so it gets exercised in the > >> forwprop case. > >> Note also if either of the max are used more than just in this > >> simplification, it could increase the lifetime of @0, maybe you need > >> to add :s to the max expressions. > >> > > > > Thanks for the feedback. I'm not quite sure what a vector test case > would look like for this. Could I get some guidance on that? > > Yes this should produce the pattern at forwprop1 time (with the C++ > front-end, the C front-end does not support vector selects): > typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > > vint foo(vint x) { > vint t = (x >= 0 ? x : 0) ; > vint xx = -x; > vint t1 = (xx >= 0 ? xx : 0); > return t + t1; > } > > int foo(int x) { > int t = (x >= 0 ? x : 0) ; > int xx = -x; > int t1 = (xx >= 0 ? xx : 0); > return t + t1; > } > > Thanks, > Andrew Pinski > > Thanks for the help. I'm still having trouble with the vector test, though. When I try to compile, I get an error saying "used vector type where scalar is required", referring to the max expressions. How do I use the max expression with two vectors as the inputs? Thanks -Sam > > > > > Thanks > > -Sam > > > >> > >> Thanks, > >> Andrew > >> > >> > > >> > Tests are also included to be added to the testsuite. > >> > > >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > >> > > >> > PR tree-optimization/94290 > >> > > >> > gcc/ChangeLog: > >> > > >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > simplification. > >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > >> > > >> > gcc/testsuite/ChangeLog: > >> > > >> > * gcc.c-torture/execute/pr94290-1.c: New test. > >> > * gcc.dg/pr94290-2.c: New test. > >> > * gcc.dg/pr94290.c: New test. > >> > --- > >> > gcc/match.pd | 15 ++++++ > >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > +++++++++++++++++++ > >> > 4 files changed, 92 insertions(+) > >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > >> > > >> > diff --git a/gcc/match.pd b/gcc/match.pd > >> > index 45aefd96688..55ca79d7ac9 100644 > >> > --- a/gcc/match.pd > >> > +++ b/gcc/match.pd > >> > @@ -7848,3 +7848,18 @@ and, > >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > >> > (bit_and @0 @1) > >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > >> > + > >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > >> > +(simplify > >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> > + (abs @0)) > >> > + > >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > >> > +(simplify > >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > >> > + (abs @0)) > >> > + > >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > >> > +(simplify > >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > >> > + (max (negate @0) @1)) > >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> > new file mode 100644 > >> > index 00000000000..93b80d569aa > >> > --- /dev/null > >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> > @@ -0,0 +1,16 @@ > >> > +/* PR tree-optimization/94290 */ > >> > + > >> > +#include "../../gcc.dg/pr94290.c" > >> > + > >> > +int main() { > >> > + > >> > + if (foo(0) != 0 > >> > + || foo(-42) != 42 > >> > + || foo(42) != 42 > >> > + || baz(-10) != 10 > >> > + || baz(-10) != 10) { > >> > + __builtin_abort(); > >> > + } > >> > + > >> > + return 0; > >> > +} > >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > b/gcc/testsuite/gcc.dg/pr94290-2.c > >> > new file mode 100644 > >> > index 00000000000..ea6e55755f5 > >> > --- /dev/null > >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > >> > @@ -0,0 +1,15 @@ > >> > +/* PR tree-optimization/94290 */ > >> > +/* { dg-do compile } */ > >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> > + > >> > +/* Form from PR. */ > >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> > + return x <= 0 ? -x : 0; > >> > +} > >> > + > >> > +/* Changed order. */ > >> > +__attribute__((noipa)) unsigned int bar(int x) { > >> > + return 0 >= x ? -x : 0; > >> > +} > >> > + > >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ > >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > b/gcc/testsuite/gcc.dg/pr94290.c > >> > new file mode 100644 > >> > index 00000000000..47617c36c02 > >> > --- /dev/null > >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > >> > @@ -0,0 +1,46 @@ > >> > +/* PR tree-optimization/94290 */ > >> > +/* { dg-do compile } */ > >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> > + > >> > + > >> > +/* Same form as PR. */ > >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> > +} > >> > + > >> > +/* Signed function. */ > >> > +__attribute__((noipa)) int bar(int x) { > >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> > +} > >> > + > >> > +/* Commutative property. */ > >> > +__attribute__((noipa)) unsigned int baz(int x) { > >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > >> > +} > >> > + > >> > +/* Flipped order for max expressions. */ > >> > +__attribute__((noipa)) unsigned int quux(int x) { > >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > >> > +} > >> > + > >> > +/* Not zero so should not optimize. */ > >> > +__attribute__((noipa)) unsigned int waldo(int x) { > >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > >> > +} > >> > + > >> > +/* Not zero so should not optimize. */ > >> > +__attribute__((noipa)) unsigned int fred(int x) { > >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > >> > +} > >> > + > >> > +/* Incorrect pattern. */ > >> > +__attribute__((noipa)) unsigned int goo(int x) { > >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > >> > +} > >> > + > >> > +/* Incorrect pattern. */ > >> > +__attribute__((noipa)) int qux(int x) { > >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > >> > +} > >> > + > >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ > >> > > >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > >> > -- > >> > 2.31.1 > >> > > >> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-14 19:38 ` Sam Feifer @ 2022-07-14 19:53 ` Andrew Pinski 2022-07-18 13:07 ` Sam Feifer 0 siblings, 1 reply; 11+ messages in thread From: Andrew Pinski @ 2022-07-14 19:53 UTC (permalink / raw) To: Sam Feifer; +Cc: GCC Patches On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: > > > > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: >> > >> > >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> wrote: >> >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches >> >> <gcc-patches@gcc.gnu.org> wrote: >> >> > >> >> > This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to write a second simplification in match.pd to handle the commutative property as the match was not ocurring otherwise. Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. >> >> >> >> You could use :c for the commutative property instead and that should >> >> simplify things. >> >> That is: >> >> >> >> (simplify >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> (abs @0)) >> >> >> >> Also since integer_zerop works on vectors, it seems like you should >> >> add a testcase or two for the vector case. >> >> Also would be useful if you write a testcase that uses different >> >> statements rather than one big one so it gets exercised in the >> >> forwprop case. >> >> Note also if either of the max are used more than just in this >> >> simplification, it could increase the lifetime of @0, maybe you need >> >> to add :s to the max expressions. >> >> >> > >> > Thanks for the feedback. I'm not quite sure what a vector test case would look like for this. Could I get some guidance on that? >> >> Yes this should produce the pattern at forwprop1 time (with the C++ >> front-end, the C front-end does not support vector selects): >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; >> >> vint foo(vint x) { >> vint t = (x >= 0 ? x : 0) ; >> vint xx = -x; >> vint t1 = (xx >= 0 ? xx : 0); >> return t + t1; >> } >> >> int foo(int x) { >> int t = (x >= 0 ? x : 0) ; >> int xx = -x; >> int t1 = (xx >= 0 ? xx : 0); >> return t + t1; >> } >> >> Thanks, >> Andrew Pinski >> > > Thanks for the help. I'm still having trouble with the vector test, though. When I try to compile, I get an error saying "used vector type where scalar is required", referring to the max expressions. How do I use the max expression with two vectors as the inputs? As I mentioned it only works with the C++ front-end :). The C front-end does not support ?: with vectors types. Thanks, Andrew Pinski > > Thanks > -Sam >> >> >> > >> > Thanks >> > -Sam >> > >> >> >> >> Thanks, >> >> Andrew >> >> >> >> > >> >> > Tests are also included to be added to the testsuite. >> >> > >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> >> > >> >> > PR tree-optimization/94290 >> >> > >> >> > gcc/ChangeLog: >> >> > >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. >> >> > >> >> > gcc/testsuite/ChangeLog: >> >> > >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. >> >> > * gcc.dg/pr94290-2.c: New test. >> >> > * gcc.dg/pr94290.c: New test. >> >> > --- >> >> > gcc/match.pd | 15 ++++++ >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 +++++++++++++++++++ >> >> > 4 files changed, 92 insertions(+) >> >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c >> >> > >> >> > diff --git a/gcc/match.pd b/gcc/match.pd >> >> > index 45aefd96688..55ca79d7ac9 100644 >> >> > --- a/gcc/match.pd >> >> > +++ b/gcc/match.pd >> >> > @@ -7848,3 +7848,18 @@ and, >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) >> >> > (bit_and @0 @1) >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) >> >> > + >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ >> >> > +(simplify >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> > + (abs @0)) >> >> > + >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ >> >> > +(simplify >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) >> >> > + (abs @0)) >> >> > + >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ >> >> > +(simplify >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) >> >> > + (max (negate @0) @1)) >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> > new file mode 100644 >> >> > index 00000000000..93b80d569aa >> >> > --- /dev/null >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> > @@ -0,0 +1,16 @@ >> >> > +/* PR tree-optimization/94290 */ >> >> > + >> >> > +#include "../../gcc.dg/pr94290.c" >> >> > + >> >> > +int main() { >> >> > + >> >> > + if (foo(0) != 0 >> >> > + || foo(-42) != 42 >> >> > + || foo(42) != 42 >> >> > + || baz(-10) != 10 >> >> > + || baz(-10) != 10) { >> >> > + __builtin_abort(); >> >> > + } >> >> > + >> >> > + return 0; >> >> > +} >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> > new file mode 100644 >> >> > index 00000000000..ea6e55755f5 >> >> > --- /dev/null >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> > @@ -0,0 +1,15 @@ >> >> > +/* PR tree-optimization/94290 */ >> >> > +/* { dg-do compile } */ >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> > + >> >> > +/* Form from PR. */ >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> > + return x <= 0 ? -x : 0; >> >> > +} >> >> > + >> >> > +/* Changed order. */ >> >> > +__attribute__((noipa)) unsigned int bar(int x) { >> >> > + return 0 >= x ? -x : 0; >> >> > +} >> >> > + >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c b/gcc/testsuite/gcc.dg/pr94290.c >> >> > new file mode 100644 >> >> > index 00000000000..47617c36c02 >> >> > --- /dev/null >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c >> >> > @@ -0,0 +1,46 @@ >> >> > +/* PR tree-optimization/94290 */ >> >> > +/* { dg-do compile } */ >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> > + >> >> > + >> >> > +/* Same form as PR. */ >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Signed function. */ >> >> > +__attribute__((noipa)) int bar(int x) { >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Commutative property. */ >> >> > +__attribute__((noipa)) unsigned int baz(int x) { >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); >> >> > +} >> >> > + >> >> > +/* Flipped order for max expressions. */ >> >> > +__attribute__((noipa)) unsigned int quux(int x) { >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Not zero so should not optimize. */ >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); >> >> > +} >> >> > + >> >> > +/* Not zero so should not optimize. */ >> >> > +__attribute__((noipa)) unsigned int fred(int x) { >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); >> >> > +} >> >> > + >> >> > +/* Incorrect pattern. */ >> >> > +__attribute__((noipa)) unsigned int goo(int x) { >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); >> >> > +} >> >> > + >> >> > +/* Incorrect pattern. */ >> >> > +__attribute__((noipa)) int qux(int x) { >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); >> >> > +} >> >> > + >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } } */ >> >> > >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 >> >> > -- >> >> > 2.31.1 >> >> > >> >> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-14 19:53 ` Andrew Pinski @ 2022-07-18 13:07 ` Sam Feifer 2022-07-18 17:30 ` Sam Feifer 0 siblings, 1 reply; 11+ messages in thread From: Sam Feifer @ 2022-07-18 13:07 UTC (permalink / raw) To: Andrew Pinski; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 8813 bytes --] Here's an updated version of the patch. Thanks -Sam On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> wrote: > On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: > > > > > > > > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> wrote: > >> > >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > >> > > >> > > >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> > wrote: > >> >> > >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > >> >> <gcc-patches@gcc.gnu.org> wrote: > >> >> > > >> >> > This patch is intended to fix a missed optimization in match.pd. > It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > write a second simplification in match.pd to handle the commutative > property as the match was not ocurring otherwise. Additionally, the pattern > (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > other simplification rule. > >> >> > >> >> You could use :c for the commutative property instead and that should > >> >> simplify things. > >> >> That is: > >> >> > >> >> (simplify > >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> (abs @0)) > >> >> > >> >> Also since integer_zerop works on vectors, it seems like you should > >> >> add a testcase or two for the vector case. > >> >> Also would be useful if you write a testcase that uses different > >> >> statements rather than one big one so it gets exercised in the > >> >> forwprop case. > >> >> Note also if either of the max are used more than just in this > >> >> simplification, it could increase the lifetime of @0, maybe you need > >> >> to add :s to the max expressions. > >> >> > >> > > >> > Thanks for the feedback. I'm not quite sure what a vector test case > would look like for this. Could I get some guidance on that? > >> > >> Yes this should produce the pattern at forwprop1 time (with the C++ > >> front-end, the C front-end does not support vector selects): > >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > >> > >> vint foo(vint x) { > >> vint t = (x >= 0 ? x : 0) ; > >> vint xx = -x; > >> vint t1 = (xx >= 0 ? xx : 0); > >> return t + t1; > >> } > >> > >> int foo(int x) { > >> int t = (x >= 0 ? x : 0) ; > >> int xx = -x; > >> int t1 = (xx >= 0 ? xx : 0); > >> return t + t1; > >> } > >> > >> Thanks, > >> Andrew Pinski > >> > > > > Thanks for the help. I'm still having trouble with the vector test, > though. When I try to compile, I get an error saying "used vector type > where scalar is required", referring to the max expressions. How do I use > the max expression with two vectors as the inputs? > > As I mentioned it only works with the C++ front-end :). The C > front-end does not support ?: with vectors types. > > Thanks, > Andrew Pinski > > > > > Thanks > > -Sam > >> > >> > >> > > >> > Thanks > >> > -Sam > >> > > >> >> > >> >> Thanks, > >> >> Andrew > >> >> > >> >> > > >> >> > Tests are also included to be added to the testsuite. > >> >> > > >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > >> >> > > >> >> > PR tree-optimization/94290 > >> >> > > >> >> > gcc/ChangeLog: > >> >> > > >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > simplification. > >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > >> >> > > >> >> > gcc/testsuite/ChangeLog: > >> >> > > >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. > >> >> > * gcc.dg/pr94290-2.c: New test. > >> >> > * gcc.dg/pr94290.c: New test. > >> >> > --- > >> >> > gcc/match.pd | 15 ++++++ > >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > +++++++++++++++++++ > >> >> > 4 files changed, 92 insertions(+) > >> >> > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > >> >> > > >> >> > diff --git a/gcc/match.pd b/gcc/match.pd > >> >> > index 45aefd96688..55ca79d7ac9 100644 > >> >> > --- a/gcc/match.pd > >> >> > +++ b/gcc/match.pd > >> >> > @@ -7848,3 +7848,18 @@ and, > >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > >> >> > (bit_and @0 @1) > >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > >> >> > + > >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > >> >> > +(simplify > >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> > + (abs @0)) > >> >> > + > >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > >> >> > +(simplify > >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > >> >> > + (abs @0)) > >> >> > + > >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > >> >> > +(simplify > >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > >> >> > + (max (negate @0) @1)) > >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> > new file mode 100644 > >> >> > index 00000000000..93b80d569aa > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> > @@ -0,0 +1,16 @@ > >> >> > +/* PR tree-optimization/94290 */ > >> >> > + > >> >> > +#include "../../gcc.dg/pr94290.c" > >> >> > + > >> >> > +int main() { > >> >> > + > >> >> > + if (foo(0) != 0 > >> >> > + || foo(-42) != 42 > >> >> > + || foo(42) != 42 > >> >> > + || baz(-10) != 10 > >> >> > + || baz(-10) != 10) { > >> >> > + __builtin_abort(); > >> >> > + } > >> >> > + > >> >> > + return 0; > >> >> > +} > >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> > new file mode 100644 > >> >> > index 00000000000..ea6e55755f5 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> > @@ -0,0 +1,15 @@ > >> >> > +/* PR tree-optimization/94290 */ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> > + > >> >> > +/* Form from PR. */ > >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> > + return x <= 0 ? -x : 0; > >> >> > +} > >> >> > + > >> >> > +/* Changed order. */ > >> >> > +__attribute__((noipa)) unsigned int bar(int x) { > >> >> > + return 0 >= x ? -x : 0; > >> >> > +} > >> >> > + > >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } > } */ > >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > b/gcc/testsuite/gcc.dg/pr94290.c > >> >> > new file mode 100644 > >> >> > index 00000000000..47617c36c02 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > >> >> > @@ -0,0 +1,46 @@ > >> >> > +/* PR tree-optimization/94290 */ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> > + > >> >> > + > >> >> > +/* Same form as PR. */ > >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Signed function. */ > >> >> > +__attribute__((noipa)) int bar(int x) { > >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Commutative property. */ > >> >> > +__attribute__((noipa)) unsigned int baz(int x) { > >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Flipped order for max expressions. */ > >> >> > +__attribute__((noipa)) unsigned int quux(int x) { > >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Not zero so should not optimize. */ > >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { > >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > >> >> > +} > >> >> > + > >> >> > +/* Not zero so should not optimize. */ > >> >> > +__attribute__((noipa)) unsigned int fred(int x) { > >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > >> >> > +} > >> >> > + > >> >> > +/* Incorrect pattern. */ > >> >> > +__attribute__((noipa)) unsigned int goo(int x) { > >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > >> >> > +} > >> >> > + > >> >> > +/* Incorrect pattern. */ > >> >> > +__attribute__((noipa)) int qux(int x) { > >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > >> >> > +} > >> >> > + > >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } > } */ > >> >> > > >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > >> >> > -- > >> >> > 2.31.1 > >> >> > > >> >> > >> > > [-- Attachment #2: 0001-match.pd-Add-new-abs-pattern-PR94290.patch --] [-- Type: text/x-patch, Size: 4897 bytes --] From 894b10a6a9802eeee90524692dd6a00bf2c4261c Mon Sep 17 00:00:00 2001 From: Sam Feifer <sfeifer@redhat.com> Date: Fri, 15 Jul 2022 13:39:23 -0400 Subject: [PATCH] match.pd: Add new abs pattern [PR94290] This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. Tests are also included to be added to the testsuite. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR tree-optimization/94290 gcc/ChangeLog: * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. * match.pd (x <= 0 ? -x : 0): New simplification. gcc/testsuite/ChangeLog: * g++.dg/pr94290-1.C: New test. * g++.dg/pr94290.C: New test. * gcc.dg/pr94290-2.c: New test. --- gcc/match.pd | 10 +++++ gcc/testsuite/g++.dg/pr94290-1.C | 17 +++++++++ gcc/testsuite/g++.dg/pr94290.C | 63 ++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++++ 4 files changed, 105 insertions(+) create mode 100644 gcc/testsuite/g++.dg/pr94290-1.C create mode 100644 gcc/testsuite/g++.dg/pr94290.C create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c diff --git a/gcc/match.pd b/gcc/match.pd index 45aefd96688..ed6609295db 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7848,3 +7848,13 @@ and, (if (TYPE_UNSIGNED (TREE_TYPE (@0))) (bit_and @0 @1) (cond (le @0 @1) @0 (bit_and @0 @1)))))) + +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ +(simplify + (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) + (abs @0)) + +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) + (max (negate @0) @1)) diff --git a/gcc/testsuite/g++.dg/pr94290-1.C b/gcc/testsuite/g++.dg/pr94290-1.C new file mode 100644 index 00000000000..954408c069f --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94290-1.C @@ -0,0 +1,17 @@ +/* PR tree-optimization/94290 */ +/* { dg-do run } */ + +#include "pr94290.C" + +int main() { + + if (foo(0) != 0 + || foo(-42) != 42 + || foo(42) != 42 + || baz(-10) != 10 + || baz(-10) != 10) { + __builtin_abort(); + } + + return 0; +} diff --git a/gcc/testsuite/g++.dg/pr94290.C b/gcc/testsuite/g++.dg/pr94290.C new file mode 100644 index 00000000000..072ff327977 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94290.C @@ -0,0 +1,63 @@ +/* PR tree-optimization/94290 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +typedef int __attribute__((vector_size(4*sizeof(int)))) vint; + +/* Same form as PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Test for forward propogation. */ +__attribute__((noipa)) unsigned int corge(int x) { + int w = (x >= 0 ? x : 0); + int y = -x; + int z = (y >= 0 ? y : 0); + return w + z; +} + +/* Vector case. */ +__attribute__((noipa)) vint thud(vint x) { + vint t = (x >= 0 ? x : 0) ; + vint xx = -x; + vint t1 = (xx >= 0 ? xx : 0); + return t + t1; +} + +/* Signed function. */ +__attribute__((noipa)) int bar(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Commutative property. */ +__attribute__((noipa)) unsigned int baz(int x) { + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); +} + +/* Flipped order for max expressions. */ +__attribute__((noipa)) unsigned int quux(int x) { + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int waldo(int x) { + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int fred(int x) { + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) unsigned int goo(int x) { + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) int qux(int x) { + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); +} + +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 6 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c b/gcc/testsuite/gcc.dg/pr94290-2.c new file mode 100644 index 00000000000..ea6e55755f5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94290-2.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/94290 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +/* Form from PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return x <= 0 ? -x : 0; +} + +/* Changed order. */ +__attribute__((noipa)) unsigned int bar(int x) { + return 0 >= x ? -x : 0; +} + +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ base-commit: 6af530f914801f5e561057da55c41480f28751f7 -- 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-18 13:07 ` Sam Feifer @ 2022-07-18 17:30 ` Sam Feifer 2022-07-19 6:36 ` Richard Biener 0 siblings, 1 reply; 11+ messages in thread From: Sam Feifer @ 2022-07-18 17:30 UTC (permalink / raw) To: Andrew Pinski; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 9332 bytes --] Just realized I had mixed up the 9 and the 2 when labelling the patch. This patch is referring to pr94920 not pr94290. Attached is a fixed patch file. Sorry for any confusion. On Mon, Jul 18, 2022 at 9:07 AM Sam Feifer <sfeifer@redhat.com> wrote: > Here's an updated version of the patch. > > Thanks > -Sam > > On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> wrote: > >> On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: >> > >> > >> > >> > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> >> wrote: >> >> >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: >> >> > >> >> > >> >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> >> wrote: >> >> >> >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches >> >> >> <gcc-patches@gcc.gnu.org> wrote: >> >> >> > >> >> >> > This patch is intended to fix a missed optimization in match.pd. >> It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to >> write a second simplification in match.pd to handle the commutative >> property as the match was not ocurring otherwise. Additionally, the pattern >> (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the >> other simplification rule. >> >> >> >> >> >> You could use :c for the commutative property instead and that >> should >> >> >> simplify things. >> >> >> That is: >> >> >> >> >> >> (simplify >> >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> >> (abs @0)) >> >> >> >> >> >> Also since integer_zerop works on vectors, it seems like you should >> >> >> add a testcase or two for the vector case. >> >> >> Also would be useful if you write a testcase that uses different >> >> >> statements rather than one big one so it gets exercised in the >> >> >> forwprop case. >> >> >> Note also if either of the max are used more than just in this >> >> >> simplification, it could increase the lifetime of @0, maybe you need >> >> >> to add :s to the max expressions. >> >> >> >> >> > >> >> > Thanks for the feedback. I'm not quite sure what a vector test case >> would look like for this. Could I get some guidance on that? >> >> >> >> Yes this should produce the pattern at forwprop1 time (with the C++ >> >> front-end, the C front-end does not support vector selects): >> >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; >> >> >> >> vint foo(vint x) { >> >> vint t = (x >= 0 ? x : 0) ; >> >> vint xx = -x; >> >> vint t1 = (xx >= 0 ? xx : 0); >> >> return t + t1; >> >> } >> >> >> >> int foo(int x) { >> >> int t = (x >= 0 ? x : 0) ; >> >> int xx = -x; >> >> int t1 = (xx >= 0 ? xx : 0); >> >> return t + t1; >> >> } >> >> >> >> Thanks, >> >> Andrew Pinski >> >> >> > >> > Thanks for the help. I'm still having trouble with the vector test, >> though. When I try to compile, I get an error saying "used vector type >> where scalar is required", referring to the max expressions. How do I use >> the max expression with two vectors as the inputs? >> >> As I mentioned it only works with the C++ front-end :). The C >> front-end does not support ?: with vectors types. >> >> Thanks, >> Andrew Pinski >> >> > >> > Thanks >> > -Sam >> >> >> >> >> >> > >> >> > Thanks >> >> > -Sam >> >> > >> >> >> >> >> >> Thanks, >> >> >> Andrew >> >> >> >> >> >> > >> >> >> > Tests are also included to be added to the testsuite. >> >> >> > >> >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? >> >> >> > >> >> >> > PR tree-optimization/94290 >> >> >> > >> >> >> > gcc/ChangeLog: >> >> >> > >> >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New >> simplification. >> >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. >> >> >> > >> >> >> > gcc/testsuite/ChangeLog: >> >> >> > >> >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. >> >> >> > * gcc.dg/pr94290-2.c: New test. >> >> >> > * gcc.dg/pr94290.c: New test. >> >> >> > --- >> >> >> > gcc/match.pd | 15 ++++++ >> >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ >> >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ >> >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 >> +++++++++++++++++++ >> >> >> > 4 files changed, 92 insertions(+) >> >> >> > create mode 100644 >> gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c >> >> >> > >> >> >> > diff --git a/gcc/match.pd b/gcc/match.pd >> >> >> > index 45aefd96688..55ca79d7ac9 100644 >> >> >> > --- a/gcc/match.pd >> >> >> > +++ b/gcc/match.pd >> >> >> > @@ -7848,3 +7848,18 @@ and, >> >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) >> >> >> > (bit_and @0 @1) >> >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) >> >> >> > + >> >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ >> >> >> > +(simplify >> >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) >> >> >> > + (abs @0)) >> >> >> > + >> >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ >> >> >> > +(simplify >> >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) >> >> >> > + (abs @0)) >> >> >> > + >> >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ >> >> >> > +(simplify >> >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) >> >> >> > + (max (negate @0) @1)) >> >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> >> > new file mode 100644 >> >> >> > index 00000000000..93b80d569aa >> >> >> > --- /dev/null >> >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c >> >> >> > @@ -0,0 +1,16 @@ >> >> >> > +/* PR tree-optimization/94290 */ >> >> >> > + >> >> >> > +#include "../../gcc.dg/pr94290.c" >> >> >> > + >> >> >> > +int main() { >> >> >> > + >> >> >> > + if (foo(0) != 0 >> >> >> > + || foo(-42) != 42 >> >> >> > + || foo(42) != 42 >> >> >> > + || baz(-10) != 10 >> >> >> > + || baz(-10) != 10) { >> >> >> > + __builtin_abort(); >> >> >> > + } >> >> >> > + >> >> >> > + return 0; >> >> >> > +} >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c >> b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> >> > new file mode 100644 >> >> >> > index 00000000000..ea6e55755f5 >> >> >> > --- /dev/null >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c >> >> >> > @@ -0,0 +1,15 @@ >> >> >> > +/* PR tree-optimization/94290 */ >> >> >> > +/* { dg-do compile } */ >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> >> > + >> >> >> > +/* Form from PR. */ >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> >> > + return x <= 0 ? -x : 0; >> >> >> > +} >> >> >> > + >> >> >> > +/* Changed order. */ >> >> >> > +__attribute__((noipa)) unsigned int bar(int x) { >> >> >> > + return 0 >= x ? -x : 0; >> >> >> > +} >> >> >> > + >> >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } >> } */ >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c >> b/gcc/testsuite/gcc.dg/pr94290.c >> >> >> > new file mode 100644 >> >> >> > index 00000000000..47617c36c02 >> >> >> > --- /dev/null >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c >> >> >> > @@ -0,0 +1,46 @@ >> >> >> > +/* PR tree-optimization/94290 */ >> >> >> > +/* { dg-do compile } */ >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ >> >> >> > + >> >> >> > + >> >> >> > +/* Same form as PR. */ >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Signed function. */ >> >> >> > +__attribute__((noipa)) int bar(int x) { >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Commutative property. */ >> >> >> > +__attribute__((noipa)) unsigned int baz(int x) { >> >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Flipped order for max expressions. */ >> >> >> > +__attribute__((noipa)) unsigned int quux(int x) { >> >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Not zero so should not optimize. */ >> >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { >> >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); >> >> >> > +} >> >> >> > + >> >> >> > +/* Not zero so should not optimize. */ >> >> >> > +__attribute__((noipa)) unsigned int fred(int x) { >> >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); >> >> >> > +} >> >> >> > + >> >> >> > +/* Incorrect pattern. */ >> >> >> > +__attribute__((noipa)) unsigned int goo(int x) { >> >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* Incorrect pattern. */ >> >> >> > +__attribute__((noipa)) int qux(int x) { >> >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); >> >> >> > +} >> >> >> > + >> >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } >> } */ >> >> >> > >> >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 >> >> >> > -- >> >> >> > 2.31.1 >> >> >> > >> >> >> >> >> >> >> [-- Attachment #2: 0001-match.pd-Add-new-abs-pattern-PR94920.patch --] [-- Type: text/x-patch, Size: 4899 bytes --] From 30bc598294b8c26d03d08cc59b6a435126406450 Mon Sep 17 00:00:00 2001 From: Sam Feifer <sfeifer@redhat.com> Date: Mon, 18 Jul 2022 13:18:39 -0400 Subject: [PATCH] match.pd: Add new abs pattern [PR94920] This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. Tests are also included to be added to the testsuite. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR tree-optimization/94920 gcc/ChangeLog: * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. * match.pd (x <= 0 ? -x : 0): New simplification. gcc/testsuite/ChangeLog: * g++.dg/pr94920-1.C: New test. * g++.dg/pr94920.C: New test. * gcc.dg/pr94920-2.c: New test. --- gcc/match.pd | 10 +++++ gcc/testsuite/g++.dg/pr94920-1.C | 17 +++++++++ gcc/testsuite/g++.dg/pr94920.C | 63 ++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/pr94920-2.c | 15 ++++++++ 4 files changed, 105 insertions(+) create mode 100644 gcc/testsuite/g++.dg/pr94920-1.C create mode 100644 gcc/testsuite/g++.dg/pr94920.C create mode 100644 gcc/testsuite/gcc.dg/pr94920-2.c diff --git a/gcc/match.pd b/gcc/match.pd index 45aefd96688..05758f1372b 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7848,3 +7848,13 @@ and, (if (TYPE_UNSIGNED (TREE_TYPE (@0))) (bit_and @0 @1) (cond (le @0 @1) @0 (bit_and @0 @1)))))) + +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ +(simplify + (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) + (abs @0)) + +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) + (max (negate @0) @1)) diff --git a/gcc/testsuite/g++.dg/pr94920-1.C b/gcc/testsuite/g++.dg/pr94920-1.C new file mode 100644 index 00000000000..6c6483eab2d --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94920-1.C @@ -0,0 +1,17 @@ +/* PR tree-optimization/94920 */ +/* { dg-do run } */ + +#include "pr94920.C" + +int main() { + + if (foo(0) != 0 + || foo(-42) != 42 + || foo(42) != 42 + || baz(-10) != 10 + || baz(-10) != 10) { + __builtin_abort(); + } + + return 0; +} diff --git a/gcc/testsuite/g++.dg/pr94920.C b/gcc/testsuite/g++.dg/pr94920.C new file mode 100644 index 00000000000..925ec4f42f1 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94920.C @@ -0,0 +1,63 @@ +/* PR tree-optimization/94920 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +typedef int __attribute__((vector_size(4*sizeof(int)))) vint; + +/* Same form as PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Test for forward propogation. */ +__attribute__((noipa)) unsigned int corge(int x) { + int w = (x >= 0 ? x : 0); + int y = -x; + int z = (y >= 0 ? y : 0); + return w + z; +} + +/* Vector case. */ +__attribute__((noipa)) vint thud(vint x) { + vint t = (x >= 0 ? x : 0) ; + vint xx = -x; + vint t1 = (xx >= 0 ? xx : 0); + return t + t1; +} + +/* Signed function. */ +__attribute__((noipa)) int bar(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Commutative property. */ +__attribute__((noipa)) unsigned int baz(int x) { + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); +} + +/* Flipped order for max expressions. */ +__attribute__((noipa)) unsigned int quux(int x) { + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int waldo(int x) { + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int fred(int x) { + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) unsigned int goo(int x) { + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) int qux(int x) { + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); +} + +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 6 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/pr94920-2.c b/gcc/testsuite/gcc.dg/pr94920-2.c new file mode 100644 index 00000000000..a2d23324cfa --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94920-2.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/94920 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +/* Form from PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return x <= 0 ? -x : 0; +} + +/* Changed order. */ +__attribute__((noipa)) unsigned int bar(int x) { + return 0 >= x ? -x : 0; +} + +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ base-commit: 6af530f914801f5e561057da55c41480f28751f7 -- 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-18 17:30 ` Sam Feifer @ 2022-07-19 6:36 ` Richard Biener 2022-07-19 13:48 ` Sam Feifer 0 siblings, 1 reply; 11+ messages in thread From: Richard Biener @ 2022-07-19 6:36 UTC (permalink / raw) To: Sam Feifer; +Cc: Andrew Pinski, GCC Patches On Mon, Jul 18, 2022 at 7:31 PM Sam Feifer via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > Just realized I had mixed up the 9 and the 2 when labelling the patch. This > patch is referring to pr94920 not pr94290. Attached is a fixed patch file. > Sorry for any confusion. Can you put the patterns somewhere related? The abs producing one seems to fit around line 323, the max producing before line 3415? +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) + (max (negate @0) @1)) you can re-use the existing (negate @0) in the result by doing +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1) + (max @2 @1)) OK with those changes. Thanks, Richard. > On Mon, Jul 18, 2022 at 9:07 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > Here's an updated version of the patch. > > > > Thanks > > -Sam > > > > On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> wrote: > > > >> On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> wrote: > >> > > >> > > >> > > >> > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> > >> wrote: > >> >> > >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> wrote: > >> >> > > >> >> > > >> >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com> > >> wrote: > >> >> >> > >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > >> >> >> <gcc-patches@gcc.gnu.org> wrote: > >> >> >> > > >> >> >> > This patch is intended to fix a missed optimization in match.pd. > >> It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I had to > >> write a second simplification in match.pd to handle the commutative > >> property as the match was not ocurring otherwise. Additionally, the pattern > >> (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the > >> other simplification rule. > >> >> >> > >> >> >> You could use :c for the commutative property instead and that > >> should > >> >> >> simplify things. > >> >> >> That is: > >> >> >> > >> >> >> (simplify > >> >> >> (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> >> (abs @0)) > >> >> >> > >> >> >> Also since integer_zerop works on vectors, it seems like you should > >> >> >> add a testcase or two for the vector case. > >> >> >> Also would be useful if you write a testcase that uses different > >> >> >> statements rather than one big one so it gets exercised in the > >> >> >> forwprop case. > >> >> >> Note also if either of the max are used more than just in this > >> >> >> simplification, it could increase the lifetime of @0, maybe you need > >> >> >> to add :s to the max expressions. > >> >> >> > >> >> > > >> >> > Thanks for the feedback. I'm not quite sure what a vector test case > >> would look like for this. Could I get some guidance on that? > >> >> > >> >> Yes this should produce the pattern at forwprop1 time (with the C++ > >> >> front-end, the C front-end does not support vector selects): > >> >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > >> >> > >> >> vint foo(vint x) { > >> >> vint t = (x >= 0 ? x : 0) ; > >> >> vint xx = -x; > >> >> vint t1 = (xx >= 0 ? xx : 0); > >> >> return t + t1; > >> >> } > >> >> > >> >> int foo(int x) { > >> >> int t = (x >= 0 ? x : 0) ; > >> >> int xx = -x; > >> >> int t1 = (xx >= 0 ? xx : 0); > >> >> return t + t1; > >> >> } > >> >> > >> >> Thanks, > >> >> Andrew Pinski > >> >> > >> > > >> > Thanks for the help. I'm still having trouble with the vector test, > >> though. When I try to compile, I get an error saying "used vector type > >> where scalar is required", referring to the max expressions. How do I use > >> the max expression with two vectors as the inputs? > >> > >> As I mentioned it only works with the C++ front-end :). The C > >> front-end does not support ?: with vectors types. > >> > >> Thanks, > >> Andrew Pinski > >> > >> > > >> > Thanks > >> > -Sam > >> >> > >> >> > >> >> > > >> >> > Thanks > >> >> > -Sam > >> >> > > >> >> >> > >> >> >> Thanks, > >> >> >> Andrew > >> >> >> > >> >> >> > > >> >> >> > Tests are also included to be added to the testsuite. > >> >> >> > > >> >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > >> >> >> > > >> >> >> > PR tree-optimization/94290 > >> >> >> > > >> >> >> > gcc/ChangeLog: > >> >> >> > > >> >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > >> simplification. > >> >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > >> >> >> > > >> >> >> > gcc/testsuite/ChangeLog: > >> >> >> > > >> >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. > >> >> >> > * gcc.dg/pr94290-2.c: New test. > >> >> >> > * gcc.dg/pr94290.c: New test. > >> >> >> > --- > >> >> >> > gcc/match.pd | 15 ++++++ > >> >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > >> >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > >> >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > >> +++++++++++++++++++ > >> >> >> > 4 files changed, 92 insertions(+) > >> >> >> > create mode 100644 > >> gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > >> >> >> > > >> >> >> > diff --git a/gcc/match.pd b/gcc/match.pd > >> >> >> > index 45aefd96688..55ca79d7ac9 100644 > >> >> >> > --- a/gcc/match.pd > >> >> >> > +++ b/gcc/match.pd > >> >> >> > @@ -7848,3 +7848,18 @@ and, > >> >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > >> >> >> > (bit_and @0 @1) > >> >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > >> >> >> > + > >> >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > >> >> >> > +(simplify > >> >> >> > + (plus (max @0 integer_zerop) (max (negate @0) integer_zerop)) > >> >> >> > + (abs @0)) > >> >> >> > + > >> >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > >> >> >> > +(simplify > >> >> >> > + (plus (max (negate @0) integer_zerop) (max @0 integer_zerop) ) > >> >> >> > + (abs @0)) > >> >> >> > + > >> >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > >> >> >> > +(simplify > >> >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > >> >> >> > + (max (negate @0) @1)) > >> >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> >> > new file mode 100644 > >> >> >> > index 00000000000..93b80d569aa > >> >> >> > --- /dev/null > >> >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > >> >> >> > @@ -0,0 +1,16 @@ > >> >> >> > +/* PR tree-optimization/94290 */ > >> >> >> > + > >> >> >> > +#include "../../gcc.dg/pr94290.c" > >> >> >> > + > >> >> >> > +int main() { > >> >> >> > + > >> >> >> > + if (foo(0) != 0 > >> >> >> > + || foo(-42) != 42 > >> >> >> > + || foo(42) != 42 > >> >> >> > + || baz(-10) != 10 > >> >> >> > + || baz(-10) != 10) { > >> >> >> > + __builtin_abort(); > >> >> >> > + } > >> >> >> > + > >> >> >> > + return 0; > >> >> >> > +} > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > >> b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> >> > new file mode 100644 > >> >> >> > index 00000000000..ea6e55755f5 > >> >> >> > --- /dev/null > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > >> >> >> > @@ -0,0 +1,15 @@ > >> >> >> > +/* PR tree-optimization/94290 */ > >> >> >> > +/* { dg-do compile } */ > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> >> > + > >> >> >> > +/* Form from PR. */ > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> >> > + return x <= 0 ? -x : 0; > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Changed order. */ > >> >> >> > +__attribute__((noipa)) unsigned int bar(int x) { > >> >> >> > + return 0 >= x ? -x : 0; > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } > >> } */ > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > >> b/gcc/testsuite/gcc.dg/pr94290.c > >> >> >> > new file mode 100644 > >> >> >> > index 00000000000..47617c36c02 > >> >> >> > --- /dev/null > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > >> >> >> > @@ -0,0 +1,46 @@ > >> >> >> > +/* PR tree-optimization/94290 */ > >> >> >> > +/* { dg-do compile } */ > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > >> >> >> > + > >> >> >> > + > >> >> >> > +/* Same form as PR. */ > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Signed function. */ > >> >> >> > +__attribute__((noipa)) int bar(int x) { > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Commutative property. */ > >> >> >> > +__attribute__((noipa)) unsigned int baz(int x) { > >> >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Flipped order for max expressions. */ > >> >> >> > +__attribute__((noipa)) unsigned int quux(int x) { > >> >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Not zero so should not optimize. */ > >> >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { > >> >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Not zero so should not optimize. */ > >> >> >> > +__attribute__((noipa)) unsigned int fred(int x) { > >> >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Incorrect pattern. */ > >> >> >> > +__attribute__((noipa)) unsigned int goo(int x) { > >> >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* Incorrect pattern. */ > >> >> >> > +__attribute__((noipa)) int qux(int x) { > >> >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > >> >> >> > +} > >> >> >> > + > >> >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 "optimized" } > >> } */ > >> >> >> > > >> >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > >> >> >> > -- > >> >> >> > 2.31.1 > >> >> >> > > >> >> >> > >> >> > >> > >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] match.pd: Add new abs pattern [PR94290] 2022-07-19 6:36 ` Richard Biener @ 2022-07-19 13:48 ` Sam Feifer 0 siblings, 0 replies; 11+ messages in thread From: Sam Feifer @ 2022-07-19 13:48 UTC (permalink / raw) To: Richard Biener; +Cc: Andrew Pinski, GCC Patches [-- Attachment #1: Type: text/plain, Size: 11225 bytes --] Attached is a patch file with those changes. Thanks -Sam On Tue, Jul 19, 2022 at 2:36 AM Richard Biener <richard.guenther@gmail.com> wrote: > On Mon, Jul 18, 2022 at 7:31 PM Sam Feifer via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > Just realized I had mixed up the 9 and the 2 when labelling the patch. > This > > patch is referring to pr94920 not pr94290. Attached is a fixed patch > file. > > Sorry for any confusion. > > Can you put the patterns somewhere related? The abs producing one > seems to fit around line 323, the max producing before line 3415? > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > +(simplify > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > + (max (negate @0) @1)) > > you can re-use the existing (negate @0) in the result by doing > > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > +(simplify > + (cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1) > + (max @2 @1)) > > OK with those changes. > > Thanks, > Richard. > > > On Mon, Jul 18, 2022 at 9:07 AM Sam Feifer <sfeifer@redhat.com> wrote: > > > > > Here's an updated version of the patch. > > > > > > Thanks > > > -Sam > > > > > > On Thu, Jul 14, 2022 at 3:54 PM Andrew Pinski <pinskia@gmail.com> > wrote: > > > > > >> On Thu, Jul 14, 2022 at 12:38 PM Sam Feifer <sfeifer@redhat.com> > wrote: > > >> > > > >> > > > >> > > > >> > On Thu, Jul 14, 2022 at 1:24 PM Andrew Pinski <pinskia@gmail.com> > > >> wrote: > > >> >> > > >> >> On Thu, Jul 14, 2022 at 7:09 AM Sam Feifer <sfeifer@redhat.com> > wrote: > > >> >> > > > >> >> > > > >> >> > On Wed, Jul 13, 2022 at 3:36 PM Andrew Pinski <pinskia@gmail.com > > > > >> wrote: > > >> >> >> > > >> >> >> On Wed, Jul 13, 2022 at 12:26 PM Sam Feifer via Gcc-patches > > >> >> >> <gcc-patches@gcc.gnu.org> wrote: > > >> >> >> > > > >> >> >> > This patch is intended to fix a missed optimization in > match.pd. > > >> It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). I > had to > > >> write a second simplification in match.pd to handle the commutative > > >> property as the match was not ocurring otherwise. Additionally, the > pattern > > >> (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with > the > > >> other simplification rule. > > >> >> >> > > >> >> >> You could use :c for the commutative property instead and that > > >> should > > >> >> >> simplify things. > > >> >> >> That is: > > >> >> >> > > >> >> >> (simplify > > >> >> >> (plus:c (max @0 integer_zerop) (max (negate @0) > integer_zerop)) > > >> >> >> (abs @0)) > > >> >> >> > > >> >> >> Also since integer_zerop works on vectors, it seems like you > should > > >> >> >> add a testcase or two for the vector case. > > >> >> >> Also would be useful if you write a testcase that uses different > > >> >> >> statements rather than one big one so it gets exercised in the > > >> >> >> forwprop case. > > >> >> >> Note also if either of the max are used more than just in this > > >> >> >> simplification, it could increase the lifetime of @0, maybe you > need > > >> >> >> to add :s to the max expressions. > > >> >> >> > > >> >> > > > >> >> > Thanks for the feedback. I'm not quite sure what a vector test > case > > >> would look like for this. Could I get some guidance on that? > > >> >> > > >> >> Yes this should produce the pattern at forwprop1 time (with the C++ > > >> >> front-end, the C front-end does not support vector selects): > > >> >> typedef int __attribute__((vector_size(4*sizeof(int)))) vint; > > >> >> > > >> >> vint foo(vint x) { > > >> >> vint t = (x >= 0 ? x : 0) ; > > >> >> vint xx = -x; > > >> >> vint t1 = (xx >= 0 ? xx : 0); > > >> >> return t + t1; > > >> >> } > > >> >> > > >> >> int foo(int x) { > > >> >> int t = (x >= 0 ? x : 0) ; > > >> >> int xx = -x; > > >> >> int t1 = (xx >= 0 ? xx : 0); > > >> >> return t + t1; > > >> >> } > > >> >> > > >> >> Thanks, > > >> >> Andrew Pinski > > >> >> > > >> > > > >> > Thanks for the help. I'm still having trouble with the vector test, > > >> though. When I try to compile, I get an error saying "used vector type > > >> where scalar is required", referring to the max expressions. How do I > use > > >> the max expression with two vectors as the inputs? > > >> > > >> As I mentioned it only works with the C++ front-end :). The C > > >> front-end does not support ?: with vectors types. > > >> > > >> Thanks, > > >> Andrew Pinski > > >> > > >> > > > >> > Thanks > > >> > -Sam > > >> >> > > >> >> > > >> >> > > > >> >> > Thanks > > >> >> > -Sam > > >> >> > > > >> >> >> > > >> >> >> Thanks, > > >> >> >> Andrew > > >> >> >> > > >> >> >> > > > >> >> >> > Tests are also included to be added to the testsuite. > > >> >> >> > > > >> >> >> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > >> >> >> > > > >> >> >> > PR tree-optimization/94290 > > >> >> >> > > > >> >> >> > gcc/ChangeLog: > > >> >> >> > > > >> >> >> > * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New > > >> simplification. > > >> >> >> > * match.pd (x <= 0 ? -x : 0): New Simplification. > > >> >> >> > > > >> >> >> > gcc/testsuite/ChangeLog: > > >> >> >> > > > >> >> >> > * gcc.c-torture/execute/pr94290-1.c: New test. > > >> >> >> > * gcc.dg/pr94290-2.c: New test. > > >> >> >> > * gcc.dg/pr94290.c: New test. > > >> >> >> > --- > > >> >> >> > gcc/match.pd | 15 ++++++ > > >> >> >> > .../gcc.c-torture/execute/pr94290-1.c | 16 +++++++ > > >> >> >> > gcc/testsuite/gcc.dg/pr94290-2.c | 15 ++++++ > > >> >> >> > gcc/testsuite/gcc.dg/pr94290.c | 46 > > >> +++++++++++++++++++ > > >> >> >> > 4 files changed, 92 insertions(+) > > >> >> >> > create mode 100644 > > >> gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290-2.c > > >> >> >> > create mode 100644 gcc/testsuite/gcc.dg/pr94290.c > > >> >> >> > > > >> >> >> > diff --git a/gcc/match.pd b/gcc/match.pd > > >> >> >> > index 45aefd96688..55ca79d7ac9 100644 > > >> >> >> > --- a/gcc/match.pd > > >> >> >> > +++ b/gcc/match.pd > > >> >> >> > @@ -7848,3 +7848,18 @@ and, > > >> >> >> > (if (TYPE_UNSIGNED (TREE_TYPE (@0))) > > >> >> >> > (bit_and @0 @1) > > >> >> >> > (cond (le @0 @1) @0 (bit_and @0 @1)))))) > > >> >> >> > + > > >> >> >> > +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ > > >> >> >> > +(simplify > > >> >> >> > + (plus (max @0 integer_zerop) (max (negate @0) > integer_zerop)) > > >> >> >> > + (abs @0)) > > >> >> >> > + > > >> >> >> > +/* (x <= 0 ? -x : 0) + (x >= 0 ? x : 0) -> abs x. */ > > >> >> >> > +(simplify > > >> >> >> > + (plus (max (negate @0) integer_zerop) (max @0 > integer_zerop) ) > > >> >> >> > + (abs @0)) > > >> >> >> > + > > >> >> >> > +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ > > >> >> >> > +(simplify > > >> >> >> > + (cond (le @0 integer_zerop@1) (negate @0) integer_zerop@1) > > >> >> >> > + (max (negate @0) @1)) > > >> >> >> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> >> >> > new file mode 100644 > > >> >> >> > index 00000000000..93b80d569aa > > >> >> >> > --- /dev/null > > >> >> >> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr94290-1.c > > >> >> >> > @@ -0,0 +1,16 @@ > > >> >> >> > +/* PR tree-optimization/94290 */ > > >> >> >> > + > > >> >> >> > +#include "../../gcc.dg/pr94290.c" > > >> >> >> > + > > >> >> >> > +int main() { > > >> >> >> > + > > >> >> >> > + if (foo(0) != 0 > > >> >> >> > + || foo(-42) != 42 > > >> >> >> > + || foo(42) != 42 > > >> >> >> > + || baz(-10) != 10 > > >> >> >> > + || baz(-10) != 10) { > > >> >> >> > + __builtin_abort(); > > >> >> >> > + } > > >> >> >> > + > > >> >> >> > + return 0; > > >> >> >> > +} > > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290-2.c > > >> b/gcc/testsuite/gcc.dg/pr94290-2.c > > >> >> >> > new file mode 100644 > > >> >> >> > index 00000000000..ea6e55755f5 > > >> >> >> > --- /dev/null > > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290-2.c > > >> >> >> > @@ -0,0 +1,15 @@ > > >> >> >> > +/* PR tree-optimization/94290 */ > > >> >> >> > +/* { dg-do compile } */ > > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > >> >> >> > + > > >> >> >> > +/* Form from PR. */ > > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > > >> >> >> > + return x <= 0 ? -x : 0; > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Changed order. */ > > >> >> >> > +__attribute__((noipa)) unsigned int bar(int x) { > > >> >> >> > + return 0 >= x ? -x : 0; > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 > "optimized" } > > >> } */ > > >> >> >> > diff --git a/gcc/testsuite/gcc.dg/pr94290.c > > >> b/gcc/testsuite/gcc.dg/pr94290.c > > >> >> >> > new file mode 100644 > > >> >> >> > index 00000000000..47617c36c02 > > >> >> >> > --- /dev/null > > >> >> >> > +++ b/gcc/testsuite/gcc.dg/pr94290.c > > >> >> >> > @@ -0,0 +1,46 @@ > > >> >> >> > +/* PR tree-optimization/94290 */ > > >> >> >> > +/* { dg-do compile } */ > > >> >> >> > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > > >> >> >> > + > > >> >> >> > + > > >> >> >> > +/* Same form as PR. */ > > >> >> >> > +__attribute__((noipa)) unsigned int foo(int x) { > > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Signed function. */ > > >> >> >> > +__attribute__((noipa)) int bar(int x) { > > >> >> >> > + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Commutative property. */ > > >> >> >> > +__attribute__((noipa)) unsigned int baz(int x) { > > >> >> >> > + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Flipped order for max expressions. */ > > >> >> >> > +__attribute__((noipa)) unsigned int quux(int x) { > > >> >> >> > + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Not zero so should not optimize. */ > > >> >> >> > +__attribute__((noipa)) unsigned int waldo(int x) { > > >> >> >> > + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Not zero so should not optimize. */ > > >> >> >> > +__attribute__((noipa)) unsigned int fred(int x) { > > >> >> >> > + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Incorrect pattern. */ > > >> >> >> > +__attribute__((noipa)) unsigned int goo(int x) { > > >> >> >> > + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* Incorrect pattern. */ > > >> >> >> > +__attribute__((noipa)) int qux(int x) { > > >> >> >> > + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); > > >> >> >> > +} > > >> >> >> > + > > >> >> >> > +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 4 > "optimized" } > > >> } */ > > >> >> >> > > > >> >> >> > base-commit: 6af530f914801f5e561057da55c41480f28751f7 > > >> >> >> > -- > > >> >> >> > 2.31.1 > > >> >> >> > > > >> >> >> > > >> >> > > >> > > >> > > [-- Attachment #2: 0001-match.pd-Add-new-abs-pattern-PR94920.patch --] [-- Type: text/x-patch, Size: 5233 bytes --] From 96e2dabedfa0203bcd75aae5f544f3e57ee57a1c Mon Sep 17 00:00:00 2001 From: Sam Feifer <sfeifer@redhat.com> Date: Mon, 18 Jul 2022 13:18:39 -0400 Subject: [PATCH] match.pd: Add new abs pattern [PR94920] This patch is intended to fix a missed optimization in match.pd. It optimizes (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) to just abs(x). Additionally, the pattern (x <= 0 ? -x : 0) now gets optimized to max(-x, 0), which helps with the other simplification rule. Tests are also included to be added to the testsuite. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR tree-optimization/94920 gcc/ChangeLog: * match.pd (x >= 0 ? x : 0) + (x <= 0 ? -x : 0): New simplification. * match.pd (x <= 0 ? -x : 0): New simplification. gcc/testsuite/ChangeLog: * g++.dg/pr94920-1.C: New test. * g++.dg/pr94920.C: New test. * gcc.dg/pr94920-2.c: New test. --- gcc/match.pd | 10 +++++ gcc/testsuite/g++.dg/pr94920-1.C | 17 +++++++++ gcc/testsuite/g++.dg/pr94920.C | 63 ++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/pr94920-2.c | 15 ++++++++ 4 files changed, 105 insertions(+) create mode 100644 gcc/testsuite/g++.dg/pr94920-1.C create mode 100644 gcc/testsuite/g++.dg/pr94920.C create mode 100644 gcc/testsuite/gcc.dg/pr94920-2.c diff --git a/gcc/match.pd b/gcc/match.pd index 45aefd96688..714c8133ccf 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -339,6 +339,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0))) (COPYSIGN_ALL (negate @0) @1))) +/* (x >= 0 ? x : 0) + (x <= 0 ? -x : 0) -> abs x. */ +(simplify + (plus:c (max @0 integer_zerop) (max (negate @0) integer_zerop)) + (abs @0)) + /* X * 1, X / 1 -> X. */ (for op (mult trunc_div ceil_div floor_div round_div exact_div) (simplify @@ -3316,6 +3321,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && (GIMPLE || !TREE_SIDE_EFFECTS (@1))) (cond (convert:boolean_type_node @2) @1 @0))) +/* (x <= 0 ? -x : 0) -> max(-x, 0). */ +(simplify + (cond (le @0 integer_zerop@1) (negate@2 @0) integer_zerop@1) + (max @2 @1)) + /* Simplifications of shift and rotates. */ (for rotate (lrotate rrotate) diff --git a/gcc/testsuite/g++.dg/pr94920-1.C b/gcc/testsuite/g++.dg/pr94920-1.C new file mode 100644 index 00000000000..6c6483eab2d --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94920-1.C @@ -0,0 +1,17 @@ +/* PR tree-optimization/94920 */ +/* { dg-do run } */ + +#include "pr94920.C" + +int main() { + + if (foo(0) != 0 + || foo(-42) != 42 + || foo(42) != 42 + || baz(-10) != 10 + || baz(-10) != 10) { + __builtin_abort(); + } + + return 0; +} diff --git a/gcc/testsuite/g++.dg/pr94920.C b/gcc/testsuite/g++.dg/pr94920.C new file mode 100644 index 00000000000..925ec4f42f1 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr94920.C @@ -0,0 +1,63 @@ +/* PR tree-optimization/94920 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +typedef int __attribute__((vector_size(4*sizeof(int)))) vint; + +/* Same form as PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Test for forward propogation. */ +__attribute__((noipa)) unsigned int corge(int x) { + int w = (x >= 0 ? x : 0); + int y = -x; + int z = (y >= 0 ? y : 0); + return w + z; +} + +/* Vector case. */ +__attribute__((noipa)) vint thud(vint x) { + vint t = (x >= 0 ? x : 0) ; + vint xx = -x; + vint t1 = (xx >= 0 ? xx : 0); + return t + t1; +} + +/* Signed function. */ +__attribute__((noipa)) int bar(int x) { + return (x >= 0 ? x : 0) + (x <= 0 ? -x : 0); +} + +/* Commutative property. */ +__attribute__((noipa)) unsigned int baz(int x) { + return (x <= 0 ? -x : 0) + (x >= 0 ? x : 0); +} + +/* Flipped order for max expressions. */ +__attribute__((noipa)) unsigned int quux(int x) { + return (0 <= x ? x : 0) + (0 >= x ? -x : 0); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int waldo(int x) { + return (x >= 4 ? x : 4) + (x <= 4 ? -x : 4); +} + +/* Not zero so should not optimize. */ +__attribute__((noipa)) unsigned int fred(int x) { + return (x >= -4 ? x : -4) + (x <= -4 ? -x : -4); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) unsigned int goo(int x) { + return (x <= 0 ? x : 0) + (x >= 0 ? -x : 0); +} + +/* Incorrect pattern. */ +__attribute__((noipa)) int qux(int x) { + return (x >= 0 ? x : 0) + (x >= 0 ? x : 0); +} + +/* { dg-final {scan-tree-dump-times " ABS_EXPR " 6 "optimized" } } */ diff --git a/gcc/testsuite/gcc.dg/pr94920-2.c b/gcc/testsuite/gcc.dg/pr94920-2.c new file mode 100644 index 00000000000..a2d23324cfa --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr94920-2.c @@ -0,0 +1,15 @@ +/* PR tree-optimization/94920 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ + +/* Form from PR. */ +__attribute__((noipa)) unsigned int foo(int x) { + return x <= 0 ? -x : 0; +} + +/* Changed order. */ +__attribute__((noipa)) unsigned int bar(int x) { + return 0 >= x ? -x : 0; +} + +/* { dg-final {scan-tree-dump-times " MAX_EXPR " 2 "optimized" } } */ base-commit: 6af530f914801f5e561057da55c41480f28751f7 -- 2.31.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-07-19 13:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-13 19:24 [PATCH] match.pd: Add new abs pattern [PR94290] Sam Feifer 2022-07-13 19:35 ` Andrew Pinski 2022-07-14 6:52 ` Richard Biener 2022-07-14 14:09 ` Sam Feifer 2022-07-14 17:24 ` Andrew Pinski 2022-07-14 19:38 ` Sam Feifer 2022-07-14 19:53 ` Andrew Pinski 2022-07-18 13:07 ` Sam Feifer 2022-07-18 17:30 ` Sam Feifer 2022-07-19 6:36 ` Richard Biener 2022-07-19 13:48 ` Sam Feifer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).