Hi Jennifer,
> -----Original Message-----
> From: Jennifer Schmitz <jschm...@nvidia.com>
> Sent: Tuesday, September 24, 2024 9:23 AM
> To: gcc-patches@gcc.gnu.org
> Cc: Tamar Christina <tamar.christ...@arm.com>; Richard Sandiford
> <richard.sandif...@arm.com>; Kyrylo Tkachov <ktkac...@exchange.nvidia.com>
> Subject: Re: [RFC][PATCH] AArch64: Remove
> AARCH64_EXTRA_TUNE_USE_NEW_VECTOR_COSTS
>
>
>
> > On 28 Aug 2024, at 14:56, Kyrylo Tkachov <ktkac...@nvidia.com> wrote:
> >
> >
> >
> >> On 28 Aug 2024, at 10:27, Tamar Christina <tamar.christ...@arm.com> wrote:
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >>> -----Original Message-----
> >>> From: Kyrylo Tkachov <ktkac...@nvidia.com>
> >>> Sent: Wednesday, August 28, 2024 8:55 AM
> >>> To: Tamar Christina <tamar.christ...@arm.com>
> >>> Cc: Richard Sandiford <richard.sandif...@arm.com>; Jennifer Schmitz
> >>> <jschm...@nvidia.com>; gcc-patches@gcc.gnu.org; Kyrylo Tkachov
> >>> <ktkac...@exchange.nvidia.com>
> >>> Subject: Re: [RFC][PATCH] AArch64: Remove
> >>> AARCH64_EXTRA_TUNE_USE_NEW_VECTOR_COSTS
> >>>
> >>> Hi all,
> >>>
> >>> Thanks to Jennifer for proposing a patch and Tamar and Richard for digging
> into it.
> >>>
> >>>> On 27 Aug 2024, at 13:16, Tamar Christina <tamar.christ...@arm.com>
> wrote:
> >>>>
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Richard Sandiford <richard.sandif...@arm.com>
> >>>>> Sent: Tuesday, August 27, 2024 11:46 AM
> >>>>> To: Tamar Christina <tamar.christ...@arm.com>
> >>>>> Cc: Jennifer Schmitz <jschm...@nvidia.com>; gcc-patches@gcc.gnu.org;
> Kyrylo
> >>>>> Tkachov <ktkac...@exchange.nvidia.com>
> >>>>> Subject: Re: [RFC][PATCH] AArch64: Remove
> >>>>> AARCH64_EXTRA_TUNE_USE_NEW_VECTOR_COSTS
> >>>>>
> >>>>> Tamar Christina <tamar.christ...@arm.com> writes:
> >>>>>> Hi Jennifer,
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: Jennifer Schmitz <jschm...@nvidia.com>
> >>>>>>> Sent: Friday, August 23, 2024 1:07 PM
> >>>>>>> To: gcc-patches@gcc.gnu.org
> >>>>>>> Cc: Richard Sandiford <richard.sandif...@arm.com>; Kyrylo Tkachov
> >>>>>>> <ktkac...@exchange.nvidia.com>
> >>>>>>> Subject: [RFC][PATCH] AArch64: Remove
> >>>>>>> AARCH64_EXTRA_TUNE_USE_NEW_VECTOR_COSTS
> >>>>>>>
> >>>>>>> This patch removes the
> AARCH64_EXTRA_TUNE_USE_NEW_VECTOR_COSTS
> >>>>>>> tunable and
> >>>>>>> use_new_vector_costs entry in aarch64-tuning-flags.def and makes the
> >>>>>>> AARCH64_EXTRA_TUNE_USE_NEW_VECTOR_COSTS paths in the backend
> >>> the
> >>>>>>> default.
> >>>>>
> >>>>> Thanks for doing this. This has been on my TODO list ever since the
> >>>>> tunable was added.
> >>>>>
> >>>>> The history is that these "new" costs were originally added in stage 4
> >>>>> of GCC 11 for Neoverse V1. Since the costs were added so late, it
> >>>>> wasn't
> >>>>> appropriate to change the behaviour for any other core. All the new
> >>>>> code
> >>>>> was therefore gated on this option.
> >>>>>
> >>>>> The new costs do two main things:
> >>>>>
> >>>>> (1) use throughput-based calculations where available, including to
> >>>>> choose
> >>>>> between Advanced SIMD and SVE
> >>>>>
> >>>>> (2) try to make the latency-based costs more precise, by looking more
> closely
> >>>>> at the provided stmt_info
> >>>>>
> >>>>> Old cost models won't be affected by (1) either way, since they don't
> >>>>> provide any throughput information. But they should in principle
> >>>>> benefit
> >>>>> from (2). So...
> >>>>>
> >>>>>>> To that end, the function aarch64_use_new_vector_costs_p and its uses
> >>> were
> >>>>>>> removed. Additionally, guards were added prevent nullpointer
> dereferences
> >>> of
> >>>>>>> fields in cpu_vector_cost.
> >>>>>>>
> >>>>>>
> >>>>>> I'm not against this change, but it does mean that we now switch old
> >>>>>> Adv.
> >>> SIMD
> >>>>>> cost models as well to the new throughput based cost models. That
> >>>>>> means
> >>> that
> >>>>>> -mcpu=generic now behaves differently, and -mcpu=neoverse-n1 and I
> think
> >>>>>> some distros explicitly use this (I believe yocto for instance does).
> >>>>>
> >>>>> ...it shouldn't mean that we start using throughput-based models for
> >>>>> cortexa53 etc., since there's no associated issue info.
> >>>>
> >>>> Yes, I was using throughput based model as a name. But as you indicated
> >>>> in
> (2)
> >>>> it does change the latency calculation.
> >>>>
> >>>> My question was because of things in e.g. aarch64_adjust_stmt_cost and
> >>> friends,
> >>>> e.g. aarch64_multiply_add_p changes the cost between FMA SIMD vs scalar.
> >>>>
> >>>> So my question..
> >>>>
> >>>>>
> >>>>>> Have we validated that the old generic cost model still behaves
> >>>>>> sensibly
> with
> >>> this
> >>>>> change?
> >>>>
> >>>> is still valid I think, we *are* changing the cost for all models,
> >>>> and while they should indeed be more accurate, there could be knock on
> effects.
> >>>>
> >>>
> >>> We can run SPEC on a Grace system with -mcpu=generic to see what the
> >>> effect
> is,
> >>> but wider benchmarking would be more appropriate. Can you help with that
> >>> Tamar once we agree on the other implementation details in this patch?
> >>>
> >>
> >> Sure that's not a problem. Just ping me when you have a patch you want me
> >> to
> test :)
> >>
> >>>
> >>>> Thanks,
> >>>> Tamar
> >>>>
> >>>>>>
> >>>>>>> The patch was bootstrapped and regtested on aarch64-linux-gnu:
> >>>>>>> No problems bootstrapping, but several test files (in aarch64-sve.exp:
> >>>>>>> gather_load_extend_X.c
> >>>>>>> where X is 1 to 4, strided_load_2.c, strided_store_2.c) fail because
> >>>>>>> of
> small
> >>>>>>> differences
> >>>>>>> in codegen that make some of the scan-assembler-times tests fail.
> >>>>>>>
> >>>>>>> Kyrill suggested to add a -fvect-cost-model=unlimited flag to these
> >>>>>>> tests
> and
> >>>>> add
> >>>>>>> some
> >>>>>>
> >>>>>> I don't personally like unlimited here as unlimited means just
> >>>>>> vectorize at
> any
> >>>>>> cost. This means that costing between modes are also disabled. A lot
> >>>>>> of
> these
> >>>>>> testcases are intended to test not just that we vectorize but that we
> vectorize
> >>>>>> with efficient code.
> >>>>>>
> >>>>>> I'd prefer to use -fvect-cost-model=dynamic if that fixes the
> >>>>>> testcases.
> >>>>>
> >>>>> Yeah, I don't think -fvect-cost-model=unlimited would work for the
> >>>>> gather_load_extend_X.c tests, since we require the cost model to decide
> >>>>> when to use extending loads vs loading a full vector and unpacking.
> >>>
> >>> I had suggested using -fvect-cost-model=unlimited here because I thought
> these
> >>> tests wanted to test the capability of GCC to detect and generate the
> >>> particular
> SVE
> >>> feature (gather load extends) for all supported data types regardless of
> >>> profitability.
> >>
> >> I think the problem only specifically for the load_extend tests, because
> unlimited also
> >> disables SVE vs SVE comparisons wrt to changes to VF, so not just Adv.
> >> SIMD vs
> SVE.
> >> That is while it would generate a gather, it may choose to instead of
> >> gather from
> >> B -> D, do B -> S and then extend the results. Because this has a higher
> >> VF.
> >>
> >> Without the cross SVE mode comparisons it wouldn't know that the extensions
> would
> >> actually slow it down.
> >>
> >>> If the tests are intended to also make the right profitability decisions
> >>> for the
> generic
> >>> tuning model then I agree using -fvect-cost-model=unlimited is not
> appropriate
> >>> here, though I do think that it’d be useful to fix the backend vector
> >>> cost model
> >>> hooks to honor -fvect-cost-model=unlimited and not limit generation of
> >>> gather/scatter in that case. What it should do for the SVE vs Neon
> >>> decisions is
> an
> >>> open question.
> >>>
> >>>
> >>>>>
> >>>>> [...tries patch...]
> >>>>>
> >>>>> It seems like three main things are contributing to the difference:
> >>>>>
> >>>>> 1. we now cost 0 for a scalar prologue extension from a loaded value
> >>>>> 2. we now cost gathers & scatters based on gather_load_x32/x64_cost
> >>>>> 3. we now apply a large one-off cost for gathers (Tamar's recent change)
> >>>>>
> >>>>> (1) is expected.
> >>>>>
> >>>>> (2) partly looks like a latent bug. We're using the x32 cost for
> >>>>> VNx2QI->VNx2SI, even though those are really .B->.D loads.
> >>>>>
> >>>>> @@ -16819,7 +16811,7 @@ aarch64_detect_vector_stmt_subtype
> (vec_info
> >>>>> *vinfo, vect_cost_for_stmt kind,
> >>>>> && STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) ==
> >>>>> VMAT_GATHER_SCATTER)
> >>>>> {
> >>>>> unsigned int nunits = vect_nunits_for_cost (vectype);
> >>>>> - if (GET_MODE_UNIT_BITSIZE (TYPE_MODE (vectype)) == 64)
> >>>>> + if (known_eq (GET_MODE_NUNITS (TYPE_MODE (vectype)),
> >>>>> aarch64_sve_vg))
> >>>>> return { sve_costs->gather_load_x64_cost, nunits };
> >>>>> return { sve_costs->gather_load_x32_cost, nunits };
> >>>>> }
> >>>>>
> >>>>> fixes that.
> >>>
> >>> Would you be willing to test and push that to trunk to get it out of the
> >>> way?
> >>>
> >>>
> >>>>>
> >>>>> (3) is interesting. generic_vector_cost isn't used by default for any
> >>>>> SVE CPU, or any -march=...+sve. So the question is: should we treat it
> >>>>> as "architectural intent"/somewhat idealised? Or should we try to make
> >>>>> it produce good code for existing SVE cores, in which case it would
> >>>>> overlap quite a lot with generic_armv8_a and generic_armv9_a.
> >>>
> >>>>>
> >>>>> If the former, we could set gather_load_x32_init_cost and
> >>>>> gather_load_x64_init_cost to 0 for generic_sve_vector_cost
> >>>>> (and nothing else, so that generic_armv*_a are unaffected).
> >>>
> >>> I don’t have strong opinions on this point but if Tamar is concerned about
> >>> deviating too much from the known-good Advanced SIMD generic tuning we
> have
> >>> now then we should aim for minimal codegen changes in that?
> >>
> >> No I'm fine with Richard's suggestion. The only way you'd be able to get
> >> this
> model
> >> to fire is with -march=xxx=sve -mtune=generic, at which point I'm fine with
> assuming
> >> gathers have no initial overhead.
> >
> > As an aside, Jennifer pointed out to me that running the aarch64-sve.exp
> > tests
> ends up using “-march=armv8.2-a+sve -mtune=generic -moverride=tune=none”
> for the tests but aarch64-sve.exp does’t add that -mtune and -moverride. It
> seems
> that aarch64-sve-acle-asm.exp does add these flags (to avoid the
> CSE_SVE_VL_CONSTANTS logic IIRC), but it seems odd to use them in the
> top-level
> aarch64-sve.exp and I’m not sure by that DejaGNU mechanism they end up
> propagating there.
> Without making any more changes to the patch, I reran the testsuite:
> With Richard's patch, the gather_load_extend tests now pass.
> However, 3 tests still fail: strided_load_2.c, strided_store_2.c, and
> strided_store_5.c.
> Below you see an extract of the diff of the produced assembly for these three
> tests. Interestingly, there seem to be cases where the codegen is better with
> the
> patch (strided_load_2.c, f_int64_t_32), but also cases that are worse
> (strided_store_5.c, f_int32_t_5).
> Can you help diagnosing what the problem of the bad codegen in some cases
> could be and advise on how to proceed?
> Thank you, Jennifer
>
A lot of them look like costing issues, I'm not exactly sure what
-mtune=generic -moverride=tune=none
Resolves to. I would assume it goes back to the default tuning?
> ===== strided_load_2.c =====
> Compiled with:
> -O2 -ftree-vectorize -march=armv8.2-a+sve -mtune=generic -
> moverride=tune=none
>
> Failed test:
> gcc.target/aarch64/sve/strided_load_2.c scan-assembler-times \\tld1d\\tz[0-
> 9]+\\.d, p[0-7]/z, \\[x[0-9]+, z[0-9]+.d, lsl 3\\]\\n 15
> Instead found 9 times
>
> Example of difference in produced assembly:
> f_int64_t_32:
> cbz w3, .L92
> mov x4, 0
> uxtw x3, w3
> + cntd x5
> + whilelo p7.d, xzr, x3
> + mov z29.s, w5
> mov z31.s, w2
> - whilelo p6.d, xzr, x3
> - mov x2, x3
> - index z30.s, #0, #1
> - uqdecd x2
> - ptrue p5.b, all
> - whilelo p7.d, xzr, x2
> + index z30.d, #0, #1
> + ptrue p6.b, all
> .p2align 3,,7
> .L94:
> - ld1d z27.d, p7/z, [x0, #1, mul vl]
> - ld1d z28.d, p6/z, [x0]
> - movprfx z29, z30
> - mul z29.s, p5/m, z29.s, z31.s
> - incw x4
> - uunpklo z0.d, z29.s
> - uunpkhi z29.d, z29.s
> - ld1d z25.d, p6/z, [x1, z0.d, lsl 3]
> - ld1d z26.d, p7/z, [x1, z29.d, lsl 3]
> - add z25.d, z28.d, z25.d
> + ld1d z27.d, p7/z, [x0, x4, lsl 3]
> + movprfx z28, z30
> + mul z28.s, p6/m, z28.s, z31.s
> + ld1d z26.d, p7/z, [x1, z28.d, uxtw 3]
> add z26.d, z27.d, z26.d
> - st1d z26.d, p7, [x0, #1, mul vl]
> - whilelo p7.d, x4, x2
> - st1d z25.d, p6, [x0]
> - incw z30.s
> - incb x0, all, mul #2
> - whilelo p6.d, x4, x3
> + st1d z26.d, p7, [x0, x4, lsl 3]
> + add z30.s, z30.s, z29.s
> + incd x4
> + whilelo p7.d, x4, x3
> b.any .L94
> .L92:
> ret
>
This to me looks like unrolling. It looks like the old code decided to unroll
once and the new one doesn't.
I agree it does look better overall.
> ===== strided_store_2.c =====
> Compiled with:
> -O2 -ftree-vectorize -march=armv8.2-a+sve -mtune=generic -
> moverride=tune=none
>
> Failed test:
> gcc.target/aarch64/sve/strided_store_2.c scan-assembler-times \\tst1d\\tz[0-
> 9]+\\.d, p[0-7], \\[x[0-9]+, z[0-9]+.d, lsl 3\\]\\n 15
> Instead found 9 times
>
> Example of difference in produced assembly:
> f_float_64:
> .LFB11:
> .cfi_startproc
> cbz x3, .L67
> - lsl x2, x2, 2
> - add x3, x1, x3, lsl 2
> - fmov s31, 1.0e+0
> - .p2align 3,,7
> + sub x4, x3, #1
> + cmp x4, 2
> + bls .L73
> + lsr x9, x3, 2
> + lsl x8, x2, 2
> + fmov v31.4s, 1.0e+0
> + lsl x10, x2, 4
> + add x4, x0, x8
> + add x9, x1, x9, lsl 4
> + neg x11, x8
> + lsl x2, x2, 3
> + mov x5, x1
> + .p2align 3,,7
> +.L70:
> + ldr q30, [x5], 16
> + add x7, x4, x8
> + add x6, x4, x2
> + fadd v30.4s, v30.4s, v31.4s
> + str s30, [x4, x11]
> + st1 {v30.s}[1], [x4]
> + add x4, x4, x10
> + st1 {v30.s}[2], [x7]
> + st1 {v30.s}[3], [x6]
> + cmp x5, x9
> + bne .L70
> + and x4, x3, -4
> + tst x3, 3
> + beq .L67
> .L69:
> - ldr s30, [x1], 4
> - fadd s30, s30, s31
> - str s30, [x0]
> - add x0, x0, x2
> - cmp x1, x3
> - bne .L69
> + madd x0, x8, x4, x0
> + fmov s29, 1.0e+0
> +.L72:
> + ldr s28, [x1, x4, lsl 2]
> + add x4, x4, 1
> + fadd s28, s28, s29
> + str s28, [x0]
> + add x0, x0, x8
> + cmp x3, x4
> + bhi .L72
> .L67:
> ret
> +.L73:
> + lsl x8, x2, 2
> + mov x4, 0
> + b .L69
>
This one feels like the vectorizer emulated a scatter store.
Did you rebase by chance? I think our cost model most likely
does not model the store cots of a single lane. I'd look at costing of
the loop from the vectorizer.
>
> f_int64_t_32:
> .LFB14:
> .cfi_startproc
> - cbz w3, .L84
> - addvl x5, x1, #1
> + cbz w3, .L92
> mov x4, 0
> uxtw x3, w3
> + cntd x5
> + whilelo p7.d, xzr, x3
> + mov z29.s, w5
> mov z31.s, w2
> - whilelo p6.d, xzr, x3
> - mov x2, x3
> - index z30.s, #0, #1
> - uqdecd x2
> - ptrue p5.b, all
> - whilelo p7.d, xzr, x2
> - .p2align 3,,7
> -.L86:
> - ld1d z28.d, p6/z, [x1, x4, lsl 3]
> - ld1d z27.d, p7/z, [x5, x4, lsl 3]
> - movprfx z29, z30
> - mul z29.s, p5/m, z29.s, z31.s
> - add z28.d, z28.d, #1
> - uunpklo z26.d, z29.s
> - st1d z28.d, p6, [x0, z26.d, lsl 3]
> - incw x4
> - uunpkhi z29.d, z29.s
> + index z30.d, #0, #1
> + ptrue p6.b, all
> + .p2align 3,,7
> +.L94:
> + ld1d z27.d, p7/z, [x1, x4, lsl 3]
> + movprfx z28, z30
> + mul z28.s, p6/m, z28.s, z31.s
> add z27.d, z27.d, #1
> - st1d z27.d, p7, [x0, z29.d, lsl 3]
> - whilelo p7.d, x4, x2
> - incw z30.s
> - whilelo p6.d, x4, x3
> - b.any .L86
> -.L84:
> + st1d z27.d, p7, [x0, z28.d, uxtw 3]
> + incd x4
> + add z30.s, z30.s, z29.s
> + whilelo p7.d, x4, x3
> + b.any .L94
> +.L92:
> ret
>
Same as before it looks like we stopped unrolling on a long dependency chain.
So I think the code is better,
> ==== strided_store_5.c =====
> Compiled with:
> -O2 -ftree-vectorize -march=armv8.2-a+sve -mtune=generic -
> moverride=tune=none -msve-vector-bits=256
>
> Failed tests:
> gcc.target/aarch64/sve/strided_store_5. c scan-assembler-times \\tst1d\\tz[0-
> 9]+\\.d, p[0-7], \\[x[0-9]+, z[0-9]+.d\\]\\n 15
> Instead found 0 times
> gcc.target/aarch64/sve/strided_store_5. c scan-assembler-times \\tst1w\\tz[0-
> 9]+\\.s, p[0-7], \\[x[0-9]+, z[0-9]+.s, sxtw\\]\\n 3
> Instead found 0 times
> gcc.target/aarch64/sve/strided_store_5. c scan-assembler-times \\tst1w\\tz[0-
> 9]+\\.s, p[0-7], \\[x[0-9]+, z[0-9]+.s, uxtw\\]\\n 12
> Instead found 0 times
>
> Example of difference in produced assembly:
> f_int32_t_5:
> cmp x2, 0
> ble .L1
> - mov x3, 0
> - mov w4, 20
> - whilelo p7.s, xzr, x2
> - index z31.s, #0, w4
> - .p2align 3,,7
> + sub x3, x2, #1
> + cmp x3, 6
> + bls .L7
> + lsr x8, x2, 3
> + mov x4, x1
> + mov x3, x0
> + ptrue p6.b, vl32
> + add x8, x1, x8, lsl 5
> + pfalse p7.b
> + .p2align 3,,7
> +.L4:
> + ld1w z31.s, p6/z, [x4]
> + add z31.s, z31.s, #1
> + umov w7, v31.s[1]
> + umov w6, v31.s[2]
> + umov w5, v31.s[3]
> + dup z1.s, z31.s[4]
> + dup z0.s, z31.s[5]
> + dup z30.s, z31.s[6]
> + lastb s29, p7, z31.s
> + add x4, x4, 32
> + str s31, [x3]
> + add x3, x3, 160
> + str w7, [x3, -140]
> + str w6, [x3, -120]
> + str w5, [x3, -100]
> + str s1, [x3, -80]
> + str s0, [x3, -60]
> + str s30, [x3, -40]
> + str s29, [x3, -20]
> + cmp x8, x4
> + bne .L4
> + and x3, x2, -8
> + cmp x2, x3
> + beq .L1
> .L3:
> - ld1w z30.s, p7/z, [x1, x3, lsl 2]
> - add z30.s, z30.s, #1
> - st1w z30.s, p7, [x0, z31.s, uxtw]
> - add x3, x3, 8
> - add x0, x0, 160
> - whilelo p7.s, x3, x2
> - b.any .L3
> + lsl x4, x3, 2
> + sub x2, x2, x3
> + add x3, x4, x3
> + whilelo p7.d, xzr, x2
> + add x1, x1, x4
> + mov x4, 20
> + ld1w z2.d, p7/z, [x1]
> + add x3, x0, x3, lsl 2
> + index z28.d, #0, x4
> + add z2.s, z2.s, #1
> + st1w z2.d, p7, [x3, z28.d]
> + mov x0, 4
> + whilelo p7.d, x0, x2
> + b.any .L10
> .L1:
> ret
> + .p2align 2,,3
> +.L10:
> + ld1w z29.d, p7/z, [x1, #1, mul vl]
> + add x3, x3, 80
> + add z29.s, z29.s, #1
> + st1w z29.d, p7, [x3, z28.d]
> + ret
> +.L7:
> + mov x3, 0
> + b .L3
>
This one kinda feels like we vectorized a constructor we don't really support
codegen for.
You can check if the code at dce6 makes sense and if the scalarization is done
by veclower21.
Thanks,
Tamar
> >
> > Thanks,
> > Kyrill
> >
> >>
> >> Cheers,
> >> Tamar
> >>
> >>> Thanks again,
> >>> Kyrill
> >>>
> >>>
> >>>>>
> >>>>> On the patch:
> >>>>>
> >>>>>> @@ -16733,7 +16723,8 @@ aarch64_in_loop_reduction_latency
> (vec_info
> >>>>> *vinfo, stmt_vec_info stmt_info,
> >>>>>> {
> >>>>>> const cpu_vector_cost *vec_costs = aarch64_tune_params.vec_costs;
> >>>>>> const sve_vec_cost *sve_costs = nullptr;
> >>>>>> - if (vec_flags & VEC_ANY_SVE)
> >>>>>> + if (vec_costs->sve
> >>>>>> + && vec_flags & VEC_ANY_SVE)
> >>>>>> sve_costs = aarch64_tune_params.vec_costs->sve;
> >>>>>
> >>>>> This doesn't seem necessary. I agree we might as well reuse the
> >>>>> vec_costs local variable in the "if" body though.
> >>>>>
> >>>>>> [...]
> >>>>>> @@ -16794,9 +16785,10 @@ aarch64_detect_vector_stmt_subtype
> >>> (vec_info
> >>>>> *vinfo, vect_cost_for_stmt kind,
> >>>>>> fractional_cost stmt_cost)
> >>>>>> {
> >>>>>> const simd_vec_cost *simd_costs = aarch64_simd_vec_costs (vectype);
> >>>>>> + const cpu_vector_cost *vec_costs = aarch64_tune_params.vec_costs;
> >>>>>> const sve_vec_cost *sve_costs = nullptr;
> >>>>>> - if (aarch64_sve_mode_p (TYPE_MODE (vectype)))
> >>>>>> - sve_costs = aarch64_tune_params.vec_costs->sve;
> >>>>>> + if (aarch64_sve_mode_p (TYPE_MODE (vectype)) && vec_costs->sve)
> >>>>>> + sve_costs = vec_costs->sve;
> >>>>>
> >>>>> Similarly here, this doesn't seem necessary.
> >>>>>
> >>>>>> [...]
> >>>>>> @@ -17301,11 +17293,13 @@ aarch64_vector_costs::add_stmt_cost (int
> >>>>> count, vect_cost_for_stmt kind,
> >>>>>> where, stmt_cost);
> >>>>>
> >>>>>> /* Check if we've seen an SVE gather/scatter operation and which
> >>>>>> size. */
> >>>>>> + const cpu_vector_cost *vec_costs =
> >>>>>> aarch64_tune_params.vec_costs;
> >>>>>> if (kind == scalar_load
> >>>>>> && aarch64_sve_mode_p (TYPE_MODE (vectype))
> >>>>>> - && STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) ==
> >>>>> VMAT_GATHER_SCATTER)
> >>>>>> + && STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) ==
> >>>>> VMAT_GATHER_SCATTER
> >>>>>> + && vec_costs->sve)
> >>>>>> {
> >>>>>> - const sve_vec_cost *sve_costs =
> >>>>>> aarch64_tune_params.vec_costs->sve;
> >>>>>> + const sve_vec_cost *sve_costs = vec_costs->sve;
> >>>>>> if (sve_costs)
> >>>>>> {
> >>>>>> if (GET_MODE_UNIT_BITSIZE (TYPE_MODE (vectype)) == 64)
> >>>>>
> >>>>> Here too.
> >>>>>
> >>>>> Thanks,
> >>>>> Richard
>
