diff mbox series

tree-optimization/110838 - vectorization of widened right shifts

Message ID 20230804101959.1008513904@imap2.suse-dmz.suse.de
State Unresolved
Headers
Series tree-optimization/110838 - vectorization of widened right shifts |

Checks

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

Commit Message

Richard Biener Aug. 4, 2023, 10:19 a.m. UTC 
  The following fixes a problem with my last attempt of avoiding
out-of-bound shift values for vectorized right shifts of widened
operands.  Instead of truncating the shift amount with a bitwise
and we actually need to saturate it to the target precision.

The following does that and adds test coverage for the constant
and invariant but variable case that would previously have failed.

Bootstrap & regtest on x86_64-unknown-linux-gnu in progress, I plan
to push this soon, just in case you have any comments here.

Richard.

	PR tree-optimization/110838
	* tree-vect-patterns.cc (vect_recog_over_widening_pattern):
	Fix right-shift value sanitizing.  Properly emit external
	def mangling in the preheader rather than in the pattern
	def sequence where it will fail vectorizing.

	* gcc.dg/vect/pr110838.c: New testcase.
---
 gcc/testsuite/gcc.dg/vect/pr110838.c | 31 ++++++++++++++++++++++++++++
 gcc/tree-vect-patterns.cc            | 22 +++++++++++++++-----
 2 files changed, 48 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/pr110838.c

Comments

Richard Sandiford Aug. 4, 2023, 1:28 p.m. UTC | #1 
Richard Biener <rguenther@suse.de> writes:
> The following fixes a problem with my last attempt of avoiding
> out-of-bound shift values for vectorized right shifts of widened
> operands.  Instead of truncating the shift amount with a bitwise
> and we actually need to saturate it to the target precision.
>
> The following does that and adds test coverage for the constant
> and invariant but variable case that would previously have failed.
>
> Bootstrap & regtest on x86_64-unknown-linux-gnu in progress, I plan
> to push this soon, just in case you have any comments here.

LGTM FWIW.

Richard

> Richard.
>
> 	PR tree-optimization/110838
> 	* tree-vect-patterns.cc (vect_recog_over_widening_pattern):
> 	Fix right-shift value sanitizing.  Properly emit external
> 	def mangling in the preheader rather than in the pattern
> 	def sequence where it will fail vectorizing.
>
> 	* gcc.dg/vect/pr110838.c: New testcase.
> ---
>  gcc/testsuite/gcc.dg/vect/pr110838.c | 31 ++++++++++++++++++++++++++++
>  gcc/tree-vect-patterns.cc            | 22 +++++++++++++++-----
>  2 files changed, 48 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/pr110838.c
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr110838.c b/gcc/testsuite/gcc.dg/vect/pr110838.c
> new file mode 100644
> index 00000000000..cf8765be603
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr110838.c
> @@ -0,0 +1,31 @@
> +/* { dg-do run } */
> +
> +#include "tree-vect.h"
> +
> +short a[32], b[32];
> +
> +void __attribute__((noipa)) foo ()
> +{
> +  for (int i = 0; i < 32; ++i)
> +    a[i] = b[i] >> 16;
> +}
> +
> +void __attribute__((noipa)) bar (int n)
> +{
> +  int np = n & 31;
> +  for (int i = 0; i < 32; ++i)
> +    a[i] = b[i] >> np;
> +}
> +
> +int main ()
> +{
> +  check_vect ();
> +  b[0] = -8;
> +  foo ();
> +  if (a[0] != -1)
> +    abort ();
> +  bar (16);
> +  if (a[0] != -1)
> +    abort ();
> +  return 0;
> +}
> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> index e4ab8c2d65b..2cedf238450 100644
> --- a/gcc/tree-vect-patterns.cc
> +++ b/gcc/tree-vect-patterns.cc
> @@ -3109,8 +3109,8 @@ vect_recog_over_widening_pattern (vec_info *vinfo,
>        wide_int min_value, max_value;
>        if (TREE_CODE (ops[1]) == INTEGER_CST)
>  	ops[1] = wide_int_to_tree (op_type,
> -				   wi::bit_and (wi::to_wide (ops[1]),
> -						new_precision - 1));
> +				   wi::umin (wi::to_wide (ops[1]),
> +					     new_precision - 1));
>        else if (!vect_get_range_info (ops[1], &min_value, &max_value)
>  	       || wi::ge_p (max_value, new_precision, TYPE_SIGN (op_type)))
>  	{
> @@ -3118,11 +3118,23 @@ vect_recog_over_widening_pattern (vec_info *vinfo,
>  	     same argument widened shifts and it un-CSEs same arguments.  */
>  	  tree new_var = vect_recog_temp_ssa_var (op_type, NULL);
>  	  gimple *pattern_stmt
> -	    = gimple_build_assign (new_var, BIT_AND_EXPR, ops[1],
> +	    = gimple_build_assign (new_var, MIN_EXPR, ops[1],
>  				   build_int_cst (op_type, new_precision - 1));
> -	  ops[1] = new_var;
>  	  gimple_set_location (pattern_stmt, gimple_location (last_stmt));
> -	  append_pattern_def_seq (vinfo, last_stmt_info, pattern_stmt);
> +	  if (unprom[1].dt == vect_external_def)
> +	    {
> +	      if (edge e = vect_get_external_def_edge (vinfo, ops[1]))
> +		{
> +		  basic_block new_bb
> +		    = gsi_insert_on_edge_immediate (e, pattern_stmt);
> +		  gcc_assert (!new_bb);
> +		}
> +	      else
> +		return NULL;
> +	    }
> +	  else
> +	    append_pattern_def_seq (vinfo, last_stmt_info, pattern_stmt);
> +	  ops[1] = new_var;
>  	}
>      }
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/vect/pr110838.c b/gcc/testsuite/gcc.dg/vect/pr110838.c
new file mode 100644
index 00000000000..cf8765be603
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr110838.c
@@ -0,0 +1,31 @@ 
+/* { dg-do run } */
+
+#include "tree-vect.h"
+
+short a[32], b[32];
+
+void __attribute__((noipa)) foo ()
+{
+  for (int i = 0; i < 32; ++i)
+    a[i] = b[i] >> 16;
+}
+
+void __attribute__((noipa)) bar (int n)
+{
+  int np = n & 31;
+  for (int i = 0; i < 32; ++i)
+    a[i] = b[i] >> np;
+}
+
+int main ()
+{
+  check_vect ();
+  b[0] = -8;
+  foo ();
+  if (a[0] != -1)
+    abort ();
+  bar (16);
+  if (a[0] != -1)
+    abort ();
+  return 0;
+}
diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
index e4ab8c2d65b..2cedf238450 100644
--- a/gcc/tree-vect-patterns.cc
+++ b/gcc/tree-vect-patterns.cc
@@ -3109,8 +3109,8 @@  vect_recog_over_widening_pattern (vec_info *vinfo,
       wide_int min_value, max_value;
       if (TREE_CODE (ops[1]) == INTEGER_CST)
 	ops[1] = wide_int_to_tree (op_type,
-				   wi::bit_and (wi::to_wide (ops[1]),
-						new_precision - 1));
+				   wi::umin (wi::to_wide (ops[1]),
+					     new_precision - 1));
       else if (!vect_get_range_info (ops[1], &min_value, &max_value)
 	       || wi::ge_p (max_value, new_precision, TYPE_SIGN (op_type)))
 	{
@@ -3118,11 +3118,23 @@  vect_recog_over_widening_pattern (vec_info *vinfo,
 	     same argument widened shifts and it un-CSEs same arguments.  */
 	  tree new_var = vect_recog_temp_ssa_var (op_type, NULL);
 	  gimple *pattern_stmt
-	    = gimple_build_assign (new_var, BIT_AND_EXPR, ops[1],
+	    = gimple_build_assign (new_var, MIN_EXPR, ops[1],
 				   build_int_cst (op_type, new_precision - 1));
-	  ops[1] = new_var;
 	  gimple_set_location (pattern_stmt, gimple_location (last_stmt));
-	  append_pattern_def_seq (vinfo, last_stmt_info, pattern_stmt);
+	  if (unprom[1].dt == vect_external_def)
+	    {
+	      if (edge e = vect_get_external_def_edge (vinfo, ops[1]))
+		{
+		  basic_block new_bb
+		    = gsi_insert_on_edge_immediate (e, pattern_stmt);
+		  gcc_assert (!new_bb);
+		}
+	      else
+		return NULL;
+	    }
+	  else
+	    append_pattern_def_seq (vinfo, last_stmt_info, pattern_stmt);
+	  ops[1] = new_var;
 	}
     }