[PATCH] D83859: [SVE] Don't use LocalStackAllocation for SVE objects

Wed Jul 15 04:33:13 PDT 2020

paulwalker-arm added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.h:107

+  bool bundleStackIdIntoLocalArea(unsigned StackId) const override {
+    // We don't support putting SVE objects into the pre-allocated local
----------------
To me the name implies something that is safe either way, whereas you're making the change to fix a bug.  Would isStackIdSafeForLocalArea be a better fit?

================
Comment at: llvm/test/CodeGen/AArch64/sve-localstackalloc.mir:1
+# RUN: llc -mtriple=aarch64--linux-gnu -mattr=+sve -start-before=localstackalloc -debug-only=localstackalloc -o - %s 2>&1 | FileCheck %s
+
----------------
I forget the exact requirement but does the use of -debug-only mean this tests needs
```
REQUIRES: Asserts
```
or something.  It is not possible to test the expected frame/stack layout instead, rather the rely on debug?

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

https://reviews.llvm.org/D83859