Hi Jeff, From an empirical perspective, the value of your patch is hard to determine at this stage - one benchmark improved about 0.5% but others have marginally regressed.
From an intellectual perspective, however, your patch is clearly a Good Thing. If I am understanding correctly, your aim is to prevent cases where reload evicts a pseudo from a register that conflicts with the pseudo that we are trying to reload, thereby undoing the clever cost- based logic in IRA that gave the register to the current owner in the first place? My guess is that the performance drop could be attributed to reload being lucky in some cases and your patch is preventing this luck from happening. Whilst I'm more of a risk-taker by nature, my employer would prefer more predictable behaviour from its compiler, so we will likely commit your patch to our development branch. Thanks very much! Is there an ETA on when reload will be gone away? ;-) Best regards, Ian > -----Original Message----- > From: Jeff Law [mailto:l...@redhat.com] > Sent: 22 October 2009 23:05 > To: Ian Bolton > Cc: Vladimir Makarov; gcc@gcc.gnu.org > Subject: Re: Understanding IRA > > On 10/19/09 12:30, Ian Bolton wrote: > > Hi Jeff and Vladimir. > > > > Jeff: I'd be interested in trying the patch if you can send it my > way. > > > It's nothing special. > > /* Return nonzero if REGNO is a particularly bad choice for reloading > X. */ > static int > ira_bad_reload_regno_1 (int regno, rtx x) > { > int x_regno; > ira_allocno_t a; > enum reg_class pref; > > /* We only deal with pseudo regs. */ > if (! x || GET_CODE (x) != REG) > return 0; > > x_regno = REGNO (x); > if (x_regno < FIRST_PSEUDO_REGISTER) > return 0; > > /* If the pseudo prefers REGNO explicitly, then do not consider > REGNO a bad spill choice. */ > pref = reg_preferred_class (x_regno); > if (reg_class_size[pref] == 1 > && TEST_HARD_REG_BIT (reg_class_contents[pref], regno)) > return 0; > > /* If the pseudo conflicts with REGNO, then we consider REGNO a > poor choice for a reload regno. */ > a = ira_regno_allocno_map[x_regno]; > if (TEST_HARD_REG_BIT (ALLOCNO_TOTAL_CONFLICT_HARD_REGS (a), regno)) > return 1; > > return 0; > } > > /* Return nonzero if REGNO is a particularly bad choice for reloading > IN or OUT. */ > int > ira_bad_reload_regno (int regno, rtx in, rtx out) > { > return (ira_bad_reload_regno_1 (regno, in) > || ira_bad_reload_regno_1 (regno, out)); > } > > Then change the loop in allocate_reload_reg to iterate 3 times intead > of > 2. And add this fragment > > > > if (pass == 1 > && ira_bad_reload_regno (regnum, rld[r].in, rld[r].out)) > continue; > > > To body of hte conditional starting with > > if ((reload_reg_free_p ... > > > It's really just a hack. I don't want to spend much time on that code > as ultimately I want it all to go away. > > Jeff > > >
