[PATCH] D94401: [AArch64][SVE] Allow accesses to SVE stack objects to use frame pointer

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1945
+        // locals/spills.
+        if (FPOffsetFits && PreferFP && !SVEStackSize)
           UseFP = true;
----------------
is it worth folding the condition for SVEStackSize into FPOffsetFits above?

  // The FP offset will not fit if there is an SVE area in the way.
  if (SVEStackSize && FPOffset < 0)
    FPOffsetFits = false;


================
Comment at: llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h:251-253
+  bool isCalleeSavedStackSizeComputed() const {
+    return HasCalleeSavedStackSize;
+  }
----------------
You should be able to use `MFI.isCalleeSavedInfoValid()` (instead of having to add this function).


================
Comment at: llvm/test/CodeGen/AArch64/sve-frame-registers.ll:2
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve -O3 -o - %s | FileCheck %s
+
+; Check that FP is used for SVE stack objects and non-SVE objects in the CSR
----------------
Can you amend `llvm/test/CodeGen/AArch64/framelayout-sve.mir` with these test cases, in favour of having this LLVM IR file?
There should already be existing tests for checking the right base register is used (search for `test_address_*`)


================
Comment at: llvm/test/CodeGen/AArch64/sve-frame-registers.ll:15
+  store volatile <vscale x 2 x i32> %v0, <vscale x 2 x i32>* %local0
+  ; CHECK: st1w { z0.d }, p0, [x29, #-1, mul vl]
+  store volatile i32 %v1, i32* %local1
----------------
nit: normally the CHECK lines are all bundled together under the CHECK-LABEL. (that's also what scripts like `update_llc_test_checks.py` do).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94401