On Fri, Jul 10, 2026 at 5:44 PM Jovan Dmitrovic <[email protected]> wrote: > > Hello Richard, > > I see your point. The spilling of invariables > still happens partly in the loop, but only the loading, > as there is no need to save the invariable because > it does not change in the loop body, if I am understanding > this correct. > > That kinda makes sense with previous implementation, > where spilling the IVs is twice as expensive than spilling > the invariables. In that case, shouldn't we divide spilling > cost of invariables by 2, and not multiply the spilling cost > of IVs by 2?
Currently target_spill_cost is computed as store + load, while target_reg_cost is just a reg-reg move. I think a lot of the complexity in the ivopts_estimate_reg_pressure function is because we cobble up some guesstimate on actual costs we add to the candidate costs. For the invariant spills it would be better if target_spill_cost and target_reload_cost would be separate (see cfgloopanal.cc:init_set_costs ()), dividing spill cost by two might be a good enough "guesstimate" for both. So yes, for IVs target_spill_cost looks correct while for invariants we'd have scaled half spill_cost plus half spill_cost. > I'm not sure if I follow on target_reg_cost, that cost should > be independent of the loop body, so therefore shouldn't > we scale it accordingly? target_reg_cost is weird IMO, I'm not sure how we have the idea that when we have 3 spare regs there are no moves required but else we need regs_needed moves. Either we always need regs_needed times reg-reg move (why? maybe assume irregular ISAs like x86?), or we should never assume anything about reg-reg moves (possibly if we have calls, due to ABI?). IMO all of the computation is way too complicated and with too little "detail" on actual instructions and target. I'd have just added spill-cost when we need more regs than available (maybe +- some magic number). Possibly scaled for invariants as you say. I'll note this is all quite fragile since this cost is eventually added to cand_cost which is _much_ more accurate as we know exactly how many uses we have and how they look like. So I'd even avoid adding the two, cand_cost and reg_pressure_cost together in this trivial way (of course just using the latter as tie-breaker isn't enough). > The calculation based on regs_needed (and therefore on n_old) > seems incorrect to me now that you mentioned it, > n_old shouldn't be a part of this cost. > > Regards, > Jovan > > On 7/9/26 10:24, Richard Biener wrote: > > CAUTION: This email originated from outside of the organization. Do not > > click links or open attachments unless you recognize the sender and know > > the content is safe. > > > > > > On Wed, Jun 24, 2026 at 4:28 PM Jovan Dmitrovic > > <[email protected]> wrote: > >> Currently, ivopt calculates register pressure cost so that the cost of > >> spilling induction variables is exactly double the cost of spilling > >> loop invariants, which is not exactly right. > >> As ivopts already has `adjust_setup_cost` function, we can use it > >> in order to make the invariant part of the cost relatively cheaper. > >> > >> gcc/ChangeLog: > >> > >> * tree-ssa-loop-ivopts.cc (ivopts_estimate_reg_pressure): > >> Change register pressure estimate in order to respect > >> the impact of spilling induction variables, compared > >> to spilling loop invariants. > >> > >> Co-authored-by: Radosav Krunić <[email protected]> > >> Signed-off-by: Djordje Todorović <[email protected]> > >> Signed-off-by: Chao-Ying Fu <[email protected]> > >> Signed-off-by: Jovan Dmitrović <[email protected]> > >> Signed-off-by: Radosav Krunić <[email protected]> > >> --- > >> gcc/tree-ssa-loop-ivopts.cc | 21 +++++++++++++-------- > >> 1 file changed, 13 insertions(+), 8 deletions(-) > >> > >> diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc > >> index 00d5031893c1..95c6dcf767ab 100644 > >> --- a/gcc/tree-ssa-loop-ivopts.cc > >> +++ b/gcc/tree-ssa-loop-ivopts.cc > >> @@ -6083,6 +6083,7 @@ ivopts_estimate_reg_pressure (struct ivopts_data > >> *data, unsigned n_invs, > >> unsigned cost; > >> unsigned n_old = data->regs_used, n_new = n_invs + n_cands; > >> unsigned regs_needed = n_new + n_old, available_regs = > >> target_avail_regs; > >> + unsigned iv_spill_cost = 0; > >> bool speed = data->speed; > >> > >> /* If there is a call in the loop body, the call-clobbered registers > >> @@ -6101,17 +6102,21 @@ ivopts_estimate_reg_pressure (struct ivopts_data > >> *data, unsigned n_invs, > >> else if (n_cands <= available_regs) > >> cost = target_reg_cost [speed] * available_regs > >> + target_spill_cost [speed] * (regs_needed - available_regs); > >> - /* If the number of candidates runs out available registers, we penalize > >> - extra candidate registers using target_spill_cost * 2. Because it is > >> - more expensive to spill induction variable than invariant. */ > >> + /* If the number of candidates runs out available registers, > >> + save the spill cost as separate variable iv_spill_cost. */ > >> else > >> - cost = target_reg_cost [speed] * available_regs > >> - + target_spill_cost [speed] * (n_cands - available_regs) * 2 > >> - + target_spill_cost [speed] * (regs_needed - n_cands); > >> + { > >> + cost = target_reg_cost [speed] * available_regs > >> + + target_spill_cost [speed] * (regs_needed - n_cands); > >> + iv_spill_cost = target_spill_cost[speed] * > >> + (n_cands - available_regs); > > As you say adjust_setup_cost is to scale cost in the loop setup which > > is not relevant for spilling of IVs, half relevant for invariants (the > > reload > > still happens inside of the loop) and of unknown relevance for the rest > > (n_old). > > > > You apply the adjustment to 'cost', but the target_reg_cost[speed] * > > available_regs > > part is not about setup cost. IMO it would make more sense to track > > a inv_spill_cost than a iv_spill_cost and only scale that. The > > n_cands <= available_regs > > case needs adjustment as well - we might assume that we spill invariants > > first, but then there might not be any. > > > >> + } > >> > >> - /* Finally, add the number of candidates, so that we prefer eliminating > >> + /* Adjust cost of invariant register pressure in order to make > >> + the spilling of induction variables the most taxing. > >> + Finally, add the number of candidates, so that we prefer eliminating > >> induction variables if possible. */ > >> - return cost + n_cands; > >> + return adjust_setup_cost(data, cost) + iv_spill_cost + n_cands; > >> } > >> > >> /* For each size of the induction variable set determine the penalty. */ > >> -- > >> 2.34.1
