Applied (with a shortened comment) to master as 7e35df8c412. Thanks! -- Philipp.
On Wed, 8 Jul 2026 at 08:19, Kyrylo Tkachov <[email protected]> wrote: > > Ok but a comment nit inline... > > > On 7 Jul 2026, at 23:37, Andrea Pinski <[email protected]> > > wrote: > > > > On Tue, Jul 7, 2026 at 2:25 PM Philipp Tomsich <[email protected]> > > wrote: > >> > >> The ampere1, ampere1a and ampere1b tunes leave the vector issue_info > >> in cpu_vector_cost as NULL, so determine_suggested_unroll_factor bails > >> out early and reduction loops are never multi-accumulator-unrolled on > >> these cores. Fill it in from the ampere1 optimization guide (L1D two > >> 128-bit loads plus one store per cycle, four general ops per cycle). > >> > >> gcc/ChangeLog: > >> > >> * config/aarch64/tuning_models/ampere1.h (ampere1_scalar_issue_info) > >> (ampere1_advsimd_issue_info, ampere1_vec_issue_info): New. > >> (ampere1_vector_cost): Use ampere1_vec_issue_info for issue_info. > >> * config/aarch64/tuning_models/ampere1b.h > >> (ampere1b_scalar_issue_info) > >> (ampere1b_advsimd_issue_info, ampere1b_vec_issue_info): New. > >> (ampere1b_vector_cost): Use ampere1b_vec_issue_info for issue_info. > >> > >> gcc/testsuite/ChangeLog: > >> > >> * gcc.target/aarch64/ampere1-reduction-unroll-1.c: New test. > > > > LGTM. > > > >> > >> Signed-off-by: Philipp Tomsich <[email protected]> > >> --- > >> > >> gcc/config/aarch64/tuning_models/ampere1.h | 32 ++++++++++++++++++- > >> gcc/config/aarch64/tuning_models/ampere1b.h | 32 ++++++++++++++++++- > >> .../aarch64/ampere1-reduction-unroll-1.c | 20 ++++++++++++ > >> 3 files changed, 82 insertions(+), 2 deletions(-) > >> create mode 100644 > >> gcc/testsuite/gcc.target/aarch64/ampere1-reduction-unroll-1.c > >> > >> diff --git a/gcc/config/aarch64/tuning_models/ampere1.h > >> b/gcc/config/aarch64/tuning_models/ampere1.h > >> index 53d45761b42f..e2a42f585dd9 100644 > >> --- a/gcc/config/aarch64/tuning_models/ampere1.h > >> +++ b/gcc/config/aarch64/tuning_models/ampere1.h > >> @@ -46,6 +46,36 @@ static const advsimd_vec_cost > >> ampere1_advsimd_vector_cost = > >> 1 /* store_cost */ > >> }; > >> > >> +static const aarch64_scalar_vec_issue_info ampere1_scalar_issue_info = > >> +{ > >> + 3, /* loads_stores_per_cycle */ > >> + 1, /* stores_per_cycle */ > >> + 4, /* general_ops_per_cycle */ > >> + 0, /* fp_simd_load_general_ops */ > >> + 1 /* fp_simd_store_general_ops */ > >> +}; > >> + > >> +static const aarch64_advsimd_vec_issue_info ampere1_advsimd_issue_info = > >> +{ > >> + { > >> + 2, /* loads_stores_per_cycle */ > >> + 2, /* stores_per_cycle */ > >> + 4, /* general_ops_per_cycle */ > >> + 0, /* fp_simd_load_general_ops */ > >> + 1 /* fp_simd_store_general_ops */ > >> + }, > >> + 2, /* ld2_st2_general_ops */ > >> + 2, /* ld3_st3_general_ops */ > >> + 3 /* ld4_st4_general_ops */ > >> +}; > >> + > >> +static const aarch64_vec_issue_info ampere1_vec_issue_info = > >> +{ > >> + &ere1_scalar_issue_info, > >> + &ere1_advsimd_issue_info, > >> + nullptr /* sve */ > >> +}; > >> + > >> /* Ampere-1 costs for vector insn classes. */ > >> static const struct cpu_vector_cost ampere1_vector_cost = > >> { > >> @@ -57,7 +87,7 @@ static const struct cpu_vector_cost ampere1_vector_cost = > >> 1, /* cond_not_taken_branch_cost */ > >> &ere1_advsimd_vector_cost, /* advsimd */ > >> nullptr, /* sve */ > >> - nullptr /* issue_info */ > >> + &ere1_vec_issue_info /* issue_info */ > >> }; > >> > >> static const cpu_prefetch_tune ampere1_prefetch_tune = > >> diff --git a/gcc/config/aarch64/tuning_models/ampere1b.h > >> b/gcc/config/aarch64/tuning_models/ampere1b.h > >> index b84713b53668..8ba434d27bbf 100644 > >> --- a/gcc/config/aarch64/tuning_models/ampere1b.h > >> +++ b/gcc/config/aarch64/tuning_models/ampere1b.h > >> @@ -57,6 +57,36 @@ static const advsimd_vec_cost > >> ampere1b_advsimd_vector_cost = > >> 1 /* store_cost */ > >> }; > >> > >> +static const aarch64_scalar_vec_issue_info ampere1b_scalar_issue_info = > >> +{ > >> + 3, /* loads_stores_per_cycle */ > >> + 1, /* stores_per_cycle */ > >> + 4, /* general_ops_per_cycle */ > >> + 0, /* fp_simd_load_general_ops */ > >> + 1 /* fp_simd_store_general_ops */ > >> +}; > >> + > >> +static const aarch64_advsimd_vec_issue_info ampere1b_advsimd_issue_info = > >> +{ > >> + { > >> + 2, /* loads_stores_per_cycle */ > >> + 2, /* stores_per_cycle */ > >> + 4, /* general_ops_per_cycle */ > >> + 0, /* fp_simd_load_general_ops */ > >> + 1 /* fp_simd_store_general_ops */ > >> + }, > >> + 2, /* ld2_st2_general_ops */ > >> + 2, /* ld3_st3_general_ops */ > >> + 3 /* ld4_st4_general_ops */ > >> +}; > >> + > >> +static const aarch64_vec_issue_info ampere1b_vec_issue_info = > >> +{ > >> + &ere1b_scalar_issue_info, > >> + &ere1b_advsimd_issue_info, > >> + nullptr /* sve */ > >> +}; > >> + > >> /* Ampere-1B costs for vector insn classes. */ > >> static const struct cpu_vector_cost ampere1b_vector_cost = > >> { > >> @@ -68,7 +98,7 @@ static const struct cpu_vector_cost ampere1b_vector_cost > >> = > >> 1, /* cond_not_taken_branch_cost */ > >> &ere1b_advsimd_vector_cost, /* advsimd */ > >> nullptr, /* sve */ > >> - nullptr /* issue_info */ > >> + &ere1b_vec_issue_info /* issue_info */ > >> }; > >> > >> static const struct tune_params ampere1b_tunings = > >> diff --git a/gcc/testsuite/gcc.target/aarch64/ampere1-reduction-unroll-1.c > >> b/gcc/testsuite/gcc.target/aarch64/ampere1-reduction-unroll-1.c > >> new file mode 100644 > >> index 000000000000..61ab57304894 > >> --- /dev/null > >> +++ b/gcc/testsuite/gcc.target/aarch64/ampere1-reduction-unroll-1.c > >> @@ -0,0 +1,20 @@ > >> +/* { dg-do compile } */ > >> +/* { dg-options "-Ofast -mcpu=ampere1" } */ > >> + > >> +/* The ampere1 family used a nullptr for the vector issue-info struct, so > >> + aarch64_vector_costs::determine_suggested_unroll_factor returned 1 at > >> the > >> + `if (!vec_issue) return 1;' guard and reduction loops were never > >> + multi-accumulator-unrolled. With issue_info populated, a unit-stride > >> FP > >> + dot product widens to a suggested unroll factor of 4 and emits four > >> + independent `fmla v.4s' accumulator chains in the inner loop. */ > > I don’t find it useful for comments to talk about what the code used to be, > they should describe what the code does now. I suggest you greatly simplify > this comment and just say that for ampere1 you expect this loop to have > unroll factor 4, or something to that effect. > Thanks, > Kyrill > > > >> + > >> +float > >> +dot (const float *a, const float *b, int n) > >> +{ > >> + float s = 0.0f; > >> + for (int i = 0; i < n; i++) > >> + s += a[i] * b[i]; > >> + return s; > >> +} > >> + > >> +/* { dg-final { scan-assembler-times {\tfmla\tv[0-9]+\.4s} 4 } } */ > >> -- > >> 2.34.1 > >> > >> base-commit: 3b443e764144526748d2b51d017d745f1956377e > >
