Przemyslaw Wirkus <przemyslaw.wir...@arm.com> writes:
> > 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.
Note that for this last bit I meant:
tree vectype_unsigned = build_vector_type
(scalar_type_unsigned, TYPE_VECTOR_SUBPARTS (vectype));
which should become:
tree vectype_unsigned = get_same_sized_vectype (scalar_type_unsigned,
vectype);
This is the “transform” code that partners the “analysis” code that
you're patching. Changing one but not the other would cause problems
if (say) the Advanced SIMD REDUC_MAX patterns were disabled. We'd then
correctly pick an SVE mode like VNx4SI when doing the analysis, but generate
an unsupported V4SI REDUC_MAX in vect_create_epilog_for_reduction.
That in turn would trip the kind of expand-time assert that was
reported in the PR, just for a different case.
It's better for the modes to match up anyway: we should use a VNx4SI
reduction when operating on SVE and a V4SI reducation when operating on
Advanced SIMD. This is particularly true for big endian, where mixing
SVE and Advanced SIMD can involve a permute.
> 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;
> +}
It'd be better to put these g++.target/aarch64/sve and drop the -march
option. That way we'll test with the user's specified -march or -mcpu
if that -march/-mcpu already supports SVE.
Same idea for the other tests (including the C ones).
OK for trunk with those changes, thanks.
Richard
