Hi Dominic, > -----Original Message----- > From: Dominic P <[email protected]> > Sent: 28 September 2026 19:30 > To: [email protected] > Cc: Richard Biener <[email protected]>; Richard Sandiford > <[email protected]>; Tamar Christina > <[email protected]>; Robin Dapp <[email protected]> > Subject: [PATCH] vect: Do not let grouped gather offsets wrap > > vect_use_grouped_gather computes the offsets { 0, X, X*2, ... } in the > unsigned element type of the punned vector, so if that is narrower than > a pointer a negative step is zero-extended and a large one wraps. > With -msve-vector-bits=256, a grouped load with a negative run-time > stride faults (ld1w ..., [x4, z31.s, uxtw]). Ask for a signed offset > for a negative constant step, and use the gather only if the returned > offset type is pointer-wide or the constant step's offsets fit it. > This also fixes vect-alias-check-10.c and -11.c with SVE-256. > > Bootstrapped and regression tested on x86_64-pc-linux-gnu; regression > tested on aarch64-linux-gnu (qemu, default and SVE-256 boards). > > Assisted-by: Claude Opus 5 (Anthropic) > > gcc/ChangeLog: > > * tree-vect-stmts.cc (vect_use_grouped_gather): Use a signed > offset type for a negative constant step. Require the offsets > not to wrap in the offset type. > > gcc/testsuite/ChangeLog: > > * gcc.dg/vect/vect-strided-neg-step-1.c: New test. > * gcc.target/aarch64/sve/strided_grouped_1.c: New test. > * gcc.target/aarch64/sve/strided_grouped_1_run.c: New test. > > Signed-off-by: Dominic P <[email protected]> > --- > OK for trunk? > > .../gcc.dg/vect/vect-strided-neg-step-1.c | 82 +++++++++++++++++++ > .../aarch64/sve/strided_grouped_1.c | 8 ++ > .../aarch64/sve/strided_grouped_1_run.c | 4 + > gcc/tree-vect-stmts.cc | 23 ++++-- > 4 files changed, 110 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/vect-strided-neg-step-1.c > create mode 100644 > gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1.c > create mode 100644 > gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1_run.c > > diff --git a/gcc/testsuite/gcc.dg/vect/vect-strided-neg-step-1.c > b/gcc/testsuite/gcc.dg/vect/vect-strided-neg-step-1.c > new file mode 100644 > index 00000000000..0d0fe976334 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/vect-strided-neg-step-1.c > @@ -0,0 +1,82 @@ > +/* Grouped strided accesses with a negative step. */ > + > +#include "tree-vect.h" > + > +typedef unsigned char uc; > +typedef unsigned short us; > + > +#define N 32 > +#define S 40 > + > +#define TEST(T, G) \ > + void __attribute__ ((noipa)) > \ > + load_##T (T *restrict o, T *restrict p, long stride, int n) > \ > + { \ > + for (int i = 0; i < n; i++) > \ > + { > \ > + for (int k = 0; k < G; k++) \ > + o[i * G + k] = p[k] * 3; \ > + p += stride; \ > + } > \ > + } \ > + void __attribute__ ((noipa)) > \ > + store_##T (T *restrict q, T *restrict a, long stride, int n) > \ > + { \ > + for (int i = 0; i < n; i++) > \ > + { > \ > + for (int k = 0; k < G; k++) \ > + q[k] = a[i * G + k] * 3; \ > + q += stride; \ > + } > \ > + } \ > + void __attribute__ ((noipa)) > \ > + store_down_##T (T *restrict q, T *restrict a, int n) > \ > + { \ > + for (int i = 0; i < n; i++) > \ > + { > \ > + for (int k = 0; k < G; k++) \ > + q[k] = a[i * G + k] * 3; \ > + q -= S; \ > + } > \ > + } \ > + void __attribute__ ((noipa)) > \ > + check_##T (void) \ > + { \ > + static T a[N * G], o[N * G], m[N * S]; \ > + T *last = m + (N - 1) * S; > \ > + _Pragma ("GCC novector") \ > + for (int i = 0; i < N * S; i++) \ > + m[i] = i * 7 + 1; > \ > + _Pragma ("GCC novector") \ > + for (int i = 0; i < N * G; i++) \ > + a[i] = i * 5 + 2; > \ > + load_##T (o, last, -S, N); > \ > + _Pragma ("GCC novector") \ > + for (int i = 0; i < N * G; i++) \ > + if (o[i] != (T) (last[i % G - i / G * S] * 3)) \ > + __builtin_abort (); \ > + store_##T (last, a, -S, N); > \ > + _Pragma ("GCC novector") \ > + for (int i = 0; i < N * G; i++) \ > + if (last[i % G - i / G * S] != (T) (a[i] * 3)) \ > + __builtin_abort (); \ > + for (int i = 0; i < N * G; i++) \ > + a[i] += 1; \ > + store_down_##T (last, a, N); \ > + _Pragma ("GCC novector") \ > + for (int i = 0; i < N * G; i++) \ > + if (last[i % G - i / G * S] != (T) (a[i] * 3)) \ > + __builtin_abort (); \ > + } > + > +TEST (uc, 4) > +TEST (us, 2) > + > +int > +main (void) > +{ > + check_vect (); > + check_uc (); > + check_us (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1.c > b/gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1.c > new file mode 100644 > index 00000000000..662f9753ecc > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1.c > @@ -0,0 +1,8 @@ > +/* { dg-do compile { target lp64 } } */ > +/* { dg-options "-O3 -fno-vect-cost-model -msve-vector-bits=256" } */ > + > +#include "../../../gcc.dg/vect/vect-strided-neg-step-1.c" > + > +/* { dg-final { scan-assembler-not {\tld1w\tz[0-9]+\.s, p[0-7]/z, \[x[0-9]+, > z[0-9]+\.s, uxtw\]} } } */ > +/* { dg-final { scan-assembler-not {\tst1w\tz[0-9]+\.s, p[0-7], \[x[0-9]+, > z[0- > 9]+\.s, uxtw\]} } } */ > +/* { dg-final { scan-assembler-times {\tst1w\tz[0-9]+\.s, p[0-7], \[x[0-9]+, > z[0-9]+\.s, sxtw\]} 2 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1_run.c > b/gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1_run.c > new file mode 100644 > index 00000000000..12c245c6ec1 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve/strided_grouped_1_run.c > @@ -0,0 +1,4 @@ > +/* { dg-do run { target { aarch64_sve256_hw && lp64 } } } */ > +/* { dg-options "-O3 -fno-vect-cost-model -msve-vector-bits=256" } */ > + > +#include "../../../gcc.dg/vect/vect-strided-neg-step-1.c" > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > index 244f319d0e4..faef8a5daad 100644 > --- a/gcc/tree-vect-stmts.cc > +++ b/gcc/tree-vect-stmts.cc > @@ -1760,22 +1760,31 @@ vect_use_grouped_gather (dr_vec_info > *dr_info, tree vectype, > > internal_fn ifn; > tree offset_vectype = *pun_vectype; > + tree step = DR_STEP (dr); > + if (TREE_CODE (step) == INTEGER_CST && tree_int_cst_sgn (step) < 0) > + offset_vectype = signed_type_for (offset_vectype); > > internal_fn strided_ifn = DR_IS_READ (dr) > ? IFN_MASK_LEN_STRIDED_LOAD : IFN_MASK_LEN_STRIDED_STORE; > > - /* Check if we have a gather/scatter with the new type. We're just trying > - with the type itself as offset for now. If not, check if we have a > - strided load/store. These have fewer constraints (for example no offset > - type must exist) so it is possible that even though a gather/scatter is > - not available we still have a strided load/store. */ > + /* Check if we have a gather/scatter with the new type as offset whose > + offsets { 0, X, X*2, ... } for DR_STEP X do not wrap. If not, check > + if we have a strided load/store. These have fewer constraints (for > + example no offset type must exist) so it is possible that even though > + a gather/scatter is not available we still have a strided > + load/store. */ > bool ok = false; > tree tmp_vectype; > int tmp_scale; > if (vect_gather_scatter_fn_p > (loop_vinfo, DR_IS_READ (dr), masked_p, *pun_vectype, > - TREE_TYPE (*pun_vectype), *pun_vectype, 1, &tmp_scale, &ifn, > - &offset_vectype, &tmp_vectype, elsvals)) > + TREE_TYPE (*pun_vectype), offset_vectype, 1, &tmp_scale, &ifn, > + &offset_vectype, &tmp_vectype, elsvals) > + && (TYPE_PRECISION (TREE_TYPE (offset_vectype)) >= POINTER_SIZE > + || (TREE_CODE (step) == INTEGER_CST > + && wi::fits_to_tree_p (wi::to_widest (step) > + * (vect_max_vf (loop_vinfo) - 1), > + TREE_TYPE (offset_vectype)))))
Taking vect_max_vf here can end up pessimisting low iteration loop counts where the latch iteration are less than VF. Also the multiply might overflow, causing fits_to_tree_p to check the wrapped values. We do a similar check as this in vect_truncate_gather_scatter_offset, where the "count" is also reduced by max_loop_iterations and wi::mul is used to check for overflows. I'd suggest refactoring that function such that the validation part can be re-used and use that here instead. Thanks, Tamar > ok = true; > else if (internal_strided_fn_supported_p (strided_ifn, *pun_vectype, > elsvals)) > -- > 2.55.0
