[PATCH] D97193: [AArch64][SVE] Ensure hasFP has a consistent return value

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:373
+  // Only perform the below in the presence of SVE so as to avoid reserving x29
+  // unnecessarily.
+  if (STI.hasSVE()) {
----------------
nit: maybe add a TODO/FIXME comment to say that this approach is probably temporary and may be removed in the future when LocalStackSlotAllocation works on multiple StackIDs. (as that will have created virtual base pointers, so there's no long the need to always force the availability of the FP).


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2951
 
+void AArch64FrameLowering::processFunctionBeforeCalleeSpill(
+    MachineFunction &MF) const {
----------------
Given that the approach is probably temporary, does it necessarily need a new generic callback, or could this be called from early-on in determineCalleeSaves?


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:2955
+  AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
+
+  // Determine whether this function should use a frame pointer or not. This
----------------
This can probably do with an early bail-out if it is not compiling for SVE.


================
Comment at: llvm/test/CodeGen/AArch64/sve-calling-convention-mixed.ll:45
 ; CHECK-NEXT:    addvl sp, sp, #-4
+; CHECK-NEXT:    sub sp, sp, #16 // =16
 ; CHECK-NEXT:    ptrue p0.b
----------------
Why does this function not require the frame-pointer? (it has both locals for passing GPRs and ZPRs)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97193