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 > >