Dominic P <[email protected]> writes: > Since r14-2335 (PR110474) the epilogue of an unrolled main loop needs a > VF below the main loop's VF before unrolling, so when the main loop > already has the lowest VF the leftover iterations stay scalar. This > makes that rule a preference in vect_analyze_loop and otherwise accepts > any VF below the main loop's; without the preference PR110474 comes back > (a double dot product on neoverse-n2 executes twice the instructions). > > With -mcpu=neoverse-v2, -v1, -n2 and cortex-a710 an 8-byte SAD over > rows (VF 1, unrolled four times) goes from 189 to 82 instructions and > executes 27% fewer over 0 to 40 rows. In gcc.dg/vect and > gcc.target/aarch64 at -O3 the only code changes are added epilogues > (neoverse-v2: 93 files, x86_64: 4). > > Bootstrapped and regtested on x86_64-pc-linux-gnu; regtested on > aarch64-linux-gnu (cross, qemu; vect.exp and execute.exp also with > -march=armv8-a+sve -msve-vector-bits=256).
I suppose both this patch and the one for PR110474 are heuristics that try to cope with the fact that we only support a single epilogue loop. Lifting that restriction would be good :) Perhaps I'm wrong, but it seems that both this patch and PR110474 are motivated by trying to use the smallest possible VF for the epilogue. Are there any cases where: - the iteration count is unknown and - partial vectors are not supported where we'd want to use a larger epilogue VF instead? Using a larger VF would always leave some cases to scalar code that could have been handled by vector code. Those cases would in turn provide a motivation for another tweak to the heuristic. That is, it sounds from the description that both pr110474.c and the new tests would pass if we simply picked the lowest VF in the absence of LOOP_VINFO_USING_PARTIAL_VECTORS_P. Is that right? Thanks, Richard > > Assisted-by: Claude Opus 5 (Anthropic) > > gcc/ChangeLog: > > * tree-vect-loop.cc (vect_analyze_loop_2): Only require a > non-partial epilogue to have a lower VF than the main loop. > (vect_analyze_loop): Prefer an epilogue whose VF is lower than > the main loop's VF before unrolling, or that uses partial vectors, > and keep looking for one after finding another epilogue. > > gcc/testsuite/ChangeLog: > > * gcc.dg/vect/vect-unroll-epilogue-1.c: New test. > * gcc.target/aarch64/vect-unroll-epilogue-1.c: New test. > > Signed-off-by: Dominic P <[email protected]> > --- > OK for trunk? > > .../gcc.dg/vect/vect-unroll-epilogue-1.c | 75 +++++++++++++++++++ > .../aarch64/vect-unroll-epilogue-1.c | 19 +++++ > gcc/tree-vect-loop.cc | 40 ++++++---- > 3 files changed, 119 insertions(+), 15 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vect/vect-unroll-epilogue-1.c > create mode 100644 gcc/testsuite/gcc.target/aarch64/vect-unroll-epilogue-1.c > > diff --git a/gcc/testsuite/gcc.dg/vect/vect-unroll-epilogue-1.c > b/gcc/testsuite/gcc.dg/vect/vect-unroll-epilogue-1.c > new file mode 100644 > index 00000000000..dfe901d5c4a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/vect/vect-unroll-epilogue-1.c > @@ -0,0 +1,75 @@ > +/* { dg-additional-options "-O3" } */ > + > +#include "tree-vect.h" > + > +#define N 40 > + > +__attribute__((noipa)) int > +sad16 (unsigned char *p, unsigned char *q, int h) > +{ > + int s = 0; > +#pragma GCC unroll 4 > + for (int i = 0; i < h; i++, p += 16, q += 16) > + for (int k = 0; k < 16; k++) > + s += __builtin_abs (p[k] - q[k]); > + return s; > +} > + > +__attribute__((noipa)) void > +st16 (unsigned char *restrict o, unsigned char *p, int h) > +{ > +#pragma GCC unroll 4 > + for (int i = 0; i < h; i++, p += 16, o += 16) > + for (int k = 0; k < 16; k++) > + o[k] = p[k] + 3; > +} > + > +__attribute__((noipa)) void > +add (double *restrict a, double *b, double *c, int n) > +{ > +#pragma GCC unroll 8 > + for (int i = 0; i < n; i++) > + a[i] = b[i] + c[i]; > +} > + > +unsigned char p[16 * N], q[16 * N], o[16 * N + 1]; > +double a[N + 1], b[N], c[N]; > + > +int > +main (void) > +{ > + check_vect (); > + for (int i = 0; i < 16 * N; i++) > + { > + p[i] = i * 7; > + q[i] = i * 13; > + } > + for (int i = 0; i < N; i++) > + { > + b[i] = i; > + c[i] = 2 * i; > + } > + for (int n = 0; n <= N; n++) > + { > + int s = 0; > + for (int i = 0; i < 16 * n; i++) > + s += __builtin_abs (p[i] - q[i]); > + if (sad16 (p, q, n) != s) > + abort (); > + o[16 * n] = 1; > + st16 (o, p, n); > + for (int i = 0; i < 16 * n; i++) > + if (o[i] != (unsigned char) (p[i] + 3)) > + abort (); > + if (o[16 * n] != 1) > + abort (); > + a[n] = -1; > + add (a, b, c, n); > + for (int i = 0; i < n; i++) > + if (a[i] != 3 * i) > + abort (); > + if (a[n] != -1) > + abort (); > + } > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/aarch64/vect-unroll-epilogue-1.c > b/gcc/testsuite/gcc.target/aarch64/vect-unroll-epilogue-1.c > new file mode 100644 > index 00000000000..e8e62655089 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/vect-unroll-epilogue-1.c > @@ -0,0 +1,19 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O3 -fdump-tree-vect-details" } */ > + > +/* The main loop has VF 1 and is unrolled; its epilogue can only have VF 1. > */ > + > +#pragma GCC target "+nosve" > + > +int > +f (unsigned char *p, unsigned char *q, int h) > +{ > + int s = 0; > +#pragma GCC unroll 4 > + for (int i = 0; i < h; i++, p += 16, q += 16) > + for (int k = 0; k < 16; k++) > + s += __builtin_abs (p[k] - q[k]); > + return s; > +} > + > +/* { dg-final { scan-tree-dump "Choosing epilogue vector mode V16QI" "vect" > } } */ > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc > index 9b2afad6034..c3fb64fa362 100644 > --- a/gcc/tree-vect-loop.cc > +++ b/gcc/tree-vect-loop.cc > @@ -2518,16 +2518,12 @@ start_over: > to be able to handle fewer than VF scalars, or needs to have a lower VF > than the main loop. */ > if (LOOP_VINFO_EPILOGUE_P (loop_vinfo) > - && !LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo)) > - { > - poly_uint64 unscaled_vf > - = exact_div (LOOP_VINFO_VECT_FACTOR (orig_loop_vinfo), > - orig_loop_vinfo->suggested_unroll_factor); > - if (maybe_ge (LOOP_VINFO_VECT_FACTOR (loop_vinfo), unscaled_vf)) > - return opt_result::failure_at (vect_location, > - "Vectorization factor too high for" > - " epilogue loop.\n"); > - } > + && !LOOP_VINFO_USING_PARTIAL_VECTORS_P (loop_vinfo) > + && maybe_ge (LOOP_VINFO_VECT_FACTOR (loop_vinfo), > + LOOP_VINFO_VECT_FACTOR (orig_loop_vinfo))) > + return opt_result::failure_at (vect_location, > + "Vectorization factor too high for" > + " epilogue loop.\n"); > > /* If the epilogue needs peeling for gaps but the main loop doesn't give > up on the epilogue. */ > @@ -3110,6 +3106,17 @@ vect_analyze_loop (class loop *loop, gimple > *loop_vectorized_call, > loop_vec_info orig_loop_vinfo = first_loop_vinfo; > do > { > + /* Prefer an epilogue with a VF below that of the main loop before > + unrolling. */ > + poly_uint64 unscaled_vf > + = exact_div (LOOP_VINFO_VECT_FACTOR (orig_loop_vinfo), > + orig_loop_vinfo->suggested_unroll_factor); > + auto low_vf_p = [&] (loop_vec_info vinfo) > + { > + return (LOOP_VINFO_USING_PARTIAL_VECTORS_P (vinfo) > + || known_lt (LOOP_VINFO_VECT_FACTOR (vinfo), unscaled_vf)); > + }; > + > /* Let the user override what the target suggests. */ > if (OPTION_SET_P (param_vect_partial_vector_usage)) > masked_p = -1; > @@ -3161,10 +3168,12 @@ vect_analyze_loop (class loop *loop, gimple > *loop_vectorized_call, > > if (loop_vinfo) > { > - if (pick_lowest_cost_p > - && orig_loop_vinfo->epilogue_vinfo > - && vect_joust_loop_vinfos (loop_vinfo, > - orig_loop_vinfo->epilogue_vinfo)) > + loop_vec_info old = orig_loop_vinfo->epilogue_vinfo; > + if (old > + && (low_vf_p (loop_vinfo) != low_vf_p (old) > + ? low_vf_p (loop_vinfo) > + : (pick_lowest_cost_p > + && vect_joust_loop_vinfos (loop_vinfo, old)))) > { > gcc_assert (vect_epilogues); > delete orig_loop_vinfo->epilogue_vinfo; > @@ -3181,7 +3190,8 @@ vect_analyze_loop (class loop *loop, gimple > *loop_vectorized_call, > /* For now only allow one epilogue loop, but allow > pick_lowest_cost_p to replace it, so commit to the > first epilogue if we have no reason to try alternatives. */ > - if (!pick_lowest_cost_p) > + if (!pick_lowest_cost_p > + && low_vf_p (orig_loop_vinfo->epilogue_vinfo)) > break; > }
