[llvm-commits] [llvm] r78318 - in /llvm/trunk: include/llvm/CodeGen/RegisterScavenging.h lib/CodeGen/RegisterScavenging.cpp

[John Mosby]

Hey Jakob,
Thanks for the reviews:

>>I was just about to remove RegScavenger::backward(). Will you be
>>needing it?

No, go ahead and kill it (if nobody else needs it).

>>Perhaps CalleeSavedRegs could be renamed to UsedCalleeSavedRegs or
>>something along those lines? Just so it is clear it doesn't correspond
>>to TRI->getCalleeSavedRegs().

I just used what was there. You're right, UsedCalleeSavedRegs is what I it
should be. I'll change the name.

>>Please check the assert condition - it doesn't do what the comment says.

Thanks, I'll fix it.

>>Have you considered simply marking the free CSRs as used in
>>initRegState? Then you wouldn't have to disable check for these registers.

If a CSR isn't used in a function, it doesn't need to be spilled on
entry/restored on return, so it should be usable by RS, hence must not be
taken out of RegsAvailable. Perhaps I'm not following you here(?)

On Thu, Aug 6, 2009 at 12:42 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

>
> On 06/08/2009, at 19.23, Jakob Stoklund Olesen wrote:
> >> void RegScavenger::enterBasicBlock(MachineBasicBlock *mbb) {
> >>  MachineFunction &MF = *mbb->getParent();
> >>  const TargetMachine &TM = MF.getTarget();
> >> @@ -94,6 +117,7 @@
> >>  assert((NumPhysRegs == 0 || NumPhysRegs == TRI->getNumRegs()) &&
> >>         "Target changed?");
> >>
> >> +  // Self-initialize.
> >>  if (!MBB) {
> >>    NumPhysRegs = TRI->getNumRegs();
> >>    RegsAvailable.resize(NumPhysRegs);
> >> @@ -104,29 +128,23 @@
> >>    // Create callee-saved registers bitvector.
> >>    CalleeSavedRegs.resize(NumPhysRegs);
> >>    const unsigned *CSRegs = TRI->getCalleeSavedRegs();
> >> -    if (CSRegs != NULL)
> >> -      for (unsigned i = 0; CSRegs[i]; ++i)
> >> -        CalleeSavedRegs.set(CSRegs[i]);
> >> -  }
> >
> > Perhaps CalleeSavedRegs could be renamed to UsedCalleeSavedRegs or
> > something along those lines? Just so it is clear it doesn't correspond
> > to TRI->getCalleeSavedRegs().
>
> One more thing: FindUnusedReg takes an ExCalleeSaved parameter. When
> that is set, all the non-spilled CSRs must be excluded. Otherwise a
> non-spilled CSR could be used without being spilled first.


If a CSR isn't used, it doesn't need to be spilled, yadda yadda yadda, RS
can use it.


>
>  From ARMLoadStoreOptimizer.cpp:
>
>         // Find a scratch register. Make sure it's a call clobbered
> register or
>         // a spilled callee-saved register.
>         unsigned Scratch = RS->FindUnusedReg(&ARM::GPRRegClass, true);
>
> I think what you want is a FreeCalleeSavedRegs variable.


How about UsedCalleeSavedRegs?

Thanks much!

John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20090806/b7817de0/attachment.html>