Hi Tamar, > On 16 Sep 2026, at 15:45, Tamar Christina <[email protected]> wrote: > > 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). >
Yes, it does. > 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. > Sure, let’s go with your series since you’ve looked at it more comprehensively. Thanks for looking at it. Kyrill > 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>
