Hi Richard, Thanks for the review again! on 2020/7/25 上午12:21, Richard Sandiford wrote: > "Kewen.Lin" writes: > > Thanks, the rearrangement of the existing code looks good. Could you > split that and the new LOOP_VINFO_FULLY_WITH_LENGTH_P (loop_vinfo) stuff > out into separate patches? > Splitted to https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550691.html. errr... that subject should be with prefix "[PATCH] vect:". [snip ...] (Some comments in the snipped content will be done in v4) >> + here. */ >> + >> + /* For now we only operate length-based partial vectors on Power, >> + which has constant VF all the time, we need some tweakings below >> + if it doesn't hold in future. */ >> + gcc_assert (LOOP_VINFO_VECT_FACTOR (loop_vinfo).is_constant ()); > > Where do you rely on this? There didn't seem to be any obvious > to_constant uses. Since this is “only” a cost calculation, we should > be using assumed_vf. Sorry for the confusion. This was intended for the poly things like VF or nitems_per_ctrl which isn't constant during compilation time, then get people's attention on the possible runtime cost on things like scaling up for nitems_step etc. But I just realized that the computations like the multiply with another constant can operate on the coefficient, it looks there is no runtime cost then? If so, I think I thought too much before. ;-) >> - prologue_cost_vec.release (); >> - epilogue_cost_vec.release (); >> + (void) add_stmt_cost (loop_vinfo, target_cost_data, prol_cnt, scalar_stmt, >> + NULL, NULL_TREE, 0, vect_prologue); >> + (void) add_stmt_cost (loop_vinfo, target_cost_data, body_cnt, scalar_stmt, >> + NULL, NULL_TREE, 0, vect_body); > > IMO this seems to be reproducing too much of the functions that you > referred to. And the danger with that is that they could easily > get out of sync later. Good point! The original intention was to model as possible as we can, to avoid some bad decision due to some unmodeled pieces, like the case the loop body is small and some computation become nonnegligible. The unsync risks seems also applied for other codes. How about adding some "note" comments in those functions? The updated v4 is attached by addressing your comments as well as Segher's comments. BR, Kewen ----- gcc/ChangeLog: * config/rs6000/rs6000.c (rs6000_adjust_vect_cost_per_loop): New function. (rs6000_finish_cost): Call rs6000_adjust_vect_cost_per_loop. * tree-vect-loop.c (vect_estimate_min_profitable_iters): Add cost modeling for vector with length. * tree-vect-loop-manip.c (vect_set_loop_controls_directly): Update function comment. * tree-vect-stmts.c (vect_gen_len): Likewise.