[llvm-commits] llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp

[Chris Lattner]

On Mar 5, 2007, at 12:58 AM, Nicolas Geoffray wrote:

> Chris Lattner wrote:
>>
>> Ok, so it's not related to NoFramePointerElim?  If that's the  
>> case, you should just have Macho and ELF return different sets of  
>> callee saved regs.
>>
> No, that's not the issue.
>
> Let me rephrase why I need this patch :)
>
> In PowerPC, whether it's on ELF ABI or MachO, R31 is a normal,  
> callee-saved register. With one exception: when it is used as a  
> frame pointer. When it is used as a frame pointer, both MachO and  
> ELF remove R31 from the callee-saved registers set.

Ok.

> There are two different ways to use R31 as a frame pointer: 1) with  
> an alloca when the stack is growing in a non-compilation  
> deterministic size, or 2) when NoFramePointerElim is set.

Ah, gotcha.

> 1) When there is an alloca that modifies the size of the stack,  
> compilation goes through the LowerDYNAMIC_STACKALLOC method. In  
> this method, you find the code:
>
>
>    // Find out what the fix offset of the frame pointer save area.
>    int FPOffset = PPCFrameInfo::getFramePointerSaveOffset(IsPPC64,  
> isMachoABI);
>    // Allocate the frame index for frame pointer save area.
>    FPSI = MF.getFrameInfo()->CreateFixedObject(IsPPC64? 8 : 4,  
> FPOffset);
>    // Save the result.
>    FI->setFramePointerSaveIndex(FPSI);

Ok

> With this code you give the info to the FrameInfo object that the  
> frame pointer offset is at FPOffset. Therefore, this offset will  
> never be used as an offset for another purpose. And this is  
> _really_ important for ELF, because the frame pointer, R31, is  
> saved in the callee-saved area. With this code, you are sure that  
> no callee-saved register will be spilled to R31's offset.

Ok.

> 2) When NoFramePointerElim is set, R31 is always used as a frame  
> pointer. However, the stack size may be decided at a compilation  
> time, therefore the compilation process never goes through the  
> LowerDYNAMIC_STACKALLOC. And LowerDYNAMIC_STACKALLOC is the only  
> method that saves the frame pointer offset in the frame info.
> The bug arrives here: the frame info object never had the info that  
> R31 is saved in the callee-saved area. Therefore, it may allocate  
> the frame pointer offset for spilling an other register. This was  
> not an issue for the MachO ABI because the frame pointer offset in  
> MachO is not in the callee-saved area.
>
> With this patch I give the info to the FrameInfo object, before the  
> callee-saved scan, that R31 is saved in the callee-saved area. Thus  
> I am sure that no register will be spilled in R31's offset.

Ahhh, ok.  I think I understand now what is going on.  Thank you for  
the very clear explanation.

In this case, it seems like a clearly good thing to just call  
CreateFixedObject unconditionally early on (e.g.) when lowering the  
arguments.

I don't have your proposed patch handy, but it seems like you know  
what the issues are :), so go ahead and check it in.

-Chris