on 2021/5/19 下午6:01, Richard Biener wrote:
> 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."
> 

Thanks for catching this and the suggestion!   

Bootstrapped/regtested on powerpc64le-linux-gnu P9, x86_64-redhat-linux
and aarch64-linux-gnu.

Committed in r12-939 as the suggested wordings.   

BR,
Kewen

Reply via email to