On Wed, May 19, 2021 at 11:47 AM Kewen.Lin <li...@linux.ibm.com> wrote: > > Hi Richi, > > on 2021/5/19 下午4:15, Richard Biener wrote: > > On Wed, May 19, 2021 at 8:20 AM Kewen.Lin <li...@linux.ibm.com> wrote: > >> > >> Hi, > >> > >> This patch is to replace the current hardcoded weight factor 50 > >> for those statements in an inner loop relative to the loop being > >> vectorized with a specific parameter vect-inner-loop-weight-factor. > >> > >> The motivation behind this change is: if targets want to have one > >> unique function to gather some information in each add_stmt_cost > >> call, no matter that it's put before or after the cost tweaking > >> part for inner loop, it may have the need to adjust (expand or > >> shrink) the gathered data as the factor. Now the factor is > >> hardcoded, it's not easily maintained. Since it's possible that > >> targets have their own decisions on this costing like the others, > >> I used parameter instead of one unique macro here. > >> > >> Testing is ongoing, is it ok for trunk if everything goes well? > > > > Certainly an improvement. I suppose we might want to put > > the factor into vinfo->inner_loop_cost_factor. That way > > we could adjust it easily in common code in the vectorizer > > when we for example have (non-guessed) profile data. > > > > "weight_factor" is kind-of double-speak and I'm missing 'cost' ... > > so, bike-shedding to vect_inner_loop_cost_factor? > > > > Just suggestions - as said, the patch is an improvement already. > > > > Thanks for your nice suggestions! I've updated the patch accordingly > and attached it. Does it look better to you?
Minor nit: +@item vect-inner-loop-cost-factor +The factor which loop vectorizer uses to over weight those statements in +an inner loop relative to the loop being vectorized. + the default value should be documented here, not.. +-param=vect-inner-loop-cost-factor= +Common Joined UInteger Var(param_vect_inner_loop_cost_factor) Init(50) IntegerRange(1, 999999) Param Optimization +Indicates the factor which loop vectorizer uses to over weight those statements in an inner loop relative to the loop being vectorized. The default value is 50. + here (based on statistical analysis of existing cases). Also the params.opt docs should be the "brief" one - but for simplicity simply make both docs identical (apart from the default value doc). I suggest "The factor which the loop vectorizer applies to the cost of statements in an inner loop relative to the loop being vectorized." OK with that change. Richard. > btw, the testing on the previous patch passed, new round testing was > just kicked off. > > BR, > Kewen > ------ > gcc/ChangeLog: > > * doc/invoke.texi (vect-inner-loop-cost-factor): Document new > parameter. > * params.opt (vect-inner-loop-cost-factor): New. > * targhooks.c (default_add_stmt_cost): Replace hardcoded factor > 50 with LOOP_VINFO_INNER_LOOP_COST_FACTOR, include head file > tree-vectorizer.h and its required ones. > * config/aarch64/aarch64.c (aarch64_add_stmt_cost): Replace > hardcoded factor 50 with LOOP_VINFO_INNER_LOOP_COST_FACTOR. > * config/arm/arm.c (arm_add_stmt_cost): Likewise. > * config/i386/i386.c (ix86_add_stmt_cost): Likewise. > * config/rs6000/rs6000.c (rs6000_add_stmt_cost): Likewise. > * tree-vect-loop.c (vect_compute_single_scalar_iteration_cost): > Likewise. > (_loop_vec_info::_loop_vec_info): Init inner_loop_cost_factor. > * tree-vectorizer.h (_loop_vec_info): Add inner_loop_cost_factor. > (LOOP_VINFO_INNER_LOOP_COST_FACTOR): New macro. >