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

[Alex Bradbury via llvm-dev]

## 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