"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