Dominic P <[email protected]> writes: > vect_vfa_segment_size computes the segment length from (sizetype) > NITERS - 1. When a loop runs for the whole range of a narrower > counter, e.g. an unsigned char counter running 256 times, NITERS wraps > to zero, the length becomes SIZE_MAX times the step and the runtime > alias check can pass for overlapping accesses. The new test aborts at > -O3 on x86_64 and aarch64, with GCC 16 too. Pass NITERSM1 or VF - 1. > > For int counters the check becomes (sizetype) ((unsigned) n - 1): 16 > gcc.dg/vect files change at -O3 -mavx2 and 16 with SVE, +21 insns net. > NITERS - 1 when loop_niters_no_overflow holds would not, but that bound > can come from an early exit, and such a loop still miscompiles. > > Bootstrapped and regression tested on x86_64-pc-linux-gnu; regression tested > on aarch64-linux-gnu (qemu). > > Assisted-by: Claude Opus 5 (Anthropic)
What kind of assistance did it provide? > > gcc/ChangeLog: > > * tree-vect-data-refs.cc (vect_vfa_segment_size): Take the number > of iterations minus one. > (vect_prune_runtime_alias_test_list): Pass LOOP_VINFO_NITERSM1 or > VF - 1 to it. > > gcc/testsuite/ChangeLog: > > * gcc.dg/vect/vect-alias-check-21.c: New test. > > Signed-off-by: Dominic P <[email protected]> > --- > OK for trunk? > > .../gcc.dg/vect/vect-alias-check-21.c | 65 +++++++++++++++++++ > gcc/tree-vect-data-refs.cc | 11 ++-- > 2 files changed, 69 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/vect-alias-check-21.c > > diff --git a/gcc/testsuite/gcc.dg/vect/vect-alias-check-21.c > b/gcc/testsuite/gcc.dg/vect/vect-alias-check-21.c > new file mode 100644 > index 00000000000..e7442d8d3d4 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/vect-alias-check-21.c > @@ -0,0 +1,65 @@ > +/* An 8-bit counter running 256 times: the number of iterations wraps to > + zero in the counter type, which must not break the runtime alias check. > */ > + > +#include "tree-vect.h" > + > +#define N 1024 > + > +unsigned char a[N], b[N]; > + > +void __attribute__ ((noipa)) > +f (unsigned char *d, unsigned char *s, unsigned char n) > +{ > + unsigned char i = 0; > + do > + d[i] = s[2 * i] + 1; > + while (i++ != n); > +} > + > +void __attribute__ ((noipa)) > +g (unsigned char *d, unsigned char *s, unsigned char n) > +{ > + unsigned char i = 0; > + do > + d[2 * i] = s[i] + 1; > + while (i++ != n); > +} > + > +void > +init (void) > +{ > +#pragma GCC novector > + for (int i = 0; i < N; i++) > + a[i] = b[i] = i * 13 + 5; > +} > + > +void > +check (void) > +{ > +#pragma GCC novector > + for (int i = 0; i < N; i++) > + if (a[i] != b[i]) > + __builtin_abort (); > +} > + > +int > +main (void) > +{ > + check_vect (); > + > + init (); > + f (a + 7, a, 255); > +#pragma GCC novector > + for (int i = 0; i < 256; i++) > + b[7 + i] = b[2 * i] + 1; > + check (); > + > + init (); > + g (a + 7, a + 300, 255); > +#pragma GCC novector > + for (int i = 0; i < 256; i++) > + b[7 + 2 * i] = b[300 + i] + 1; > + check (); > + > + return 0; > +} > diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc > index 23235875fa2..19efee6e655 100644 > --- a/gcc/tree-vect-data-refs.cc > +++ b/gcc/tree-vect-data-refs.cc > @@ -3830,7 +3830,7 @@ vect_analyze_data_ref_accesses (vec_info *vinfo, > > Input: > DR_INFO: The data reference. > - LENGTH_FACTOR: segment length to consider. > + LENGTH_FACTOR: the number of iterations in the segment minus one. > > Return a value suitable for the dr_with_seg_len::seg_len field. > This is the "distance travelled" by the pointer from the first > @@ -3840,11 +3840,8 @@ vect_analyze_data_ref_accesses (vec_info *vinfo, > static tree > vect_vfa_segment_size (dr_vec_info *dr_info, tree length_factor) > { > - length_factor = size_binop (MINUS_EXPR, > - fold_convert (sizetype, length_factor), > - size_one_node); > return size_binop (MULT_EXPR, fold_convert (sizetype, DR_STEP > (dr_info->dr)), > - length_factor); > + fold_convert (sizetype, length_factor)); > } > > /* Return a value that, when added to abs (vect_vfa_segment_size (DR_INFO)), > @@ -4095,7 +4092,7 @@ vect_prune_runtime_alias_test_list (loop_vec_info > loop_vinfo) > const vec<vec_object_pair> &check_unequal_addrs > = LOOP_VINFO_CHECK_UNEQUAL_ADDRS (loop_vinfo); > poly_uint64 vect_factor = LOOP_VINFO_VECT_FACTOR (loop_vinfo); > - tree scalar_loop_iters = LOOP_VINFO_NITERS (loop_vinfo); > + tree scalar_loop_iters = LOOP_VINFO_NITERSM1 (loop_vinfo); Please instead remove this and change the later: length_factor = scalar_loop_iters; to: length_factor = LOOP_VINFO_NITERSM1 (loop_vinfo); Having "something_iters" store "iters - 1" could lead to confusion or mistakes later. Otherwise it looks ok to me, but I won't approve because of the AI question above. Thanks, Richard > > ddr_p ddr; > unsigned int i; > @@ -4248,7 +4245,7 @@ vect_prune_runtime_alias_test_list (loop_vec_info > loop_vinfo) > " uncounted loop\n"); > } > else > - length_factor = size_int (vect_factor); > + length_factor = size_int (vect_factor - 1); > segment_length_a = vect_vfa_segment_size (dr_info_a, length_factor); > segment_length_b = vect_vfa_segment_size (dr_info_b, length_factor); > }
