[PATCH] vect: Check gather/scatter offset types [PR108316]

[PATCH] vect: Check gather/scatter offset types [PR108316]

[Richard Sandiford]

The gather/scatter support can over-widen an offset if the target
requires it, but this relies on using a pattern sequence to add
the widening conversion.  That failed in the testcase because an
earlier pattern (bool) took priority.

I think we should allow patterns to be applied to other patterns,
but that's quite an invasive change and isn't suitable for stage 4.
This patch instead punts if the offset type doesn't match the
expected one.

If we switched to using the SLP representation for everything,
we would probably handle both patterns by rewriting the graph,
which should be much easier.

Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?

Richard


gcc/
	PR tree-optimization/108316
	* tree-vect-stmts.cc (get_load_store_type): When using
	internal functions for gather/scatter, make sure that the type
	of the offset argument is consistent with the offset vector type.

gcc/testsuite/
	PR tree-optimization/108316
	* gcc.dg/vect/pr108316.c: New test.
---
 gcc/testsuite/gcc.dg/vect/pr108316.c | 11 +++++++++++
 gcc/tree-vect-stmts.cc               | 17 +++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/vect/pr108316.c

diff --git a/gcc/testsuite/gcc.dg/vect/pr108316.c b/gcc/testsuite/gcc.dg/vect/pr108316.c
new file mode 100644
index 00000000000..540b7f2aed4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr108316.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+extern _Bool a[];
+
+void
+foo (short i, int b[][64][1])
+{
+  for (; i < 64; i += 4)
+    a[i] = b[0][i] != 0;
+}
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index eb4ca1f184e..c86249adcc3 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -2474,6 +2474,23 @@ get_load_store_type (vec_info  *vinfo, stmt_vec_info stmt_info,
       *memory_access_type = VMAT_GATHER_SCATTER;
       if (!vect_check_gather_scatter (stmt_info, loop_vinfo, gs_info))
 	gcc_unreachable ();
+      /* When using internal functions, we rely on pattern recognition
+	 to convert the type of the offset to the type that the target
+	 requires, with the result being a call to an internal function.
+	 If that failed for some reason (e.g. because another pattern
+	 took priority), just handle cases in which the offset already
+	 has the right type.  */
+      else if (gs_info->ifn != IFN_LAST
+	       && !is_gimple_call (stmt_info->stmt)
+	       && !tree_nop_conversion_p (TREE_TYPE (gs_info->offset),
+					  TREE_TYPE (gs_info->offset_vectype)))
+	{
+	  if (dump_enabled_p ())
+	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+			     "%s offset requires a conversion\n",
+			     vls_type == VLS_LOAD ? "gather" : "scatter");
+	  return false;
+	}
       else if (!vect_is_simple_use (gs_info->offset, vinfo,
 				    &gs_info->offset_dt,
 				    &gs_info->offset_vectype))
-- 
2.25.1

Re: [PATCH] vect: Check gather/scatter offset types [PR108316]

[Richard Biener]

> Am 08.02.2023 um 10:00 schrieb Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> The gather/scatter support can over-widen an offset if the target
> requires it, but this relies on using a pattern sequence to add
> the widening conversion.  That failed in the testcase because an
> earlier pattern (bool) took priority.
> 
> I think we should allow patterns to be applied to other patterns,
> but that's quite an invasive change and isn't suitable for stage 4.
> This patch instead punts if the offset type doesn't match the
> expected one.
> 
> If we switched to using the SLP representation for everything,
> we would probably handle both patterns by rewriting the graph,
> which should be much easier.
> 
> Tested on aarch64-linux-gnu & x86_64-linux-gnu.  OK to install?

Ok.

Richard 

> Richard
> 
> 
> gcc/
>    PR tree-optimization/108316
>    * tree-vect-stmts.cc (get_load_store_type): When using
>    internal functions for gather/scatter, make sure that the type
>    of the offset argument is consistent with the offset vector type.
> 
> gcc/testsuite/
>    PR tree-optimization/108316
>    * gcc.dg/vect/pr108316.c: New test.
> ---
> gcc/testsuite/gcc.dg/vect/pr108316.c | 11 +++++++++++
> gcc/tree-vect-stmts.cc               | 17 +++++++++++++++++
> 2 files changed, 28 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/vect/pr108316.c
> 
> diff --git a/gcc/testsuite/gcc.dg/vect/pr108316.c b/gcc/testsuite/gcc.dg/vect/pr108316.c
> new file mode 100644
> index 00000000000..540b7f2aed4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr108316.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O3" } */
> +
> +extern _Bool a[];
> +
> +void
> +foo (short i, int b[][64][1])
> +{
> +  for (; i < 64; i += 4)
> +    a[i] = b[0][i] != 0;
> +}
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index eb4ca1f184e..c86249adcc3 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -2474,6 +2474,23 @@ get_load_store_type (vec_info  *vinfo, stmt_vec_info stmt_info,
>       *memory_access_type = VMAT_GATHER_SCATTER;
>       if (!vect_check_gather_scatter (stmt_info, loop_vinfo, gs_info))
>    gcc_unreachable ();
> +      /* When using internal functions, we rely on pattern recognition
> +     to convert the type of the offset to the type that the target
> +     requires, with the result being a call to an internal function.
> +     If that failed for some reason (e.g. because another pattern
> +     took priority), just handle cases in which the offset already
> +     has the right type.  */
> +      else if (gs_info->ifn != IFN_LAST
> +           && !is_gimple_call (stmt_info->stmt)
> +           && !tree_nop_conversion_p (TREE_TYPE (gs_info->offset),
> +                      TREE_TYPE (gs_info->offset_vectype)))
> +    {
> +      if (dump_enabled_p ())
> +        dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> +                 "%s offset requires a conversion\n",
> +                 vls_type == VLS_LOAD ? "gather" : "scatter");
> +      return false;
> +    }
>       else if (!vect_is_simple_use (gs_info->offset, vinfo,
>                    &gs_info->offset_dt,
>                    &gs_info->offset_vectype))
> -- 
> 2.25.1
>