"Andre Vieira (lists)" <andre.simoesdiasvie...@arm.com> writes:
> On 15/10/2021 09:48, Richard Biener wrote:
>> On Tue, 12 Oct 2021, Andre Vieira (lists) wrote:
>>
>>> Hi Richi,
>>>
>>> I think this is what you meant, I now hide all the unrolling cost 
>>> calculations
>>> in the existing target hooks for costs. I did need to adjust 'finish_cost' 
>>> to
>>> take the loop_vinfo so the target's implementations are able to set the 
>>> newly
>>> renamed 'suggested_unroll_factor'.
>>>
>>> Also added the checks for the epilogue's VF.
>>>
>>> Is this more like what you had in mind?
>> Not exactly (sorry..).  For the target hook I think we don't want to
>> pass vec_info but instead another output parameter like the existing
>> ones.
>>
>> vect_estimate_min_profitable_iters should then via
>> vect_analyze_loop_costing and vect_analyze_loop_2 report the unroll
>> suggestion to vect_analyze_loop which should then, if the suggestion
>> was > 1, instead of iterating to the next vector mode run again
>> with a fixed VF (old VF times suggested unroll factor - there's
>> min_vf in vect_analyze_loop_2 which we should adjust to
>> the old VF times two for example and maybe store the suggested
>> factor as hint) - if it succeeds the result will end up in the
>> list of considered modes (where we now may have more than one
>> entry for the same mode but a different VF), we probably want to
>> only consider more unrolling once.
>>
>> For simplicity I'd probably set min_vf = max_vf = old VF * suggested
>> factor, thus take the targets request literally.
>>
>> Richard.
>
> Hi,
>
> I now pass an output parameter to finish_costs and route it through the 
> various calls up to vect_analyze_loop.  I tried to rework 
> vect_determine_vectorization_factor and noticed that merely setting 
> min_vf and max_vf is not enough, we only use these to check whether the 
> vectorization factor is within range, well actually we only use max_vf 
> at that stage. We only seem to use 'min_vf' to make sure the 
> data_references are valid.  I am not sure my changes are the most 
> appropriate here, for instance I am pretty sure the checks for max and 
> min vf I added in vect_determine_vectorization_factor are currently 
> superfluous as they will pass by design, but thought they might be good 
> future proofing?
>
> Also I changed how we compare against max_vf, rather than relying on the 
> 'MAX_VECTORIZATION' I decided to use the estimated_poly_value with 
> POLY_VALUE_MAX, to be able to bound it further in case we have knowledge 
> of the VL. I am not entirely about the validity of this change, maybe we 
> are better off keeping the MAX_VECTORIZATION in place and not making any 
> changes to max_vf for unrolling.

Yeah, estimated_poly_value is just an estimate (even for POLY_VALUE_MAX)
rather than a guarantee.  We can't rely on it for correctness.

Richard

Reply via email to