[PATCH] D65653: [AArch64] Change location of frame-record within callee-save area.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked an inline comment as done.
sdesmalen added inline comments.


================
Comment at: lib/Target/AArch64/AArch64CallingConvention.td:342
+def CSR_Win_AArch64_AAPCS : CalleeSavedRegs<(add X19, X20, X21, X22, X23, X24,
+                                               X25, X26, X27, X28, FP, LR,
                                                D8, D9, D10, D11,
----------------
efriedma wrote:
> Is the order of the registers in this list important?  If it is, should we explain it in a comment somewhere?
This is indeed important since various parts in AArch64FrameLowering depend on this order (see also my other comment). I've added a comment to describe this, thanks!


================
Comment at: test/CodeGen/AArch64/aarch64-dynamic-stack-layout.ll:583
+; CHECK: stp	x29, x30, [sp, #-32]!
+; CHECK: str  x19, [sp, #16]
 ;   Check that the frame pointer is created:
----------------
efriedma wrote:
> I don't understand what's happening here; are we reducing register pressure somhow?
Before it did an extra spill of `x28` to use as scratch register, but now it uses `x30` for that purpose.

This is due to a quirk in `determineCalleeSaves`, which iterates through the list of callee-saves and then spills the last unused register as a scratch register. With the previous order in which the callee-saves were specified in the .td file, this last register was `x28`. With the new ordering, the last unused register is `x30`. This register already conveniently stored as part of the frame-record since we're using a frame-pointer, so there is no need to spill an additional register.


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

https://reviews.llvm.org/D65653