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

Fri Nov 29 06:26:28 PST 2019

kiranchandramohan marked 6 inline comments as done.
kiranchandramohan added a comment.

In general, I also worry that i might have missed some checks. I was hoping to get some pointers on how to run some tests so that we can minimise this. At the same time, I also feel that this change should not cause regressions.

================
Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:1037
   if (windowsRequiresStackProbe(MF, NumBytes)) {
     uint32_t NumWords = NumBytes >> 4;
     if (NeedsWinCFI) {
----------------
efriedma wrote:
> This should also be uint64_t?
Initially missed out since this portion of the code seems to error out. But you are correct that it has to be int64_t to prevent overflows.

================
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]]
----------------
efriedma wrote:
> 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?
I have removed the variables from here. 

A few of these are CHECKS and not CHECK-NEXT.
1) Skips over some cfi attributes
; CHECK:                  sub   x[[INDEX:[0-9]+]], x[[FRAME]], #8
2) Skips over some set up for calling the print function
; CHECK:                  bl    printf
3) The CHECK-COUNTs did not seem to have a way of combining with NEXT.
; CHECK-COUNT-128:        sub   sp, sp, #[[STACK1:[0-9]+]], lsl #12
; CHECK-COUNT-128:        add   sp, sp, #[[STACK1]], lsl #12

Please let me know if this is not OK.

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

https://reviews.llvm.org/D70496