[llvm-dev] RFC: Setting MachineInstr flags through storeRegToStackSlot

[Matthias Braun via llvm-dev]

Can someone familiar with debug info comment on whether it matters to have the FrameSetup flag on the callee save spills? We could have a smart spill or shrink wrapping algorithm that delays the callee saves to a later point in the program while executing non-prologue code first.

Is this maybe just meant to indicate the point at which the stack and possible frame/base pointers are setup? Saving the callee saves should not necessary be part of that, it probably is on X86 where the pushs/pops are part of getting the final value of the stack pointer in a function without frame pointer, but for the case where you use storeRegToStackSlot() I'm not sure we even need to mark them as FrameSetup...

- Matthias

> On Feb 17, 2017, at 3:33 AM, Alex Bradbury via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> ## Problem description
> 
> One of the responsibilities of a target's implementation of
> TargetFrameLowering::emitPrologue is to set the frame pointer (if needed).
> Typically, the frame pointer will be stored to the stack just like the other
> callee-saved registers, and emitPrologue must insert the instruction to change
> its value after it was stored to the stack. Mips does this by looking at the
> number of callee-saved registers spilled for this function and skipping over
> that many instructions. Other backends such as AArch64 or X86 will skip over
> all instruction marked with the MachineInstr::FrameSetup flag[1].
> 
> From my point of view, skipping over all FrameSetup instructions sounds like
> the more robust way to go about things. I haven't fully traced through where
> the flag is used in LLVM, but it does seem that DwarfDebug::findPrologueEndLoc
> will return the wrong result if the flag isn't consistently set on frame setup
> code. The problem is that unless you override
> TargetFrameLowering::spillCalleeSavedRegisters, then PrologEpilogInserter will
> just spill callee saved registers through a series of calls to
> storeRegToStackSlot. This is fine, except storeRegToStackSlot won't set the
> FrameSetup flag.
> 
> [1]: Actually X86 will only skip over PUSH instructions with the FrameSetup
> flag. Is this extra condition necessary?
> 
> 
> ## Potential solutions
> 
> 1) Keep the status quo, but modify PrologEpilogInserter so it will mark the
> last inserted instruction with FrameSetup over calling storeRegToStackSlot.
> This is exactly what X86 does in its spillCalleeSavedRegisters implementation.
> This does make the assumption that storeRegtoStackSlot only inserts a single
> instruction. From what I can see, that is true for all in-tree backends.
> Arguably, backends like Mips could be improved by modifying their
> spillCalleeSavedRegisters to set the FrameSetup flag as well.
> 
> 2) Like the above, but modify storeRegToStackSlot to return the number of
> MachineInstr inserted and use that value when marking instructions as
> FrameSetup. This is more invasive, as it will affect all in tree and
> out-of-tree backends.
> 
> 3) Add a MachineInstr::MIFlag parameter to storeRegToStackSlot. The
> storeRegToStackSlot implementation is responsible for applying this flag to
> all inserted instructions. This could be defaulted to MachineInstr::NoFlags,
> to minimise changes to function callers, although that defaulted value would
> have to be replicated in all FooTgtInstrInfo.h headers.
> 
> For either 2) or 3), the changes are simple, it's just they will affect all
> backends. I'm very happy to implement the patch, but thought it was worth
> asking feedback. I'm CCing in Tim+Craig+Simon as AArch64/X86/Mips code owners,
> as well as Eric as debug info code owner.
> 
> 
> Any feedback is very welcome.
> 
> Thanks,
> 
> Alex
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev