[PATCH] D70496: [AArch64] Fix issues with large arrays on stack

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

The general approach here seems fine, I guess.  At least, we shouldn't crash or miscompile, and generating code to allocate the full requested stack is probably the easiest way to do that without worrying about weird edge cases.  If we really cared about this, we might want to consider some heuristics to "split" the stack so spill slots and small variables are cheap to access, but I doubt that's actually important in practice.

Realistically, I can't imagine any good result from code allocating an 8GB array on the stack; no operating system is going to allocate enough stack to make this work, at least by default.



================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1395
+  int64_t NumBytes = IsFunclet ? getWinEHFuncletFrameSize(MF)
                            : MFI.getStackSize();
   AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
----------------
Indentation here needs to be fixed.


================
Comment at: llvm/lib/Target/AArch64/AArch64MachineFunctionInfo.h:43
   /// callee.
   unsigned BytesInStackArgArea = 0;
 
----------------
Is there a potential problem here as well?


================
Comment at: llvm/test/CodeGen/AArch64/large-stack.ll:30
+; CHECK-FRAME:                mov	x[[FRAME:[0-9]+]], sp
+; CHECK-SETSTACK-COUNT-128:   sub	sp, sp, #[[STACK1:[0-9]+]], lsl #[[SHIFT:[0-9]+]]
+; CHECK-SETSTACK-NEXT:        sub	sp, sp, #[[STACK2:[0-9]+]], lsl #[[SHIFT]]
----------------
What sequence are we generating to adjust the stack pointer?  For a large array that fits in 32 bits, I see a long sequence of  "sub sp, sp, #4095, lsl #12"; are we doing the same thing here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70496