Hello Jeff and Andrea, absolutely correct, I've done a silly mistake. I apologize for taking up your precious time.
Thanks, Jovan On 8/30/26 00:25, Jeff Law 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 8/29/26 3:34 PM, Andrea Pinski wrote: >> On Sat, Aug 29, 2026 at 11:18 AM Jeff Law >> <[email protected]> wrote: >>> >>> On 8/19/26 9:04 AM, Jovan Dmitrovic 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. >>>> When invariants are spilled, only loading the invariant back to >>>> registers should be taken into account, as we don't need to save them >>>> more than once. This isn't the case for IV candidates which have to >>>> be both loaded and stored for each iteration of the loop. >>>> >>>> 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]> >>>> --- >>> I've pushed this one as well. Thanks. >> Except this one broke bootstrap. >> /home/apinski/src/upstream-gcc-new/gcc/gcc/tree-ssa-loop-ivopts.cc:6109:73: >> >> error: statement has no effect [-Werror=unused-value] >> 6109 | + target_spill_cost[speed] * (n_cands - >> available_regs) / 2; >> | >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~ >> >> >> The patch does: >> + cost = target_spill_cost [speed] * (regs_needed - >> available_regs) / 2; >> + + target_spill_cost[speed] * (n_cands - >> available_regs) / 2; >> >> Since there is a semi-colon before the plus sign, the second part of >> the expression is unused. >> The plus becomes an unary operator +. So the whole second part of what >> looks like the full expression goes unused. > > The intent here is clear and I'll go fix the undesirable semi-colon. It > raises some questions about the testing these patches received -- I'm > hoping the author just made a goof and sent the wrong version. Having > made that kind of mistake myself in the past I know it's easy to do > depending on the precise workflows one uses. > > > Jeff
