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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'm a little concerned we've missed some conversion somewhere, but I don't have any good suggestion for that; we currently don't use integer conversion warnings in LLVM, and that would be a giant project to change.

There's some potential here that the arithmetic could overflow, even in 64 bits, but I'm not sure how we can handle that cleanly; I think it's okay to put off solving that issue.



================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:920
+  int64_t NumBytes = IsFunclet ? getWinEHFuncletFrameSize(MF)
+                           : MFI.getStackSize();
   if (!AFI->hasStackFrame() && !windowsRequiresStackProbe(MF, NumBytes)) {
----------------
Indentation.


================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1037
   if (windowsRequiresStackProbe(MF, NumBytes)) {
     uint32_t NumWords = NumBytes >> 4;
     if (NeedsWinCFI) {
----------------
This should also be uint64_t?


================
Comment at: llvm/test/CodeGen/AArch64/large-stack.ll:29
+; CHECK:                  mov	x[[FRAME:[0-9]+]], sp
+; CHECK-COUNT-128:        sub	sp, sp, #[[STACK1:[0-9]+]], lsl #[[SHIFT:[0-9]+]]
+; CHECK-NEXT:             sub	sp, sp, #[[STACK2:[0-9]+]], lsl #[[SHIFT]]
----------------
Using FileCheck variables for values that will never change isn't helpful.  For example, "lsl #[[SHIFT:[0-9]+]]" is actually always "lsl #12", because that's the only legal value.

Is there a reason some of these CHECKs aren't CHECK-NEXT?


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

https://reviews.llvm.org/D70496