On 15/09/2026 12:12, [email protected] wrote:
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. Bootstrapped and tested on aarch64-none-linux-gnu. Ok for trunk? Thanks, Kyrill
Interesting change, makes sense to me fwiw. Out of curiosity did you do any performance benchmarking for this change? Thanks, Alfie
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} } } */
