Hi Kyrill, > -----Original Message----- > From: [email protected] <[email protected]> > Sent: 15 September 2026 12:12 > To: [email protected] > Cc: Tamar Christina <[email protected]>; Kyrylo Tkachov > <[email protected]> > Subject: [PATCH] aarch64: Use exact trip counts to select SVE or Advanced > SIMD > > From: Kyrylo Tkachov <[email protected]> > > aarch64_vector_costs::better_main_loop_than_p normally compares two > cycles-per-element rates. It multiplies each cost per vector iteration > by the other candidate's vectorization factor to avoid division. > > This comparison can misrank a fixed-count loop that fits in one vector > iteration of both candidates. Both candidates execute once, but the > cross multiplication gives the wider vector credit for lanes that the > loop does not use. > > For example: > > void > wadd8 (int16_t *restrict d, const uint8_t *restrict a, > const uint8_t *restrict b) > { > for (int i = 0; i < 8; ++i) > d[i] = (int16_t) (a[i] + b[i]); > } > > At -O3 -mcpu=olympus the previous comparison selects masked SVE code: > > ldr d31, [x1] > ptrue p15.b, vl8 > uunpklo z28.h, z31.b > punpklo p6.h, p15.b > punpkhi p7.h, p15.b > uunpkhi z31.h, z31.b > ldr d30, [x2] > uunpklo z29.h, z30.b > uunpkhi z30.h, z30.b > add z28.h, z29.h, z28.h > add z31.h, z30.h, z31.h > st1h z31.h, p7, [x0, #1, mul vl] > st1h z28.h, p6, [x0] > > When the exact trip count is no greater than either candidate's > vectorization factor, compare the two costs per vector iteration > directly. Do this only for loops without early exits. Keep the > existing comparison for unknown counts and for loops that need more > than one iteration of either candidate. > > This selects Advanced SIMD for the example: > > ldr d30, [x1] > ldr d31, [x2] > uaddl v30.8h, v31.8b, v30.8b > str q30, [x0] > > The tests also cover a fixed count that needs an Advanced SIMD > remainder, a symbolic bounded count, and an early-exit loop. These > cases keep their previous SVE choice. >
It was confusing to me why this was happening, and more importantly why only with the Olympus cost model. So I had spent a few hours digging into this. The difference is that the cost model says the CPU is wide enough that the unrolled instructions can all issue in the same cycle. So in terms of cycles per iteration it is correct in saying that V4QI unrolled twice has the same cost as V8QI not unrolled. What was confusing to me is that I know we already take this into account during costing, where we cap the VF to MIN (known_iters, vf) such that the code takes into account whether unrolling has any benefits and how much the vector bandwidth is used. This naturally gets it to prefer in cases of a tie the code with the higher VF (and so lower cycles per iterations). But it looks like we did this when comparing scalar vs vector but forgot it when comparing the two vector loops which is why the weighted cycles per iterations show up differently in the dump file between initial costing and cross mode costing. Does the attach patch instead fix the workload you wanted this for? (It fixes the testcases at least). And if so, how about it instead? It does point out one regression that's another genuine costing bug in that the backend plays tricks with ADB support for Adv. SIMD but we don't tell the vectorizer it's more expensive vs SVE. I have a fix for these so if you agree I can submit the series for review. Thanks, Tamar > Bootstrapped and tested on aarch64-none-linux-gnu. > Ok for trunk? > Thanks, > Kyrill > > gcc/ChangeLog: > > * config/aarch64/aarch64.cc > (aarch64_vector_costs::better_main_loop_than_p): Compare > per-iteration costs directly when an exact trip count fits in one > iteration of both candidates. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/sve_vs_advsimd_niters_1.c: New test. > * gcc.target/aarch64/sve_vs_advsimd_niters_2.c: Likewise. > > Signed-off-by: Kyrylo Tkachov <[email protected]> > --- > gcc/config/aarch64/aarch64.cc | 31 +++++++-- > .../aarch64/sve_vs_advsimd_niters_1.c | 19 ++++++ > .../aarch64/sve_vs_advsimd_niters_2.c | 68 +++++++++++++++++++ > 3 files changed, 113 insertions(+), 5 deletions(-) > create mode 100644 > gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_1.c > create mode 100644 > gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_2.c > > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc > index 5544d5f8c7a..ad4c491dfbd 100644 > --- a/gcc/config/aarch64/aarch64.cc > +++ b/gcc/config/aarch64/aarch64.cc > @@ -19586,6 +19586,11 @@ better_main_loop_than_p (const vector_costs > *uncast_other) const > } > } > > + bool niters_known_p = (LOOP_VINFO_NITERS_KNOWN_P (this_loop_vinfo) > + && !LOOP_VINFO_EARLY_BREAKS (this_loop_vinfo)); > + unsigned HOST_WIDE_INT niters > + = niters_known_p ? LOOP_VINFO_INT_NITERS (this_loop_vinfo) : 0; > + > for (unsigned int i = 0; i < m_ops.length (); ++i) > { > if (dump_enabled_p ()) > @@ -19609,11 +19614,27 @@ better_main_loop_than_p (const > vector_costs *uncast_other) const > * other->m_ops[i].vf_factor ()); > > /* If it appears that one loop could process the same amount of data > - in fewer cycles, prefer that loop over the other one. */ > - fractional_cost this_cost > - = this->m_ops[i].min_cycles_per_iter () * other_estimated_vf; > - fractional_cost other_cost > - = other->m_ops[i].min_cycles_per_iter () * this_estimated_vf; > + in fewer cycles, prefer that loop over the other one. > + > + If a fixed-count loop fits in one vector iteration of both > + candidates, compare the cost of those two iterations directly. > + Otherwise weigh each loop by the other one's vectorization factor. > + This compares the two cycles-per-element rates without division and > + avoids making assumptions about scalar remainders or early exits. */ > + fractional_cost this_cost, other_cost; > + if (niters_known_p > + && niters <= MIN (this_estimated_vf, other_estimated_vf)) > + { > + this_cost = this->m_ops[i].min_cycles_per_iter (); > + other_cost = other->m_ops[i].min_cycles_per_iter (); > + } > + else > + { > + this_cost > + = this->m_ops[i].min_cycles_per_iter () * other_estimated_vf; > + other_cost > + = other->m_ops[i].min_cycles_per_iter () * this_estimated_vf; > + } > if (dump_enabled_p ()) > { > dump_printf_loc (MSG_NOTE, vect_location, > diff --git a/gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_1.c > b/gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_1.c > new file mode 100644 > index 00000000000..00787f75573 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_1.c > @@ -0,0 +1,19 @@ > +/* Check that a loop whose trip count is smaller than the SVE vectorization > + factor is not costed as though it processed a whole SVE vector. Advanced > + SIMD covers this loop in one unpredicated iteration, so it should win. */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O3 -mcpu=olympus" } */ > + > +#include <stdint.h> > + > +void > +wadd8 (int16_t *restrict d, const uint8_t *restrict a, const uint8_t > *restrict b) > +{ > + for (int i = 0; i < 8; i++) > + d[i] = (int16_t) (a[i] + b[i]); > +} > + > +/* { dg-final { scan-assembler {\tuaddl\tv[0-9]+\.8h, v[0-9]+\.8b, v[0- > 9]+\.8b} } } */ > +/* { dg-final { scan-assembler-not {\tpunpklo\t} } } */ > +/* { dg-final { scan-assembler-not {\tst1h\t} } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_2.c > b/gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_2.c > new file mode 100644 > index 00000000000..0a8f3ba3095 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve_vs_advsimd_niters_2.c > @@ -0,0 +1,68 @@ > +/* Check cases for which the exact single-vector-iteration rule must not > + change the existing mode choice. */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O3 -mcpu=olympus" } */ > +/* { dg-final { check-function-bodies "**" "" "" } } */ > + > +#include <stdint.h> > + > +/* The Advanced SIMD candidate needs a scalar remainder. */ > +/* > +** wadd12: > +** ... > +** whilelo p[0-9]+\.h, wzr, w[0-9]+ > +** ... > +** st1h [^\n]+ > +** ... > +*/ > +void > +wadd12 (int16_t *restrict d, const uint8_t *restrict a, > + const uint8_t *restrict b) > +{ > + for (int i = 0; i < 12; ++i) > + d[i] = (int16_t) (a[i] + b[i]); > +} > + > +/* The upper bound is known, but the exact count is not. */ > +/* > +** wadd_bounded8: > +** ... > +** whilelo p[0-9]+\.h, wzr, w[0-9]+ > +** ... > +** st1h [^\n]+ > +** ... > +*/ > +void > +wadd_bounded8 (int16_t *restrict d, const uint8_t *restrict a, > + const uint8_t *restrict b, unsigned int n) > +{ > + if (n > 8) > + return; > + for (unsigned int i = 0; i < n; ++i) > + d[i] = (int16_t) (a[i] + b[i]); > +} > + > +/* Eight is the normal-exit bound, not the exact number of executions. */ > +/* > +** wadd_early8: > +** ... > +** punpkhi p[0-9]+\.h, p[0-9]+\.b > +** ... > +** st1h [^\n]+ > +** ... > +*/ > +void > +wadd_early8 (int16_t *restrict d, const uint8_t *restrict a, > + const uint8_t *restrict b) > +{ > + for (int i = 0; i < 8; ++i) > + { > + uint8_t x = a[i] + b[i]; > + if (x == 0) > + break; > + d[i] = x; > + } > +} > + > +/* { dg-final { scan-assembler-not {\tuaddl\t} } } */ > -- > 2.50.1 (Apple Git-155)
rb20905.patch
Description: rb20905.patch
