[llvm-commits] [llvm] r83136 - /llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp

[Jim Grosbach]

On Sep 30, 2009, at 1:10 AM, Evan Cheng wrote:

>
> On Sep 29, 2009, at 6:43 PM, Jim Grosbach wrote:
>> =
>> =====================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMBaseRegisterInfo.cpp Tue Sep 29  
>> 20:43:29 2009
>> @@ -654,7 +654,13 @@
>>    // adjustments also, even when the frame itself is small.
>>    if (RS && !ExtraCSSpill) {
>>      MachineFrameInfo  *MFI = MF.getFrameInfo();
>> -      if (estimateStackSize(MF, MFI) >= estimateRSStackSizeLimit(MF)
>> +      // If any of the stack slot references may be out of range  
>> of an
>> +      // immediate offset, make sure a register (or a spill slot) is
>> +      // available for the register scavenger. Note that if we're  
>> indexing
>> +      // off the frame pointer, the effective stack size is 4  
>> bytes larger
>> +      // since the FP points to the previous FP.
>> +      if (estimateStackSize(MF, MFI) + (hasFP(MF) ? 4 : 0)
>> +          >= estimateRSStackSizeLimit(MF)
>
> In processFunctionBeforeCalleeSavedScan:
>
>    // Darwin ABI requires FP to point to the stack slot that  
> contains the
>    // previous FP.
>    if (STI.isTargetDarwin() || hasFP(MF)) {
>      MF.getRegInfo().setPhysRegUsed(FramePtr);
>      NumGPRSpills++;
>    }
>
> So if it's Darwin it spills fp even if hasFP is false (unless the  
> function is frameless). Also FP points to the slot that contains  
> previous FP. It's not "FP points to the previous FP". Is the  
> adjustment 4 always right?
>

Hi Evan,

Thanks for the review.

Whether the FP is spilled or not isn't the determining factor for  
whether the adjustment is necessary. Rather, it's whether frame  
indices will be resolved via the frame pointer or the stack pointer.  
That's answered by hasFP(), even on Darwin.

The adjustment is the size of the extra stack space taken up by the  
spilled FP, which is just the size of the register. I looked again and  
didn't see alignment tricks or anything like that that aren't already  
taken into account elsewhere, so I don't believe this code has to do  
anything special.

You're right that the language of the comment is sloppy. Cleared that  
up in r83148.


Thanks,

   -Jim