[PATCH] D70800: Fix AArch64 AAPCS frame record chain

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2289
+      AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
+      if (Reg1 == AArch64::FP)
+        AFI->setFrameRecordOffset((RPI.Offset + 1) * Size);
----------------
logan wrote:
> sdesmalen wrote:
> > There is a condition here that is not yet tested. If the frame-record is saved that is both LR and FP, not just FP, so is this case needed?
> I think we don't have to check LR here because AAPCS64 guarantees FP and LR will be spilled to consecutive words.  Besides, we only care about the address of the spilled FP.
If `LR` is also spilled, shouldn't `isPaired()` be true? This is testing two layouts `(LR, FP)` and `(FP, <something else>)`, where the latter is not a frame-record.

This makes me think that this code also needs a condition for `hasFP(MF)`, because that guarantees the existence of a frame-record (as opposed to ordinary spills of FP/LR that don't necessarily constitute the framerecord)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70800