[PATCH] middle-end/114070 - folding breaking VEC_COND expansion

[PATCH] middle-end/114070 - folding breaking VEC_COND expansion

[Richard Biener]

The following properly guards the simplifications that move
operations into VEC_CONDs, in particular when that changes the
type constraints on this operation.

This needed a genmatch fix which was recording spurious implicit fors
when tcc_comparison is used in a C expression.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

	PR middle-end/114070
	* genmatch.cc (parser::parse_c_expr): Do not record operand
	lists but only mark operators used.
	* match.pd ((c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e)):
	Properly guard the case of tcc_comparison changing the VEC_COND
	value operand type.

	* gcc.dg/torture/pr114070.c: New testcase.
---
 gcc/genmatch.cc                         |  6 ++----
 gcc/match.pd                            | 15 ++++++++++++---
 gcc/testsuite/gcc.dg/torture/pr114070.c | 12 ++++++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr114070.c

diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
index 375ae90ae6c..d9ae436ce5c 100644
--- a/gcc/genmatch.cc
+++ b/gcc/genmatch.cc
@@ -4760,10 +4760,8 @@ parser::parse_c_expr (cpp_ttype start)
 	    = (const char *)CPP_HASHNODE (token->val.node.node)->ident.str;
 	  if (strcmp (str, "return") == 0)
 	    fatal_at (token, "return statement not allowed in C expression");
-	  id_base *idb = get_operator (str);
-	  user_id *p;
-	  if (idb && (p = dyn_cast<user_id *> (idb)) && p->is_oper_list)
-	    record_operlist (token->src_loc, p);
+	  /* Mark user operators corresponding to 'str' as used.  */
+	  get_operator (str);
 	}
 
       /* Record the token.  */
diff --git a/gcc/match.pd b/gcc/match.pd
index c5b6540f939..67007fc2017 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -5149,15 +5149,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 /* (c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e) */
  (simplify
   (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
-  (vec_cond @0 (op! @1 @3) (op! @2 @4)))
+  (if (TREE_CODE_CLASS (op) != tcc_comparison
+       || types_match (type, TREE_TYPE (@1))
+       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
+   (vec_cond @0 (op! @1 @3) (op! @2 @4))))
 
 /* (c ? a : b) op d  -->  c ? (a op d) : (b op d) */
  (simplify
   (op (vec_cond:s @0 @1 @2) @3)
-  (vec_cond @0 (op! @1 @3) (op! @2 @3)))
+  (if (TREE_CODE_CLASS (op) != tcc_comparison
+       || types_match (type, TREE_TYPE (@1))
+       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
+   (vec_cond @0 (op! @1 @3) (op! @2 @3))))
  (simplify
   (op @3 (vec_cond:s @0 @1 @2))
-  (vec_cond @0 (op! @3 @1) (op! @3 @2))))
+  (if (TREE_CODE_CLASS (op) != tcc_comparison
+       || types_match (type, TREE_TYPE (@1))
+       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
+   (vec_cond @0 (op! @3 @1) (op! @3 @2)))))
 
 #if GIMPLE
 (match (nop_atomic_bit_test_and_p @0 @1 @4)
diff --git a/gcc/testsuite/gcc.dg/torture/pr114070.c b/gcc/testsuite/gcc.dg/torture/pr114070.c
new file mode 100644
index 00000000000..cf46ec45a04
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr114070.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-vect-cost-model" } */
+
+int unresolved(unsigned dirmask, unsigned mask, int *unresolved_n)
+{
+  for (int i = 0; i < 1024; i++) {
+    mask |= 1;
+    if (!unresolved_n[i] || unresolved_n[i] & 70000)
+      dirmask |= 1;
+  }
+  return (dirmask == mask);
+}
-- 
2.35.3

Re: [PATCH] middle-end/114070 - folding breaking VEC_COND expansion

[Richard Biener]

[-- Attachment #1: Type: text/plain, Size: 5947 bytes --]

On Wed, 19 Jun 2024, Andrew Pinski wrote:

> On Wed, Jun 19, 2024 at 7:44 AM Vaseeharan Vinayagamoorthy
> <Vaseeharan.Vinayagamoorthy@arm.com> wrote:
> >
> > Hi,
> >
> > I have found that this patch has introduced a regression in the arm-none-eabi toolchain for a testcase, which was previously passing:
> >
> > PASS->FAIL: gcc.dg/tree-ssa/andnot-2.c scan-tree-dump-not forwprop3 "_expr"
> >
> > The toolchain was built with:
> > Build = x86_64-none-linux-gnu
> > Host = x86_64-none-linux-gnu
> > Target = arm-none-eabi
> >
> > This is also affecting the gcc-13 and gcc-14 branches.
> > Could you please let me know the impact of this regression, and whether you plan to fix the regression?
> 
> See the thread starting at
> https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646587.html
> for information on the testcase regression and what needs to be done.
> I suspect this only now effects targets which don't have a vector
> modes enabled.

I think it should be OK for those.  The problematical ones are those
with no or partial vcond_mask support, but since we try to get
rid of vcond{,u,eq} that case should fix itself as well.

So no, I don't plan to fix anything besides pushing for the latter
to happen for GCC 15.

Richard.

> Note it is a (minor) missed optimization regression so the impact
> looks to be small.
> I am not sure if people have written code with this pattern, it
> requires vectors and it fails only on targets where there is no vector
> support enabled.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95906
> 
> Thanks,
> Andrew Pinski
> 
> >
> >
> > Kind regards,
> > Vasee
> >
> > ________________________________________
> > From: Richard Biener <rguenther@suse.de>
> > Sent: 26 February 2024 07:42
> > To: gcc-patches@gcc.gnu.org
> > Subject: [PATCH] middle-end/114070 - folding breaking VEC_COND expansion
> >
> > The following properly guards the simplifications that move
> > operations into VEC_CONDs, in particular when that changes the
> > type constraints on this operation.
> >
> > This needed a genmatch fix which was recording spurious implicit fors
> > when tcc_comparison is used in a C expression.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
> >
> >         PR middle-end/114070
> >         * genmatch.cc (parser::parse_c_expr): Do not record operand
> >         lists but only mark operators used.
> >         * match.pd ((c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e)):
> >         Properly guard the case of tcc_comparison changing the VEC_COND
> >         value operand type.
> >
> >         * gcc.dg/torture/pr114070.c: New testcase.
> > ---
> >  gcc/genmatch.cc                         |  6 ++----
> >  gcc/match.pd                            | 15 ++++++++++++---
> >  gcc/testsuite/gcc.dg/torture/pr114070.c | 12 ++++++++++++
> >  3 files changed, 26 insertions(+), 7 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/torture/pr114070.c
> >
> > diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> > index 375ae90ae6c..d9ae436ce5c 100644
> > --- a/gcc/genmatch.cc
> > +++ b/gcc/genmatch.cc
> > @@ -4760,10 +4760,8 @@ parser::parse_c_expr (cpp_ttype start)
> >             = (const char *)CPP_HASHNODE (token->val.node.node)->ident.str;
> >           if (strcmp (str, "return") == 0)
> >             fatal_at (token, "return statement not allowed in C expression");
> > -         id_base *idb = get_operator (str);
> > -         user_id *p;
> > -         if (idb && (p = dyn_cast<user_id *> (idb)) && p->is_oper_list)
> > -           record_operlist (token->src_loc, p);
> > +         /* Mark user operators corresponding to 'str' as used.  */
> > +         get_operator (str);
> >         }
> >
> >        /* Record the token.  */
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index c5b6540f939..67007fc2017 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -5149,15 +5149,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >  /* (c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e) */
> >   (simplify
> >    (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
> > -  (vec_cond @0 (op! @1 @3) (op! @2 @4)))
> > +  (if (TREE_CODE_CLASS (op) != tcc_comparison
> > +       || types_match (type, TREE_TYPE (@1))
> > +       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
> > +   (vec_cond @0 (op! @1 @3) (op! @2 @4))))
> >
> >  /* (c ? a : b) op d  -->  c ? (a op d) : (b op d) */
> >   (simplify
> >    (op (vec_cond:s @0 @1 @2) @3)
> > -  (vec_cond @0 (op! @1 @3) (op! @2 @3)))
> > +  (if (TREE_CODE_CLASS (op) != tcc_comparison
> > +       || types_match (type, TREE_TYPE (@1))
> > +       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
> > +   (vec_cond @0 (op! @1 @3) (op! @2 @3))))
> >   (simplify
> >    (op @3 (vec_cond:s @0 @1 @2))
> > -  (vec_cond @0 (op! @3 @1) (op! @3 @2))))
> > +  (if (TREE_CODE_CLASS (op) != tcc_comparison
> > +       || types_match (type, TREE_TYPE (@1))
> > +       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
> > +   (vec_cond @0 (op! @3 @1) (op! @3 @2)))))
> >
> >  #if GIMPLE
> >  (match (nop_atomic_bit_test_and_p @0 @1 @4)
> > diff --git a/gcc/testsuite/gcc.dg/torture/pr114070.c b/gcc/testsuite/gcc.dg/torture/pr114070.c
> > new file mode 100644
> > index 00000000000..cf46ec45a04
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/torture/pr114070.c
> > @@ -0,0 +1,12 @@
> > +/* { dg-do compile } */
> > +/* { dg-additional-options "-fno-vect-cost-model" } */
> > +
> > +int unresolved(unsigned dirmask, unsigned mask, int *unresolved_n)
> > +{
> > +  for (int i = 0; i < 1024; i++) {
> > +    mask |= 1;
> > +    if (!unresolved_n[i] || unresolved_n[i] & 70000)
> > +      dirmask |= 1;
> > +  }
> > +  return (dirmask == mask);
> > +}
> > --
> > 2.35.3
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Re: [PATCH] middle-end/114070 - folding breaking VEC_COND expansion

[Andrew Pinski]

On Wed, Jun 19, 2024 at 7:44 AM Vaseeharan Vinayagamoorthy
<Vaseeharan.Vinayagamoorthy@arm.com> wrote:
>
> Hi,
>
> I have found that this patch has introduced a regression in the arm-none-eabi toolchain for a testcase, which was previously passing:
>
> PASS->FAIL: gcc.dg/tree-ssa/andnot-2.c scan-tree-dump-not forwprop3 "_expr"
>
> The toolchain was built with:
> Build = x86_64-none-linux-gnu
> Host = x86_64-none-linux-gnu
> Target = arm-none-eabi
>
> This is also affecting the gcc-13 and gcc-14 branches.
> Could you please let me know the impact of this regression, and whether you plan to fix the regression?

See the thread starting at
https://gcc.gnu.org/pipermail/gcc-patches/2024-February/646587.html
for information on the testcase regression and what needs to be done.
I suspect this only now effects targets which don't have a vector
modes enabled.

Note it is a (minor) missed optimization regression so the impact
looks to be small.
I am not sure if people have written code with this pattern, it
requires vectors and it fails only on targets where there is no vector
support enabled.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95906

Thanks,
Andrew Pinski

>
>
> Kind regards,
> Vasee
>
> ________________________________________
> From: Richard Biener <rguenther@suse.de>
> Sent: 26 February 2024 07:42
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH] middle-end/114070 - folding breaking VEC_COND expansion
>
> The following properly guards the simplifications that move
> operations into VEC_CONDs, in particular when that changes the
> type constraints on this operation.
>
> This needed a genmatch fix which was recording spurious implicit fors
> when tcc_comparison is used in a C expression.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>
>         PR middle-end/114070
>         * genmatch.cc (parser::parse_c_expr): Do not record operand
>         lists but only mark operators used.
>         * match.pd ((c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e)):
>         Properly guard the case of tcc_comparison changing the VEC_COND
>         value operand type.
>
>         * gcc.dg/torture/pr114070.c: New testcase.
> ---
>  gcc/genmatch.cc                         |  6 ++----
>  gcc/match.pd                            | 15 ++++++++++++---
>  gcc/testsuite/gcc.dg/torture/pr114070.c | 12 ++++++++++++
>  3 files changed, 26 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr114070.c
>
> diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
> index 375ae90ae6c..d9ae436ce5c 100644
> --- a/gcc/genmatch.cc
> +++ b/gcc/genmatch.cc
> @@ -4760,10 +4760,8 @@ parser::parse_c_expr (cpp_ttype start)
>             = (const char *)CPP_HASHNODE (token->val.node.node)->ident.str;
>           if (strcmp (str, "return") == 0)
>             fatal_at (token, "return statement not allowed in C expression");
> -         id_base *idb = get_operator (str);
> -         user_id *p;
> -         if (idb && (p = dyn_cast<user_id *> (idb)) && p->is_oper_list)
> -           record_operlist (token->src_loc, p);
> +         /* Mark user operators corresponding to 'str' as used.  */
> +         get_operator (str);
>         }
>
>        /* Record the token.  */
> diff --git a/gcc/match.pd b/gcc/match.pd
> index c5b6540f939..67007fc2017 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -5149,15 +5149,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>  /* (c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e) */
>   (simplify
>    (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
> -  (vec_cond @0 (op! @1 @3) (op! @2 @4)))
> +  (if (TREE_CODE_CLASS (op) != tcc_comparison
> +       || types_match (type, TREE_TYPE (@1))
> +       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
> +   (vec_cond @0 (op! @1 @3) (op! @2 @4))))
>
>  /* (c ? a : b) op d  -->  c ? (a op d) : (b op d) */
>   (simplify
>    (op (vec_cond:s @0 @1 @2) @3)
> -  (vec_cond @0 (op! @1 @3) (op! @2 @3)))
> +  (if (TREE_CODE_CLASS (op) != tcc_comparison
> +       || types_match (type, TREE_TYPE (@1))
> +       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
> +   (vec_cond @0 (op! @1 @3) (op! @2 @3))))
>   (simplify
>    (op @3 (vec_cond:s @0 @1 @2))
> -  (vec_cond @0 (op! @3 @1) (op! @3 @2))))
> +  (if (TREE_CODE_CLASS (op) != tcc_comparison
> +       || types_match (type, TREE_TYPE (@1))
> +       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
> +   (vec_cond @0 (op! @3 @1) (op! @3 @2)))))
>
>  #if GIMPLE
>  (match (nop_atomic_bit_test_and_p @0 @1 @4)
> diff --git a/gcc/testsuite/gcc.dg/torture/pr114070.c b/gcc/testsuite/gcc.dg/torture/pr114070.c
> new file mode 100644
> index 00000000000..cf46ec45a04
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr114070.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-fno-vect-cost-model" } */
> +
> +int unresolved(unsigned dirmask, unsigned mask, int *unresolved_n)
> +{
> +  for (int i = 0; i < 1024; i++) {
> +    mask |= 1;
> +    if (!unresolved_n[i] || unresolved_n[i] & 70000)
> +      dirmask |= 1;
> +  }
> +  return (dirmask == mask);
> +}
> --
> 2.35.3

Re: [PATCH] middle-end/114070 - folding breaking VEC_COND expansion

[Vaseeharan Vinayagamoorthy]

Hi,

I have found that this patch has introduced a regression in the arm-none-eabi toolchain for a testcase, which was previously passing:

PASS->FAIL: gcc.dg/tree-ssa/andnot-2.c scan-tree-dump-not forwprop3 "_expr"

The toolchain was built with:
Build = x86_64-none-linux-gnu
Host = x86_64-none-linux-gnu
Target = arm-none-eabi

This is also affecting the gcc-13 and gcc-14 branches.
Could you please let me know the impact of this regression, and whether you plan to fix the regression?


Kind regards,
Vasee

________________________________________
From: Richard Biener <rguenther@suse.de>
Sent: 26 February 2024 07:42
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] middle-end/114070 - folding breaking VEC_COND expansion

The following properly guards the simplifications that move
operations into VEC_CONDs, in particular when that changes the
type constraints on this operation.

This needed a genmatch fix which was recording spurious implicit fors
when tcc_comparison is used in a C expression.

Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.

        PR middle-end/114070
        * genmatch.cc (parser::parse_c_expr): Do not record operand
        lists but only mark operators used.
        * match.pd ((c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e)):
        Properly guard the case of tcc_comparison changing the VEC_COND
        value operand type.

        * gcc.dg/torture/pr114070.c: New testcase.
---
 gcc/genmatch.cc                         |  6 ++----
 gcc/match.pd                            | 15 ++++++++++++---
 gcc/testsuite/gcc.dg/torture/pr114070.c | 12 ++++++++++++
 3 files changed, 26 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr114070.c

diff --git a/gcc/genmatch.cc b/gcc/genmatch.cc
index 375ae90ae6c..d9ae436ce5c 100644
--- a/gcc/genmatch.cc
+++ b/gcc/genmatch.cc
@@ -4760,10 +4760,8 @@ parser::parse_c_expr (cpp_ttype start)
            = (const char *)CPP_HASHNODE (token->val.node.node)->ident.str;
          if (strcmp (str, "return") == 0)
            fatal_at (token, "return statement not allowed in C expression");
-         id_base *idb = get_operator (str);
-         user_id *p;
-         if (idb && (p = dyn_cast<user_id *> (idb)) && p->is_oper_list)
-           record_operlist (token->src_loc, p);
+         /* Mark user operators corresponding to 'str' as used.  */
+         get_operator (str);
        }

       /* Record the token.  */
diff --git a/gcc/match.pd b/gcc/match.pd
index c5b6540f939..67007fc2017 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -5149,15 +5149,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 /* (c ? a : b) op (c ? d : e)  -->  c ? (a op d) : (b op e) */
  (simplify
   (op (vec_cond:s @0 @1 @2) (vec_cond:s @0 @3 @4))
-  (vec_cond @0 (op! @1 @3) (op! @2 @4)))
+  (if (TREE_CODE_CLASS (op) != tcc_comparison
+       || types_match (type, TREE_TYPE (@1))
+       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
+   (vec_cond @0 (op! @1 @3) (op! @2 @4))))

 /* (c ? a : b) op d  -->  c ? (a op d) : (b op d) */
  (simplify
   (op (vec_cond:s @0 @1 @2) @3)
-  (vec_cond @0 (op! @1 @3) (op! @2 @3)))
+  (if (TREE_CODE_CLASS (op) != tcc_comparison
+       || types_match (type, TREE_TYPE (@1))
+       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
+   (vec_cond @0 (op! @1 @3) (op! @2 @3))))
  (simplify
   (op @3 (vec_cond:s @0 @1 @2))
-  (vec_cond @0 (op! @3 @1) (op! @3 @2))))
+  (if (TREE_CODE_CLASS (op) != tcc_comparison
+       || types_match (type, TREE_TYPE (@1))
+       || expand_vec_cond_expr_p (type, TREE_TYPE (@0), ERROR_MARK))
+   (vec_cond @0 (op! @3 @1) (op! @3 @2)))))

 #if GIMPLE
 (match (nop_atomic_bit_test_and_p @0 @1 @4)
diff --git a/gcc/testsuite/gcc.dg/torture/pr114070.c b/gcc/testsuite/gcc.dg/torture/pr114070.c
new file mode 100644
index 00000000000..cf46ec45a04
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr114070.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fno-vect-cost-model" } */
+
+int unresolved(unsigned dirmask, unsigned mask, int *unresolved_n)
+{
+  for (int i = 0; i < 1024; i++) {
+    mask |= 1;
+    if (!unresolved_n[i] || unresolved_n[i] & 70000)
+      dirmask |= 1;
+  }
+  return (dirmask == mask);
+}
--
2.35.3