vect: Fix vect_reduction_def check for odd/even widen mult [PR116142]

Message ID 20240807055308.3909784-2-xry111@xry111.site
State Committed
Commit 2083389a18d36684a88d9e2653bacc87ad894b50
Headers
Series vect: Fix vect_reduction_def check for odd/even widen mult [PR116142] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 warning Patch is already merged

Commit Message

Xi Ruoyao Aug. 7, 2024, 5:52 a.m. UTC
  The check was implemented incorrectly, so vec_widen_smult_{even,odd}_M
was never used.  This is not good for targets with native even/odd
widening multiplication but not lo/hi multiplication.

The fix is actually developed by Richard Biener.

gcc/ChangeLog:

	PR tree-optimization/116142
	* tree-vect-stmts.cc (supportable_widening_operation): Remove an
	redundant and incorrect vect_reduction_def check, and fix the
	operand of another vect_reduction_def check.

gcc/testsuite/ChangeLog:
	PR tree-optimization/116142
	* gcc.target/i386/pr116142.c: New test.
---

Bootstrapped and regtested on x86_64-linux-gnu.  Ok for trunk?

 gcc/testsuite/gcc.target/i386/pr116142.c | 18 ++++++++++++++++++
 gcc/tree-vect-stmts.cc                   |  3 +--
 2 files changed, 19 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr116142.c
  

Comments

Sam James Aug. 7, 2024, 6:01 a.m. UTC | #1
Xi Ruoyao <xry111@xry111.site> writes:

> The check was implemented incorrectly, so vec_widen_smult_{even,odd}_M
> was never used.  This is not good for targets with native even/odd
> widening multiplication but not lo/hi multiplication.
>
> The fix is actually developed by Richard Biener.

Please use Co-authored-by from
https://gcc.gnu.org/codingconventions.html#ChangeLogs.

> [...]

thanks,
sam
  
Richard Biener Aug. 7, 2024, 7:22 a.m. UTC | #2
On Wed, 7 Aug 2024, Xi Ruoyao wrote:

> The check was implemented incorrectly, so vec_widen_smult_{even,odd}_M
> was never used.  This is not good for targets with native even/odd
> widening multiplication but not lo/hi multiplication.
> 
> The fix is actually developed by Richard Biener.

OK.

Thanks,
Richard.

> gcc/ChangeLog:
> 
> 	PR tree-optimization/116142
> 	* tree-vect-stmts.cc (supportable_widening_operation): Remove an
> 	redundant and incorrect vect_reduction_def check, and fix the
> 	operand of another vect_reduction_def check.
> 
> gcc/testsuite/ChangeLog:
> 	PR tree-optimization/116142
> 	* gcc.target/i386/pr116142.c: New test.
> ---
> 
> Bootstrapped and regtested on x86_64-linux-gnu.  Ok for trunk?
> 
>  gcc/testsuite/gcc.target/i386/pr116142.c | 18 ++++++++++++++++++
>  gcc/tree-vect-stmts.cc                   |  3 +--
>  2 files changed, 19 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr116142.c
> 
> diff --git a/gcc/testsuite/gcc.target/i386/pr116142.c b/gcc/testsuite/gcc.target/i386/pr116142.c
> new file mode 100644
> index 00000000000..d288a50b237
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr116142.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mavx512f -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump "WIDEN_MULT_EVEN_EXPR" "optimized" } } */
> +/* { dg-final { scan-tree-dump "WIDEN_MULT_ODD_EXPR" "optimized" } } */
> +
> +typedef __INT32_TYPE__ i32;
> +typedef __INT64_TYPE__ i64;
> +
> +i32 x[16], y[16];
> +
> +i64
> +test (void)
> +{
> +  i64 ret = 0;
> +  for (int i = 0; i < 16; i++)
> +    ret ^= (i64) x[i] * y[i];
> +  return ret;
> +}
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 20cae83e820..385e63163c2 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -14179,7 +14179,6 @@ supportable_widening_operation (vec_info *vinfo,
>  	 are properly set up for the caller.  If we fail, we'll continue with
>  	 a VEC_WIDEN_MULT_LO/HI_EXPR check.  */
>        if (vect_loop
> -	  && STMT_VINFO_RELEVANT (stmt_info) == vect_used_by_reduction
>  	  && !nested_in_vect_loop_p (vect_loop, stmt_info)
>  	  && supportable_widening_operation (vinfo, VEC_WIDEN_MULT_EVEN_EXPR,
>  					     stmt_info, vectype_out,
> @@ -14192,7 +14191,7 @@ supportable_widening_operation (vec_info *vinfo,
>               same operation.  One such an example is s += a * b, where elements
>               in a and b cannot be reordered.  Here we check if the vector defined
>               by STMT is only directly used in the reduction statement.  */
> -	  tree lhs = gimple_assign_lhs (stmt_info->stmt);
> +	  tree lhs = gimple_assign_lhs (vect_orig_stmt (stmt_info)->stmt);
>  	  stmt_vec_info use_stmt_info = loop_info->lookup_single_use (lhs);
>  	  if (use_stmt_info
>  	      && STMT_VINFO_DEF_TYPE (use_stmt_info) == vect_reduction_def)
>
  
Xi Ruoyao Aug. 7, 2024, 8:06 a.m. UTC | #3
On Wed, 2024-08-07 at 07:01 +0100, Sam James wrote:
> Xi Ruoyao <xry111@xry111.site> writes:
> 
> > The check was implemented incorrectly, so
> > vec_widen_smult_{even,odd}_M
> > was never used.  This is not good for targets with native even/odd
> > widening multiplication but not lo/hi multiplication.
> > 
> > The fix is actually developed by Richard Biener.
> 
> Please use Co-authored-by from
> https://gcc.gnu.org/codingconventions.html#ChangeLogs.
> 
> > [...]
> 
> thanks,
> sam

Pushed https://gcc.gnu.org/r15-2791 with Co-authored-by added.
  

Patch

diff --git a/gcc/testsuite/gcc.target/i386/pr116142.c b/gcc/testsuite/gcc.target/i386/pr116142.c
new file mode 100644
index 00000000000..d288a50b237
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr116142.c
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx512f -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump "WIDEN_MULT_EVEN_EXPR" "optimized" } } */
+/* { dg-final { scan-tree-dump "WIDEN_MULT_ODD_EXPR" "optimized" } } */
+
+typedef __INT32_TYPE__ i32;
+typedef __INT64_TYPE__ i64;
+
+i32 x[16], y[16];
+
+i64
+test (void)
+{
+  i64 ret = 0;
+  for (int i = 0; i < 16; i++)
+    ret ^= (i64) x[i] * y[i];
+  return ret;
+}
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index 20cae83e820..385e63163c2 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -14179,7 +14179,6 @@  supportable_widening_operation (vec_info *vinfo,
 	 are properly set up for the caller.  If we fail, we'll continue with
 	 a VEC_WIDEN_MULT_LO/HI_EXPR check.  */
       if (vect_loop
-	  && STMT_VINFO_RELEVANT (stmt_info) == vect_used_by_reduction
 	  && !nested_in_vect_loop_p (vect_loop, stmt_info)
 	  && supportable_widening_operation (vinfo, VEC_WIDEN_MULT_EVEN_EXPR,
 					     stmt_info, vectype_out,
@@ -14192,7 +14191,7 @@  supportable_widening_operation (vec_info *vinfo,
              same operation.  One such an example is s += a * b, where elements
              in a and b cannot be reordered.  Here we check if the vector defined
              by STMT is only directly used in the reduction statement.  */
-	  tree lhs = gimple_assign_lhs (stmt_info->stmt);
+	  tree lhs = gimple_assign_lhs (vect_orig_stmt (stmt_info)->stmt);
 	  stmt_vec_info use_stmt_info = loop_info->lookup_single_use (lhs);
 	  if (use_stmt_info
 	      && STMT_VINFO_DEF_TYPE (use_stmt_info) == vect_reduction_def)