[llvm-commits] [llvm] r83521 - in /llvm/trunk: include/llvm/Target/TargetRegisterInfo.h lib/CodeGen/PrologEpilogInserter.cpp lib/CodeGen/PrologEpilogInserter.h lib/Target/ARM/Thumb1RegisterInfo.cpp lib/Target/ARM/Thumb1RegisterInfo.h

Evan Cheng evan.cheng at apple.com
Thu Oct 8 10:59:17 PDT 2009


On Oct 7, 2009, at 11:20 PM, Jim Grosbach wrote:

> Heya,
>
> I didn't see any regressions when I ran the singlesource tests by  
> hand, and the nightly run last night looks good with the scavenging  
> in for LLCBETA. If there's a problem on tonight's run, it'll be the  
> register re-use stuff. I wanted to get this in this evening so we'd  
> have one change at a time per nightly run to make things simpler to  
> track down if there's problems. For an additional sanity check, I  
> also built a sample application (unzip) and ran some tests of that  
> and it worked great (and had better codegen).

Ok. In addition to "no new failures", please make sure all of thumb1  
tests are passing. The #1 reason for this work is to fix remaining  
failures that are caused R3 being clobbered.

>
> Assuming things look good, I have a patch to enable allocation of R3  
> ready to go. I held off on that for tonight so as not to change too  
> many things per nightly run. Similarly, I looked more closely at  
> getting the ARM and T2 targets to use the new scheme, and I didn't  
> see anything that will make that non-trivial.
>

Ok.

> Basically, by tomorrow we should have the functionality in place.  
> Still some cleanup to do to address a few of your comments I haven't  
> gotten to yet and a few other things I've been keeping notes on that  
> I want to fix.

Ok. I strongly suggest eliminating the call to findLastUseReg() in the  
scavengeFrameVirtualRegs loop. The other changes I want are listed  
below.


>
> FWIW, I looked again at the eliminateFrameIndex() hooks and didn't  
> see any way around the changes I made there to track the values and  
> associated virtual registers. We can talk over the specifics  
> tomorrow. Maybe you'll have some ideas for ways to avoid that stuff.  
> It'd be very nice to not need to have that bit. Would make it a lot  
> easier to make other ports use the new stuff and such.

Let's talk offline about this.

BTW, please fix this compilation warning:

PrologEpilogInserter.cpp: In member function 'void  
llvm::PEI::scavengeFrameVirtualRegs(llvm::MachineFunction&)':
PrologEpilogInserter.cpp:770: warning: 'PrevValue' may be used  
uninitialized in this function

Evan

>
> Regards,
>   Jim
>
> On Oct 7, 2009, at 10:38 PM, Evan Cheng wrote:
>
>> Nice. Are all Thumb1 tests passing?
>>
>> Evan
>>
>> On Oct 7, 2009, at 6:46 PM, Jim Grosbach wrote:
>>
>>> Author: grosbach
>>> Date: Wed Oct  7 20:46:59 2009
>>> New Revision: 83521
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=83521&view=rev
>>> Log:
>>> Re-enable register scavenging in Thumb1 by default.
>>>
>>> Modified:
>>>  llvm/trunk/include/llvm/Target/TargetRegisterInfo.h
>>>  llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp
>>>  llvm/trunk/lib/CodeGen/PrologEpilogInserter.h
>>>  llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.cpp
>>>  llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.h
>>>
>>> Modified: llvm/trunk/include/llvm/Target/TargetRegisterInfo.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetRegisterInfo.h?rev=83521&r1=83520&r2=83521&view=diff
>>>
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/include/llvm/Target/TargetRegisterInfo.h (original)
>>> +++ llvm/trunk/include/llvm/Target/TargetRegisterInfo.h Wed Oct  7  
>>> 20:46:59 2009
>>> @@ -561,6 +561,12 @@
>>>   return false;
>>> }
>>>
>>> +  /// requiresFrameIndexScavenging - returns true if the target  
>>> requires post
>>> +  /// PEI scavenging of registers for materializing frame index  
>>> constants.
>>> +  virtual bool requiresFrameIndexScavenging(const MachineFunction  
>>> &MF) const {
>>> +    return false;
>>> +  }
>>> +
>>> /// hasFP - Return true if the specified function should have a  
>>> dedicated
>>> /// frame pointer register. For most targets this is true only if  
>>> the function
>>> /// has variable sized allocas or if frame pointer elimination is  
>>> disabled.
>>>
>>> Modified: llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp?rev=83521&r1=83520&r2=83521&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/PrologEpilogInserter.cpp Wed Oct  7  
>>> 20:46:59 2009
>>> @@ -44,16 +44,6 @@
>>> static RegisterPass<PEI>
>>> X("prologepilog", "Prologue/Epilogue Insertion");
>>>
>>> -// FIXME: For now, the frame index scavenging is off by default  
>>> and only
>>> -// used by the Thumb1 target. When it's the default and replaces  
>>> the current
>>> -// on-the-fly PEI scavenging for all targets,  
>>> requiresRegisterScavenging()
>>> -// will replace this.
>>> -cl::opt<bool>
>>> -FrameIndexVirtualScavenging("enable-frame-index-scavenging",
>>> -                            cl::Hidden,
>>> -                            cl::desc("Enable frame index  
>>> elimination with"
>>> -                                     "virtual register  
>>> scavenging"));
>>> -
>>> /// createPrologEpilogCodeInserter - This function returns a pass  
>>> that inserts
>>> /// prolog and epilog code, and eliminates abstract frame  
>>> references.
>>> ///
>>> @@ -66,6 +56,7 @@
>>> const Function* F = Fn.getFunction();
>>> const TargetRegisterInfo *TRI = Fn.getTarget().getRegisterInfo();
>>> RS = TRI->requiresRegisterScavenging(Fn) ? new RegScavenger() :  
>>> NULL;
>>> +  FrameIndexVirtualScavenging = TRI->requiresFrameIndexScavenging 
>>> (Fn);
>>>
>>> // Get MachineModuleInfo so that we can track the construction of  
>>> the
>>> // frame.
>>>
>>> Modified: llvm/trunk/lib/CodeGen/PrologEpilogInserter.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PrologEpilogInserter.h?rev=83521&r1=83520&r2=83521&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/lib/CodeGen/PrologEpilogInserter.h (original)
>>> +++ llvm/trunk/lib/CodeGen/PrologEpilogInserter.h Wed Oct  7  
>>> 20:46:59 2009
>>> @@ -94,6 +94,11 @@
>>>   // functions.
>>>   bool ShrinkWrapThisFunction;
>>>
>>> +    // Flag to control whether to use the register scavenger to  
>>> resolve
>>> +    // frame index materialization registers. Set according to
>>> +    // TRI->requiresFrameIndexScavenging() for the curren function.
>>> +    bool FrameIndexVirtualScavenging;
>>> +
>>>   // When using the scavenger post-pass to resolve frame reference
>>>   // materialization registers, maintain a map of the registers to
>>>   // the constant value and SP adjustment associated with it.
>>>
>>> Modified: llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.cpp?rev=83521&r1=83520&r2=83521&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.cpp (original)
>>> +++ llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.cpp Wed Oct  7  
>>> 20:46:59 2009
>>> @@ -37,11 +37,6 @@
>>> #include "llvm/Support/raw_ostream.h"
>>> using namespace llvm;
>>>
>>> -// FIXME: This cmd line option conditionalizes the new register  
>>> scavenging
>>> -// implemenation in PEI. Remove the option when scavenging works  
>>> well enough
>>> -// to be the default.
>>> -extern cl::opt<bool> FrameIndexVirtualScavenging;
>>> -
>>> Thumb1RegisterInfo::Thumb1RegisterInfo(const ARMBaseInstrInfo &tii,
>>>                                      const ARMSubtarget &sti)
>>> : ARMBaseRegisterInfo(tii, sti) {
>>> @@ -84,9 +79,16 @@
>>>
>>> bool
>>> Thumb1RegisterInfo::requiresRegisterScavenging(const  
>>> MachineFunction &MF) const {
>>> -  return FrameIndexVirtualScavenging;
>>> +  return true;
>>> +}
>>> +
>>> +bool
>>> +Thumb1RegisterInfo::requiresFrameIndexScavenging(const  
>>> MachineFunction &MF)
>>> +  const {
>>> +  return true;
>>> }
>>>
>>> +
>>> bool Thumb1RegisterInfo::hasReservedCallFrame(MachineFunction &MF)  
>>> const {
>>> const MachineFrameInfo *FFI = MF.getFrameInfo();
>>> unsigned CFSize = FFI->getMaxCallFrameSize();
>>> @@ -128,13 +130,7 @@
>>>   unsigned LdReg = DestReg;
>>>   if (DestReg == ARM::SP) {
>>>     assert(BaseReg == ARM::SP && "Unexpected!");
>>> -      if (FrameIndexVirtualScavenging) {
>>> -        LdReg = MF.getRegInfo().createVirtualRegister 
>>> (ARM::tGPRRegisterClass);
>>> -      } else {
>>> -        LdReg = ARM::R3;
>>> -        BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVtgpr2gpr),  
>>> ARM::R12)
>>> -          .addReg(ARM::R3, RegState::Kill);
>>> -      }
>>> +      LdReg = MF.getRegInfo().createVirtualRegister 
>>> (ARM::tGPRRegisterClass);
>>>   }
>>>
>>>   if (NumBytes <= 255 && NumBytes >= 0)
>>> @@ -159,10 +155,6 @@
>>>   else
>>>     MIB.addReg(LdReg).addReg(BaseReg, RegState::Kill);
>>>   AddDefaultPred(MIB);
>>> -
>>> -    if (!FrameIndexVirtualScavenging && DestReg == ARM::SP)
>>> -      BuildMI(MBB, MBBI, dl, TII.get(ARM::tMOVgpr2tgpr), ARM::R3)
>>> -        .addReg(ARM::R12, RegState::Kill);
>>> }
>>>
>>> /// calcNumMI - Returns the number of instructions required to  
>>> materialize
>>> @@ -635,7 +627,6 @@
>>>   else  // tLDR has an extra register operand.
>>>     MI.addOperand(MachineOperand::CreateReg(0, false));
>>> } else if (Desc.mayStore()) {
>>> -    if (FrameIndexVirtualScavenging) {
>>>     VReg = MF.getRegInfo().createVirtualRegister 
>>> (ARM::tGPRRegisterClass);
>>>     assert (Value && "Frame index virtual allocated, but Value arg  
>>> is NULL!");
>>>     *Value = Offset;
>>> @@ -658,52 +649,6 @@
>>>       MI.addOperand(MachineOperand::CreateReg(FrameReg, false));
>>>     else // tSTR has an extra register operand.
>>>       MI.addOperand(MachineOperand::CreateReg(0, false));
>>> -    } else {
>>> -      // FIXME! This is horrific!!! We need register scavenging.
>>> -      // Our temporary workaround has marked r3 unavailable. Of  
>>> course, r3 is
>>> -      // also a ABI register so it's possible that is is the  
>>> register that is
>>> -      // being storing here. If that's the case, we do the  
>>> following:
>>> -      // r12 = r2
>>> -      // Use r2 to materialize sp + offset
>>> -      // str r3, r2
>>> -      // r2 = r12
>>> -      unsigned ValReg = MI.getOperand(0).getReg();
>>> -      unsigned TmpReg = ARM::R3;
>>> -      bool UseRR = false;
>>> -      if (ValReg == ARM::R3) {
>>> -        BuildMI(MBB, II, dl, TII.get(ARM::tMOVtgpr2gpr), ARM::R12)
>>> -          .addReg(ARM::R2, RegState::Kill);
>>> -        TmpReg = ARM::R2;
>>> -      }
>>> -      if (TmpReg == ARM::R3 && AFI->isR3LiveIn())
>>> -        BuildMI(MBB, II, dl, TII.get(ARM::tMOVtgpr2gpr), ARM::R12)
>>> -          .addReg(ARM::R3, RegState::Kill);
>>> -      if (Opcode == ARM::tSpill) {
>>> -        if (FrameReg == ARM::SP)
>>> -          emitThumbRegPlusImmInReg(MBB, II, TmpReg, FrameReg,
>>> -                                   Offset, false, TII, *this, dl);
>>> -        else {
>>> -          emitLoadConstPool(MBB, II, dl, TmpReg, 0, Offset);
>>> -          UseRR = true;
>>> -        }
>>> -      } else
>>> -        emitThumbRegPlusImmediate(MBB, II, TmpReg, FrameReg,  
>>> Offset, TII,
>>> -                                  *this, dl);
>>> -      MI.setDesc(TII.get(ARM::tSTR));
>>> -      MI.getOperand(i).ChangeToRegister(TmpReg, false, false,  
>>> true);
>>> -      if (UseRR)  // Use [reg, reg] addrmode.
>>> -        MI.addOperand(MachineOperand::CreateReg(FrameReg, false));
>>> -      else // tSTR has an extra register operand.
>>> -        MI.addOperand(MachineOperand::CreateReg(0, false));
>>> -
>>> -      MachineBasicBlock::iterator NII = next(II);
>>> -      if (ValReg == ARM::R3)
>>> -        BuildMI(MBB, NII, dl, TII.get(ARM::tMOVgpr2tgpr), ARM::R2)
>>> -          .addReg(ARM::R12, RegState::Kill);
>>> -      if (TmpReg == ARM::R3 && AFI->isR3LiveIn())
>>> -        BuildMI(MBB, NII, dl, TII.get(ARM::tMOVgpr2tgpr), ARM::R3)
>>> -          .addReg(ARM::R12, RegState::Kill);
>>> -    }
>>> } else
>>>   assert(false && "Unexpected opcode!");
>>>
>>>
>>> Modified: llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.h?rev=83521&r1=83520&r2=83521&view=diff
>>>
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> = 
>>> ====================================================================
>>> --- llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.h (original)
>>> +++ llvm/trunk/lib/Target/ARM/Thumb1RegisterInfo.h Wed Oct  7  
>>> 20:46:59 2009
>>> @@ -41,6 +41,7 @@
>>>   getPhysicalRegisterRegClass(unsigned Reg, EVT VT = MVT::Other)  
>>> const;
>>>
>>> bool requiresRegisterScavenging(const MachineFunction &MF) const;
>>> +  bool requiresFrameIndexScavenging(const MachineFunction &MF)  
>>> const;
>>>
>>> bool hasReservedCallFrame(MachineFunction &MF) const;
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>




More information about the llvm-commits mailing list