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

[Bradley Smith via Phabricator via llvm-commits]

bsmith added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h:251-253
+  bool isCalleeSavedStackSizeComputed() const {
+    return HasCalleeSavedStackSize;
+  }
----------------
sdesmalen wrote:
> You should be able to use `MFI.isCalleeSavedInfoValid()` (instead of having to add this function).
CalleeSavedInfoValid is set before the callee saved stack size is set, hence unfortunately this isn't sufficient. Doing this triggers an assertion: 

  unsigned int llvm::AArch64FunctionInfo::getCalleeSavedStackSize() const: Assertion `HasCalleeSavedStackSize && "CalleeSavedStackSize has not been calculated"' failed.

Given that the callee saved stack size being set is protected by `HasCalleeSavedStackSize` I think it makes sense to expose that value.


================
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
----------------
sdesmalen wrote:
> 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_*`)
I think it's possible that all of these tests are actually covered already in `llvm/test/CodeGen/AArch64/framelayout-sve.mir`, perhaps I should just remove these completely?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94401