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
