Hello Richard, thank you for your input. I agree that I should've been fully transparent about all the changes made.
I have to point out that we had discussion on this topic back in 2023 [1]. the first return value was originally your idea. This also aligns with estimate_reg_pressure_cost in loop analysis, which returns 0 in the same circumstance. Again, this change should've been mentioned by the commit message. As for the n_cand bias, and correct me if I'm wrong, I was under the impression that we already have a bias in selecting a set of fewer candidates in place. If that is the case, wouldn't adding this n_cands to overall cost negatively impact our candidate selection heuristic, making it very unlikely for larger candidate sets ever to be chosen? Best, Jovan [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/618577.html On 6/16/26 15:56, Richard Biener wrote: > On Wed, May 20, 2026 at 2:38 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. >> >> Tested with CoreMark PRO on RISC-V P8700 CPU (~1.2% improvement), >> and on x86-64 Intel Core i7-12700H CPU (~0.4% improvement). >> >> 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 | 23 +++++++++++++---------- >> 1 file changed, 13 insertions(+), 10 deletions(-) >> >> diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc >> index 776487d0f8b1..19ba620fb070 100644 >> --- a/gcc/tree-ssa-loop-ivopts.cc >> +++ b/gcc/tree-ssa-loop-ivopts.cc >> @@ -6085,6 +6085,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 >> @@ -6094,7 +6095,7 @@ ivopts_estimate_reg_pressure (struct ivopts_data >> *data, unsigned n_invs, >> >> /* If we have enough registers. */ >> if (regs_needed + target_res_regs < available_regs) >> - cost = n_new; >> + cost = 0; >> /* If close to running out of registers, try to preserve them. */ >> else if (regs_needed <= available_regs) >> cost = target_reg_cost [speed] * regs_needed; >> @@ -6103,17 +6104,19 @@ 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); >> + } >> >> - /* Finally, add the number of candidates, so that we prefer eliminating >> - induction variables if possible. */ >> - return cost + n_cands; >> + /* Adjust cost of invariant register pressure in order to make >> + the spilling of induction variables the most taxing. */ >> + return adjust_setup_cost(data, cost) + iv_spill_cost; > You are changing many details here that you do not specifically > mention, one is the return value when regs_needed + target_res_regs < > available_regs, > another is no longer adding the n_cands bias. > > Please avoid that. I think that eventually scaling the invariant > spill cost with adjust_setup_cost makes sense, but please only > do that, or in a more obvious way. > >> } >> >> /* For each size of the induction variable set determine the penalty. */ >> -- >> 2.34.1
