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.
>

Reply via email to