MATCH: first of the value replacement moving from phiopt

Message ID 20230802235232.2265424-1-apinski@marvell.com
State Accepted
Headers
Series MATCH: first of the value replacement moving from phiopt |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski Aug. 2, 2023, 11:52 p.m. UTC
  This moves a few simple patterns that are done in value replacement
in phiopt over to match.pd. Just the simple ones which might show up
in other code.

This allows some optimizations to happen even without depending
on sinking from happening and in some cases where phiopt is not
invoked (cond-1.c is an example there).

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	* match.pd (`a == 0 ? b : b + a`,
	`a == 0 ? b : b - a`): New patterns.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/cond-1.c: New test.
	* gcc.dg/tree-ssa/phi-opt-33.c: New test.
	* gcc.dg/tree-ssa/phi-opt-34.c: New test.
---
 gcc/match.pd                               | 14 ++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/cond-1.c     | 17 +++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-33.c | 19 +++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-34.c | 17 +++++++++++++++++
 4 files changed, 67 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/cond-1.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-33.c
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-34.c
  

Comments

Jeff Law Aug. 3, 2023, 6:10 a.m. UTC | #1
On 8/2/23 17:52, Andrew Pinski via Gcc-patches wrote:
> This moves a few simple patterns that are done in value replacement
> in phiopt over to match.pd. Just the simple ones which might show up
> in other code.
> 
> This allows some optimizations to happen even without depending
> on sinking from happening and in some cases where phiopt is not
> invoked (cond-1.c is an example there).
> 
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> 
> gcc/ChangeLog:
> 
> 	* match.pd (`a == 0 ? b : b + a`,
> 	`a == 0 ? b : b - a`): New patterns.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/tree-ssa/cond-1.c: New test.
> 	* gcc.dg/tree-ssa/phi-opt-33.c: New test.
> 	* gcc.dg/tree-ssa/phi-opt-34.c: New test.
Are you going to remove the old implementation from phiopt?

Jeff
  
Andrew Pinski Aug. 3, 2023, 6:47 a.m. UTC | #2
On Wed, Aug 2, 2023 at 11:10 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 8/2/23 17:52, Andrew Pinski via Gcc-patches wrote:
> > This moves a few simple patterns that are done in value replacement
> > in phiopt over to match.pd. Just the simple ones which might show up
> > in other code.
> >
> > This allows some optimizations to happen even without depending
> > on sinking from happening and in some cases where phiopt is not
> > invoked (cond-1.c is an example there).
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> >       * match.pd (`a == 0 ? b : b + a`,
> >       `a == 0 ? b : b - a`): New patterns.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/tree-ssa/cond-1.c: New test.
> >       * gcc.dg/tree-ssa/phi-opt-33.c: New test.
> >       * gcc.dg/tree-ssa/phi-opt-34.c: New test.
> Are you going to remove the old implementation from phiopt?

That is the plan but I doubt it will be completed for GCC 14 because
it will require a lot of changes to the match-and-simplify part of
phi-opt to support all of what value replacement does and I have not
thought through what is fully required to do that yet.
I know having multiple things in the partial state is not a good idea
but in this case other passes benefit from this change as shown by the
cond-1.c testcase, loop if-conv also uses match-and-simplify when
creating its conditional expressions (since r13-263-g9801ca737b1dcb;
well it uses generic fold since r0-118636-gf35613b29cb159) so that
pass will benefit in this case, removing the conditional even.

I have a few other patches to remove many parts of minmax of phiopt
that I will finish up for GCC 14 though so at least there will be more
code removed from phi-opt for GCC 14.

Thanks,
Andrew

PS here is a testcase where the vectorized code is improved due to this patch:
```
int f(unsigned *a, unsigned *b, unsigned *c, unsigned *d)
{
        for(int i = 0;i < 128; i++)
        {
                unsigned aa = a[i];
                unsigned bb = b[i];
                unsigned cc = c[i];
                unsigned r;
                unsigned r1;
                if (aa == 0)
                {
                  r = bb;
                  r1 = 1;
                } else {
                   r = bb+aa;
                   r1 = 0;
                }
                c[i] = r;
                d[i] = r1;
        }
}
```

>
> Jeff
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index c62f205c13c..1ceff9691a0 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3832,6 +3832,20 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
        && (INTEGRAL_TYPE_P (TREE_TYPE (@0))))
        (op (mult (convert:type @0) @2) @1))))
 
+/* ?: Value replacement. */
+/* a == 0 ? b : b + a  -> b + a */
+(for op (plus bit_ior bit_xor)
+ (simplify
+  (cond (eq @0 integer_zerop) @1 (op:c@2 @1 @0))
+   @2))
+/* a == 0 ? b : b - a  -> b - a */
+/* a == 0 ? b : b ptr+ a  -> b ptr+ a */
+/* a == 0 ? b : b shift/rotate a -> b shift/rotate a */
+(for op (lrotate rrotate lshift rshift minus pointer_plus)
+ (simplify
+  (cond (eq @0 integer_zerop) @1 (op@2 @1 @0))
+   @2))
+
 /* Simplifications of shift and rotates.  */
 
 (for rotate (lrotate rrotate)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/cond-1.c b/gcc/testsuite/gcc.dg/tree-ssa/cond-1.c
new file mode 100644
index 00000000000..478a818b206
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/cond-1.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized-raw" } */
+
+int sub(int a, int b, int c, int d) {
+  int e = (a == 0);
+  int f = !e;
+  c = b;
+  d = b - a ;
+  return ((-e & c) | (-f & d));
+}
+
+/* In the end we end up with `(a == 0) ? (b - a) : b`
+   which then can be optimized to just `(b - a)`. */
+
+/* { dg-final { scan-tree-dump-not "cond_expr," "optimized" } } */
+/* { dg-final { scan-tree-dump-not "eq_expr," "optimized" } } */
+/* { dg-final { scan-tree-dump-times "minus_expr," 1 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-33.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-33.c
new file mode 100644
index 00000000000..809ccfe1479
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-33.c
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+/* Phi-OPT should be able to optimize this without sinking being invoked. */
+/* { dg-options "-O -fdump-tree-phiopt2 -fdump-tree-optimized -fno-tree-sink" } */
+
+int f(int a, int b, int c) {
+  int d = a + b;
+  if (c > 5) return c;
+  if (a == 0) return b;
+  return d;
+}
+
+unsigned rot(unsigned x, int n) {
+  const int bits = __CHAR_BIT__ * __SIZEOF_INT__;
+  int t = ((x << n) | (x >> (bits - n)));
+  return (n == 0) ? x : t;
+}
+
+/* { dg-final { scan-tree-dump-times "goto" 2 "phiopt2" } } */
+/* { dg-final { scan-tree-dump-times "goto" 2 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-34.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-34.c
new file mode 100644
index 00000000000..a90de8926c6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-34.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* Phi-OPT should be able to optimize this without sinking being invoked. */
+/* { dg-options "-O -fdump-tree-phiopt2 -fdump-tree-optimized -fno-tree-sink" } */
+
+char *f(char *a, __SIZE_TYPE__ b) {
+  char *d = a + b;
+  if (b == 0) return a;
+  return d;
+}
+int sub(int a, int b, int c) {
+  int d = a - b;
+  if (b == 0) return a;
+  return d;
+}
+
+/* { dg-final { scan-tree-dump-not "goto" "phiopt2" } } */
+/* { dg-final { scan-tree-dump-not "goto" "optimized" } } */