RE: [PATCH][GCC][PR target/98177] aarch64: SVE: ICE in expand_direct_optab_fn

[Przemyslaw Wirkus <Przemyslaw.Wirkus@arm.com>]

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

> Przemyslaw Wirkus <Przemyslaw.Wirkus@arm.com> writes:
> > Hi,
> >
> > Recent 'support SVE comparisons for unpacked integers' patch extends
> > operands of define_expands from SVE_FULL to SVE_ALL. This causes an
> > ICE hence this PR patch.
> >
> > This patch adds this relaxation for:
> > + reduc_<optab>_scal_<mode> and
> > + arch64_pred_reduc_<optab>_<mode>
> > in order to support extra modes. Missing modes were used in REDUC_MAX.
> >
> > Original PR snippet proposed to reproduce issue was only causing ICE
> > for C++ compiler (see pr98177-1 test cases). I've slightly modified
> > original snippet in order to reproduce issue on both C and C++
> > compilers. These are pr98177-2 test cases.
> >
> > Bootstrap/regression test for AArch64 aarch64-elf and no issues.
> 
> This is a bug in the vectoriser: the vectoriser shouldn't generate
> IFN_REDUC_MAX calls that the target doesn't support.
> 
> I think the problem comes from using the wrong interface to get the index
> type for a COND_REDUCTION.  vectorizable_reduction has:
> 
>       cr_index_vector_type = build_vector_type (cr_index_scalar_type,
>                                                 nunits_out);
> 
> which means that for fixed-length SVE we get a V2SI (a 64-bit Advanced SIMD
> vector) instead of a VNx2SI (an SVE vector that stores SI elements in DI
> containers).  It should be using:
> 
>       cr_index_vector_type = get_same_sized_vectype (cr_index_scalar_type,
>                                                      vectype_out);
> 
> instead.  Same idea for the build_vector_type call in
> vect_create_epilog_for_reduction.

Hi Richard,
I've followed your guidance and indeed root cause was as you described.
Please see new patch in attachment.

Bootstrap/regression test for AArch64 aarch64-elf and no issues.

OK for master?

gcc/ChangeLog:

	PR target/98177
	* tree-vect-loop.c (vectorizable_reduction): Use get_same_sized_vectype to
	obtain index type.

gcc/testsuite/ChangeLog:

	PR target/98177
	* g++.target/aarch64/pr98177-1.C: New test.
	* g++.target/aarch64/pr98177-2.C: New test.
	* gcc.target/aarch64/pr98177-1.c: New test.
	* gcc.target/aarch64/pr98177-2.c: New test.

> Thanks,
> Richard

[-- Attachment #2: rb13905_v2.patch --]
[-- Type: application/octet-stream, Size: 2846 bytes --]

diff --git a/gcc/testsuite/g++.target/aarch64/pr98177-1.C b/gcc/testsuite/g++.target/aarch64/pr98177-1.C
new file mode 100644
index 0000000000000000000000000000000000000000..a776b7352f966f6b1d870ed51a7c94647bc46d80
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr98177-1.C
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -march=armv8.2-a+sve -msve-vector-bits=128" } */
+
+int a, b;
+short c;
+void d(long e) {
+  for (int f = 0; f < b; f += 1)
+    for (short g = 0; g < c; g += 5)
+      a = (short)e;
+}
diff --git a/gcc/testsuite/g++.target/aarch64/pr98177-2.C b/gcc/testsuite/g++.target/aarch64/pr98177-2.C
new file mode 100644
index 0000000000000000000000000000000000000000..f89777075e962f2d11a6808d1c2b1ceac226a903
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr98177-2.C
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -march=armv8.2-a+sve -msve-vector-bits=128" } */
+
+int a, b, c;
+
+void foo(long e) {
+  for (int f = 0; f < b; f ++)
+    for (int g = 0; g < c; g ++)
+      a = (short)e;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98177-1.c b/gcc/testsuite/gcc.target/aarch64/pr98177-1.c
new file mode 100644
index 0000000000000000000000000000000000000000..a776b7352f966f6b1d870ed51a7c94647bc46d80
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98177-1.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -march=armv8.2-a+sve -msve-vector-bits=128" } */
+
+int a, b;
+short c;
+void d(long e) {
+  for (int f = 0; f < b; f += 1)
+    for (short g = 0; g < c; g += 5)
+      a = (short)e;
+}
diff --git a/gcc/testsuite/gcc.target/aarch64/pr98177-2.c b/gcc/testsuite/gcc.target/aarch64/pr98177-2.c
new file mode 100644
index 0000000000000000000000000000000000000000..f89777075e962f2d11a6808d1c2b1ceac226a903
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr98177-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast -march=armv8.2-a+sve -msve-vector-bits=128" } */
+
+int a, b, c;
+
+void foo(long e) {
+  for (int f = 0; f < b; f ++)
+    for (int g = 0; g < c; g ++)
+      a = (short)e;
+}
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 52757add0e3dbae41608a1786661b326f0da9be9..98135568bf3d3c5bdd92a304c04a3524bfb917b7 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -6929,8 +6929,8 @@ vectorizable_reduction (loop_vec_info loop_vinfo,
       int scalar_precision
 	= GET_MODE_PRECISION (SCALAR_TYPE_MODE (scalar_type));
       cr_index_scalar_type = make_unsigned_type (scalar_precision);
-      cr_index_vector_type = build_vector_type (cr_index_scalar_type,
-						nunits_out);
+      cr_index_vector_type = get_same_sized_vectype (cr_index_scalar_type,
+						vectype_out);
 
       if (direct_internal_fn_supported_p (IFN_REDUC_MAX, cr_index_vector_type,
 					  OPTIMIZE_FOR_SPEED))