Hi Will, Thanks for the comments!
on 2021/5/7 下午7:43, will schmidt wrote: > On Fri, 2021-05-07 at 10:28 +0800, Kewen.Lin via Gcc-patches 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? >> >> BR, >> Kewen >> ------ >> gcc/ChangeLog: >> >> * config/rs6000/rs6000.c (rs6000_density_test): Early return if >> calculating single scalar iteration cost. > > > > Ok, so data is passed in.. > static void > rs6000_density_test (rs6000_cost_data *data) > { > ... > and loop_vinfo is calculated via... > loop_vec_info loop_vinfo = loop_vec_info_for_loop (data->loop_info); > which is > static inline loop_vec_info > loop_vec_info_for_loop (class loop *loop) > { > return (loop_vec_info) loop->aux; > } > > >> + /* Only care about cost of vector version, so exclude scalar >> version here. */ >> >> + if (LOOP_VINFO_TARGET_COST_DATA (loop_vinfo) != (void *) data) >> >> + return; > > Can the loop contain both vector and scalar parts? Comments in the > function now mention a percentage of vector instructions within the > loop. So.. this is meant to return early if there are no(?) vector > instructions? Sorry for the confusion. There are two call sites for finish_cost in loop vectorization code, one is in function vect_compute_single_scalar_iteration_cost, which is calculating the cost of one scalar iteration of the loop, since it's costing the scalar version of the loop, I called it as scalar version. The other is in function vect_estimate_min_profitable_iters, which is to finalize the cost of the vector version of the loop, I called it as vector version. Since the density test aims at the cost calculation of vector version of the loop, we have to avoid the possible penalization when we are actually calculating the cost for the scalar version of the loop. As you pointed out, the scalar version looks easily confusing with the scalar part of the vectorized loop. I updated the comments in v2 with the explicit words saying it's computing single scalar iteration cost. Does it look good to you? v2: https://gcc.gnu.org/pipermail/gcc-patches/2021-May/569896.html BR, Kewen