[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

[Lucas Prates via Phabricator via llvm-commits]

pratlucas marked 2 inline comments as done.
pratlucas added inline comments.


================
Comment at: llvm/lib/Target/ARM/Thumb1FrameLowering.cpp:1167
+                   STI.hasV5TOps());
+  // Only unused return registers can be used as copy regs at this point
+  popRegsFromStack(MBB, MI, TII, FrameRecord, UnusedReturnRegs, IsVarArg,
----------------
efriedma wrote:
> Are we actually guaranteed to have any unused return registers at this point?  The calling convention uses r0-r3 to return values.
Although unlikely in thumb1, we could run out of return registers here indeed (the calling convention only uses `r2` and `r3` to return 128-bit containerized vectors).
I've updated the code to make use of a scratch register if that ever happens. This required D126285, a minor NFC change so it could work correctly.


================
Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:73
+; CHECK-AAPCS: mov r0, r11
+; CHECK-AAPCS: str r1, [r0, #8]
+; CHECK-AAPCS: mov r0, r11
----------------
efriedma wrote:
> pratlucas wrote:
> > efriedma wrote:
> > > Can we use sp-relative accesses here?  If we're not doing dynamic allocations, it should be a fixed offset.
> > The current criteria for the Arm and Thumb backends is that fixed  frame indices are always accessed through FP whenever it is available (See [[ https://github.com/llvm/llvm-project/blob/aed49eac87b8aa77298252ea781bae4725ae8046/llvm/lib/Target/ARM/ARMFrameLowering.cpp#L1083 | ARMFrameLowering.cpp ]]).
> > I guess that could be changed, but I feel it would fall outside the scope of this patch.
> This patch does make it much more likely that fp-relative accesses are going to be out of range, but sure, I guess we can treat it as a separate issue.
That's a good point. I've pondered over this for a bit and realized that this is not the intended behaviour - we want FP and the frame record to be compliant with AAPCS in the scenarios where they're supposed to be generated and not to make its use more likely.
I've updated the code to match this. The expected behaviour when using the new option should be:

  - `r11` is set as the frame pointer - It should be reserved and not used as a general purpose register
  - For a function, an AAPCS compliant frame record should be created and `r11` should point to its location on the stack if:
    - Frame pointer elimination is disabled for the function (leaf vs non-leaf)
    - Codegen requires the use of FP






Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125094/new/

https://reviews.llvm.org/D125094