"Kewen.Lin via Gcc-patches" <gcc-patches@gcc.gnu.org> writes: > on 2021/5/7 下午5:43, Richard Biener wrote: >> On Fri, May 7, 2021 at 5:30 AM Kewen.Lin via Gcc-patches >> <gcc-patches@gcc.gnu.org> wrote: >>> >>> Hi, >>> >>> When I was investigating density_test heuristics, I noticed that >>> the current rs6000_density_test could be used for single scalar >>> iteration cost calculation, through the call trace: >>> vect_compute_single_scalar_iteration_cost >>> -> rs6000_finish_cost >>> -> rs6000_density_test >>> >>> It looks unexpected as its desriptive function comments and Bill >>> helped to confirm this needs to be fixed (thanks!). >>> >>> So this patch is to check the passed data, if it's the same as >>> the one in loop_vinfo, it indicates it's working on vector version >>> cost calculation, otherwise just early return. >>> >>> Bootstrapped/regtested on powerpc64le-linux-gnu P9. >>> >>> Nothing remarkable was observed with SPEC2017 Power9 full run. >>> >>> Is it ok for trunk? >> >> + /* Only care about cost of vector version, so exclude scalar >> version here. */ >> + if (LOOP_VINFO_TARGET_COST_DATA (loop_vinfo) != (void *) data) >> + return; >> >> Hmm, looks like a quite "random" test to me. What about adding a >> parameter to finish_cost () (or init_cost?) indicating the cost kind? >> > > I originally wanted to change the hook interface, but noticed that > the finish_cost in function vect_estimate_min_profitable_iters is > the only invocation with LOOP_VINFO_TARGET_COST_DATA (loop_vinfo), > it looks enough to differentiate the scalar version costing or > vector version costing for loop. Do you mean this observation/ > assumption easy to be broken sometime later? > > The attached patch to add one new parameter to indicate the > costing kind explicitly as you suggested. > > Does it look better? > > gcc/ChangeLog: > > * doc/tm.texi: Regenerated. > * target.def (init_cost): Add new parameter costing_for_scalar. > * targhooks.c (default_init_cost): Adjust for new parameter. > * targhooks.h (default_init_cost): Likewise. > * tree-vect-loop.c (_loop_vec_info::_loop_vec_info): Likewise. > (vect_compute_single_scalar_iteration_cost): Likewise. > (vect_analyze_loop_2): Likewise. > * tree-vect-slp.c (_bb_vec_info::_bb_vec_info): Likewise. > (vect_bb_vectorization_profitable_p): Likewise. > * tree-vectorizer.h (init_cost): Likewise. > * config/aarch64/aarch64.c (aarch64_init_cost): Likewise. > * config/i386/i386.c (ix86_init_cost): Likewise. > * config/rs6000/rs6000.c (rs6000_init_cost): Likewise.
Just wanted to say thanks for doing this. I hit the same problem when doing the Neoverse V1 tuning near the end of stage 4. Due to the extreme lateness of the changes, I couldn't reasonably ask for target-independent help at that time, but this patch will make things simpler for AArch64. :-) Richard
