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? 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? 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
