[2/3] MATCH: Move jump_function_from_stmt support to match.pd

Message ID 20231029164049.994454-3-pinskia@gmail.com
State Unresolved
Headers
Series start of moving value replacement from phiopt to match |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Andrew Pinski Oct. 29, 2023, 4:40 p.m. UTC
  This moves the value_replacement support for jump_function_from_stmt
to match pattern.
This allows us to optimize things earlier in phiopt1 rather than waiting
to phiopt2. Which means phiopt1 needs to be disable for vrp03.c testcase.

Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	* match.pd (PTR == 0 ? 0 : &PTR->field): New pattern.

gcc/testsuite/ChangeLog:

	* gcc.dg/tree-ssa/vrp03.c: Disable phiopt1.
	* c-c++-common/analyzer/inlining-3-multiline.c: Likewise.
	* c-c++-common/analyzer/inlining-3.c: Likewise.
	* gcc.dg/tree-ssa/phi-opt-value-3.c: New testcase.
---
 gcc/match.pd                                  | 21 ++++++++++++++++++
 .../analyzer/inlining-3-multiline.c           |  5 ++++-
 .../c-c++-common/analyzer/inlining-3.c        |  3 +++
 .../gcc.dg/tree-ssa/phi-opt-value-3.c         | 22 +++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/vrp03.c         |  2 +-
 5 files changed, 51 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
  

Comments

Richard Biener Oct. 30, 2023, 9:29 a.m. UTC | #1
On Sun, Oct 29, 2023 at 5:41 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> This moves the value_replacement support for jump_function_from_stmt
> to match pattern.
> This allows us to optimize things earlier in phiopt1 rather than waiting
> to phiopt2. Which means phiopt1 needs to be disable for vrp03.c testcase.
>
> Bootstrapped and tested on x86_64-linux-gnu.

Do we need to make sure to only do this after pass_early_object_sizes
at least?  IIRC early PHI-opt didn't do value-replacement, so maybe
even after late object-size?  There's PROP_objsz, but no
function similar to optimize_vectors_before_lowering_p in
{generic,gimple}-match-head.cc

Richard.

> gcc/ChangeLog:
>
>         * match.pd (PTR == 0 ? 0 : &PTR->field): New pattern.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/tree-ssa/vrp03.c: Disable phiopt1.
>         * c-c++-common/analyzer/inlining-3-multiline.c: Likewise.
>         * c-c++-common/analyzer/inlining-3.c: Likewise.
>         * gcc.dg/tree-ssa/phi-opt-value-3.c: New testcase.
> ---
>  gcc/match.pd                                  | 21 ++++++++++++++++++
>  .../analyzer/inlining-3-multiline.c           |  5 ++++-
>  .../c-c++-common/analyzer/inlining-3.c        |  3 +++
>  .../gcc.dg/tree-ssa/phi-opt-value-3.c         | 22 +++++++++++++++++++
>  gcc/testsuite/gcc.dg/tree-ssa/vrp03.c         |  2 +-
>  5 files changed, 51 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 22899c51a2f..9bc945ccada 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -4159,6 +4159,27 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>    (cond (eq @0 integer_zerop) @1 (op@2 @1 @0))
>     @2))
>
> +/* PTR == 0 ? 0 : &PTR->field -> PTR if field offset was 0. */
> +(simplify
> + (cond (eq @0 integer_zerop) integer_zerop ADDR_EXPR@1)
> + (with {
> +   poly_int64 offset;
> +   tree res = NULL_TREE;
> +   tree tem = @1;
> +   if (TREE_CODE (tem) == SSA_NAME)
> +     if (gassign *def = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (tem)))
> +       if (gimple_assign_rhs_code (def) == ADDR_EXPR)
> +         tem = gimple_assign_rhs1 (def);
> +
> +   if (TREE_CODE (tem) == ADDR_EXPR)
> +     res = get_addr_base_and_unit_offset (TREE_OPERAND (tem, 0), &offset);
> +  }
> +  (if (res
> +       && TREE_CODE (res) == MEM_REF
> +       && known_eq (mem_ref_offset (res) + offset, 0)
> +       && operand_equal_p (TREE_OPERAND (res, 0), @0))
> +   (convert @0))))
> +
>  /* Simplifications of shift and rotates.  */
>
>  (for rotate (lrotate rrotate)
> diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c b/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
> index fbd20e949b6..9741b91abee 100644
> --- a/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
> +++ b/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
> @@ -3,6 +3,9 @@
>
>  /* { dg-additional-options "-O2 -fdiagnostics-show-path-depths" } */
>  /* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
> +/* Disable phi-opt1 because get_input_file_name gets optimized to just
> +   `return inpf;`. */
> +/* { dg-additional-options "-fdisable-tree-phiopt1" } */
>
>  #include "../../gcc.dg/analyzer/analyzer-decls.h"
>  typedef __SIZE_TYPE__ size_t;
> @@ -96,4 +99,4 @@ test (const input_file *inpf)
>      |                           (4) ...to here
>      |                           (5) argument 1 ('<unknown>') NULL where non-null expected
>      |
> -   { dg-end-multiline-output "" { target c++ } } */
> \ No newline at end of file
> +   { dg-end-multiline-output "" { target c++ } } */
> diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-3.c b/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
> index 0345585bed2..2b2b4858d45 100644
> --- a/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
> +++ b/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
> @@ -2,6 +2,9 @@
>     after early inlining.  */
>
>  /* { dg-additional-options "-O2 -fdiagnostics-show-path-depths" } */
> +/* Disable phi-opt1 because get_input_file_name gets optimized to just
> +   `return inpf;`. */
> +/* { dg-additional-options "-fdisable-tree-phiopt1" } */
>
>  #include "../../gcc.dg/analyzer/analyzer-decls.h"
>  typedef __SIZE_TYPE__ size_t;
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
> new file mode 100644
> index 00000000000..ad55bd288b9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile }  */
> +/* { dg-options "-O1 -fdump-tree-optimized" }  */
> +struct a
> +{
> +        int b[1];
> +};
> +
> +int call1(int *a);
> +
> +int f(struct a *b)
> +{
> +  int *c = b->b;
> +  int t = call1(c);
> +  int *d;
> +  if (b) d = b->b; else d = 0;
> +  int t1 = call1(d);
> +  return t+t1;
> +}
> +
> +/* There should be no if statement and 2 calls to call1. */
> +/* { dg-final { scan-tree-dump-not "if " "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "call1 " 2 "optimized"  } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> index 4cbaca41332..1adbf33cad3 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> @@ -1,5 +1,5 @@
>  /* { dg-do compile } */
> -/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fno-thread-jumps" } */
> +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fno-thread-jumps -fdisable-tree-phiopt1" } */
>
>  struct A
>  {
> --
> 2.39.3
>
  
Andrew Pinski Oct. 30, 2023, 8:36 p.m. UTC | #2
On Mon, Oct 30, 2023 at 2:29 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Sun, Oct 29, 2023 at 5:41 PM Andrew Pinski <pinskia@gmail.com> wrote:
> >
> > This moves the value_replacement support for jump_function_from_stmt
> > to match pattern.
> > This allows us to optimize things earlier in phiopt1 rather than waiting
> > to phiopt2. Which means phiopt1 needs to be disable for vrp03.c testcase.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
>
> Do we need to make sure to only do this after pass_early_object_sizes
> at least?  IIRC early PHI-opt didn't do value-replacement, so maybe
> even after late object-size?  There's PROP_objsz, but no
> function similar to optimize_vectors_before_lowering_p in
> {generic,gimple}-match-head.cc

Let me look into that.
But I suspect any which way we might end up with the same issue as the
problems you found in PR 112266 really.
So I am going to put this patch on the backburner for now (but still
look into this and the fall out from PR 112266 ).

Thanks,
Andrew

>
> Richard.
>
> > gcc/ChangeLog:
> >
> >         * match.pd (PTR == 0 ? 0 : &PTR->field): New pattern.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/tree-ssa/vrp03.c: Disable phiopt1.
> >         * c-c++-common/analyzer/inlining-3-multiline.c: Likewise.
> >         * c-c++-common/analyzer/inlining-3.c: Likewise.
> >         * gcc.dg/tree-ssa/phi-opt-value-3.c: New testcase.
> > ---
> >  gcc/match.pd                                  | 21 ++++++++++++++++++
> >  .../analyzer/inlining-3-multiline.c           |  5 ++++-
> >  .../c-c++-common/analyzer/inlining-3.c        |  3 +++
> >  .../gcc.dg/tree-ssa/phi-opt-value-3.c         | 22 +++++++++++++++++++
> >  gcc/testsuite/gcc.dg/tree-ssa/vrp03.c         |  2 +-
> >  5 files changed, 51 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 22899c51a2f..9bc945ccada 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -4159,6 +4159,27 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >    (cond (eq @0 integer_zerop) @1 (op@2 @1 @0))
> >     @2))
> >
> > +/* PTR == 0 ? 0 : &PTR->field -> PTR if field offset was 0. */
> > +(simplify
> > + (cond (eq @0 integer_zerop) integer_zerop ADDR_EXPR@1)
> > + (with {
> > +   poly_int64 offset;
> > +   tree res = NULL_TREE;
> > +   tree tem = @1;
> > +   if (TREE_CODE (tem) == SSA_NAME)
> > +     if (gassign *def = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (tem)))
> > +       if (gimple_assign_rhs_code (def) == ADDR_EXPR)
> > +         tem = gimple_assign_rhs1 (def);
> > +
> > +   if (TREE_CODE (tem) == ADDR_EXPR)
> > +     res = get_addr_base_and_unit_offset (TREE_OPERAND (tem, 0), &offset);
> > +  }
> > +  (if (res
> > +       && TREE_CODE (res) == MEM_REF
> > +       && known_eq (mem_ref_offset (res) + offset, 0)
> > +       && operand_equal_p (TREE_OPERAND (res, 0), @0))
> > +   (convert @0))))
> > +
> >  /* Simplifications of shift and rotates.  */
> >
> >  (for rotate (lrotate rrotate)
> > diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c b/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
> > index fbd20e949b6..9741b91abee 100644
> > --- a/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
> > +++ b/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
> > @@ -3,6 +3,9 @@
> >
> >  /* { dg-additional-options "-O2 -fdiagnostics-show-path-depths" } */
> >  /* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
> > +/* Disable phi-opt1 because get_input_file_name gets optimized to just
> > +   `return inpf;`. */
> > +/* { dg-additional-options "-fdisable-tree-phiopt1" } */
> >
> >  #include "../../gcc.dg/analyzer/analyzer-decls.h"
> >  typedef __SIZE_TYPE__ size_t;
> > @@ -96,4 +99,4 @@ test (const input_file *inpf)
> >      |                           (4) ...to here
> >      |                           (5) argument 1 ('<unknown>') NULL where non-null expected
> >      |
> > -   { dg-end-multiline-output "" { target c++ } } */
> > \ No newline at end of file
> > +   { dg-end-multiline-output "" { target c++ } } */
> > diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-3.c b/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
> > index 0345585bed2..2b2b4858d45 100644
> > --- a/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
> > +++ b/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
> > @@ -2,6 +2,9 @@
> >     after early inlining.  */
> >
> >  /* { dg-additional-options "-O2 -fdiagnostics-show-path-depths" } */
> > +/* Disable phi-opt1 because get_input_file_name gets optimized to just
> > +   `return inpf;`. */
> > +/* { dg-additional-options "-fdisable-tree-phiopt1" } */
> >
> >  #include "../../gcc.dg/analyzer/analyzer-decls.h"
> >  typedef __SIZE_TYPE__ size_t;
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
> > new file mode 100644
> > index 00000000000..ad55bd288b9
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
> > @@ -0,0 +1,22 @@
> > +/* { dg-do compile }  */
> > +/* { dg-options "-O1 -fdump-tree-optimized" }  */
> > +struct a
> > +{
> > +        int b[1];
> > +};
> > +
> > +int call1(int *a);
> > +
> > +int f(struct a *b)
> > +{
> > +  int *c = b->b;
> > +  int t = call1(c);
> > +  int *d;
> > +  if (b) d = b->b; else d = 0;
> > +  int t1 = call1(d);
> > +  return t+t1;
> > +}
> > +
> > +/* There should be no if statement and 2 calls to call1. */
> > +/* { dg-final { scan-tree-dump-not "if " "optimized" } } */
> > +/* { dg-final { scan-tree-dump-times "call1 " 2 "optimized"  } } */
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> > index 4cbaca41332..1adbf33cad3 100644
> > --- a/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
> > @@ -1,5 +1,5 @@
> >  /* { dg-do compile } */
> > -/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fno-thread-jumps" } */
> > +/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fno-thread-jumps -fdisable-tree-phiopt1" } */
> >
> >  struct A
> >  {
> > --
> > 2.39.3
> >
  

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index 22899c51a2f..9bc945ccada 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4159,6 +4159,27 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
   (cond (eq @0 integer_zerop) @1 (op@2 @1 @0))
    @2))
 
+/* PTR == 0 ? 0 : &PTR->field -> PTR if field offset was 0. */
+(simplify
+ (cond (eq @0 integer_zerop) integer_zerop ADDR_EXPR@1)
+ (with {
+   poly_int64 offset;
+   tree res = NULL_TREE;
+   tree tem = @1;
+   if (TREE_CODE (tem) == SSA_NAME)
+     if (gassign *def = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (tem)))
+	if (gimple_assign_rhs_code (def) == ADDR_EXPR)
+	  tem = gimple_assign_rhs1 (def);
+
+   if (TREE_CODE (tem) == ADDR_EXPR)
+     res = get_addr_base_and_unit_offset (TREE_OPERAND (tem, 0), &offset);
+  }
+  (if (res
+       && TREE_CODE (res) == MEM_REF
+       && known_eq (mem_ref_offset (res) + offset, 0)
+       && operand_equal_p (TREE_OPERAND (res, 0), @0))
+   (convert @0))))
+
 /* Simplifications of shift and rotates.  */
 
 (for rotate (lrotate rrotate)
diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c b/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
index fbd20e949b6..9741b91abee 100644
--- a/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
+++ b/gcc/testsuite/c-c++-common/analyzer/inlining-3-multiline.c
@@ -3,6 +3,9 @@ 
 
 /* { dg-additional-options "-O2 -fdiagnostics-show-path-depths" } */
 /* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+/* Disable phi-opt1 because get_input_file_name gets optimized to just
+   `return inpf;`. */
+/* { dg-additional-options "-fdisable-tree-phiopt1" } */
 
 #include "../../gcc.dg/analyzer/analyzer-decls.h"
 typedef __SIZE_TYPE__ size_t;
@@ -96,4 +99,4 @@  test (const input_file *inpf)
     |                           (4) ...to here
     |                           (5) argument 1 ('<unknown>') NULL where non-null expected
     |
-   { dg-end-multiline-output "" { target c++ } } */
\ No newline at end of file
+   { dg-end-multiline-output "" { target c++ } } */
diff --git a/gcc/testsuite/c-c++-common/analyzer/inlining-3.c b/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
index 0345585bed2..2b2b4858d45 100644
--- a/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
+++ b/gcc/testsuite/c-c++-common/analyzer/inlining-3.c
@@ -2,6 +2,9 @@ 
    after early inlining.  */
 
 /* { dg-additional-options "-O2 -fdiagnostics-show-path-depths" } */
+/* Disable phi-opt1 because get_input_file_name gets optimized to just
+   `return inpf;`. */
+/* { dg-additional-options "-fdisable-tree-phiopt1" } */
 
 #include "../../gcc.dg/analyzer/analyzer-decls.h"
 typedef __SIZE_TYPE__ size_t;
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
new file mode 100644
index 00000000000..ad55bd288b9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/phi-opt-value-3.c
@@ -0,0 +1,22 @@ 
+/* { dg-do compile }  */
+/* { dg-options "-O1 -fdump-tree-optimized" }  */
+struct a
+{
+        int b[1];
+};
+
+int call1(int *a);
+
+int f(struct a *b)
+{
+  int *c = b->b;
+  int t = call1(c);
+  int *d;
+  if (b) d = b->b; else d = 0;
+  int t1 = call1(d);
+  return t+t1;
+}
+
+/* There should be no if statement and 2 calls to call1. */
+/* { dg-final { scan-tree-dump-not "if " "optimized" } } */
+/* { dg-final { scan-tree-dump-times "call1 " 2 "optimized"  } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
index 4cbaca41332..1adbf33cad3 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp03.c
@@ -1,5 +1,5 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fno-thread-jumps" } */
+/* { dg-options "-O2 -fdisable-tree-evrp -fdump-tree-vrp1 -fno-thread-jumps -fdisable-tree-phiopt1" } */
 
 struct A
 {