[PATCH] D35919: [AArch64] Fix callee saved registers in win64 vararg functions with CombineSPBump=0

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo created this revision.
Herald added subscribers: kristof.beyls, javed.absar, rengolin, aemerson.

Previously, the pass_va test functions were built as CombineSPBump=1, but now this function gets CombineSPBump=0 and a slightly different prologue.

The previous attempt at offsetting the manually stored registers failed when the first callee saved registers are stored with a pre-indexed str with writeback, like pass_va is built now. This attempt seems much less hacky.

---

I've got a pretty big function that I can try to make a test out from - now all callee saved registers are stored in the exact same spot as they are described to reside in in the MIR.

I haven't triggered codepaths with HasFP=1 in tests yet - any suggestions on tests that would do that, or any other combinations that I should test as well? I've gotten it pretty well tested in a rather big codebase in practice now at least.


https://reviews.llvm.org/D35919

Files:
  lib/Target/AArch64/AArch64FrameLowering.cpp
  test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
  test/CodeGen/AArch64/win64_vararg.ll


Index: test/CodeGen/AArch64/win64_vararg.ll
===================================================================
--- test/CodeGen/AArch64/win64_vararg.ll
+++ test/CodeGen/AArch64/win64_vararg.ll
@@ -2,14 +2,14 @@
 
 define void @pass_va(i32 %count, ...) nounwind {
 entry:
-; CHECK: sub     sp, sp, #80
+; CHECK: str     x30, [sp, #-80]!
 ; CHECK: add     x8, sp, #24
 ; CHECK: add     x0, sp, #24
 ; CHECK: stp     x6, x7, [sp, #64]
 ; CHECK: stp     x4, x5, [sp, #48]
 ; CHECK: stp     x2, x3, [sp, #32]
 ; CHECK: str     x1, [sp, #24]
-; CHECK: stp     x30, x8, [sp]
+; CHECK: str     x8, [sp, #8]
 ; CHECK: bl      other_func
 ; CHECK: ldr     x30, [sp], #80
 ; CHECK: ret
Index: test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
===================================================================
--- test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
+++ test/CodeGen/AArch64/aarch64_win64cc_vararg.ll
@@ -2,14 +2,14 @@
 
 define win64cc void @pass_va(i32 %count, ...) nounwind {
 entry:
-; CHECK: sub     sp, sp, #80
+; CHECK: str     x30, [sp, #-80]!
 ; CHECK: add     x8, sp, #24
 ; CHECK: add     x0, sp, #24
 ; CHECK: stp     x6, x7, [sp, #64]
 ; CHECK: stp     x4, x5, [sp, #48]
 ; CHECK: stp     x2, x3, [sp, #32]
 ; CHECK: str     x1, [sp, #24]
-; CHECK: stp     x30, x8, [sp]
+; CHECK: str     x8, [sp, #8]
 ; CHECK: bl      other_func
 ; CHECK: ldr     x30, [sp], #80
 ; CHECK: ret
Index: lib/Target/AArch64/AArch64FrameLowering.cpp
===================================================================
--- lib/Target/AArch64/AArch64FrameLowering.cpp
+++ lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -506,7 +506,10 @@
     return;
   }
 
-  auto CSStackSize = AFI->getCalleeSavedStackSize();
+  bool IsWin64 = Subtarget.isCallingConvWin64(MF.getFunction()->getCallingConv());
+  unsigned GPRSaveSize = IsWin64 ? alignTo(AFI->getVarArgsGPRSize(), 16) : 0;
+
+  auto CSStackSize = AFI->getCalleeSavedStackSize() + GPRSaveSize;
   // All of the remaining stack allocations are for locals.
   AFI->setLocalStackSize(NumBytes - CSStackSize);
 
@@ -533,7 +536,7 @@
   }
   if (HasFP) {
     // Only set up FP if we actually need to. Frame pointer is fp = sp - 16.
-    int FPOffset = CSStackSize - 16;
+    int FPOffset = CSStackSize - GPRSaveSize - 16;
     if (CombineSPBump)
       FPOffset += AFI->getLocalStackSize();
 
@@ -759,7 +762,10 @@
   // AArch64TargetLowering::LowerCall figures out ArgumentPopSize and keeps
   // it as the 2nd argument of AArch64ISD::TC_RETURN.
 
-  auto CSStackSize = AFI->getCalleeSavedStackSize();
+  bool IsWin64 = Subtarget.isCallingConvWin64(MF.getFunction()->getCallingConv());
+  unsigned GPRSaveSize = IsWin64 ? alignTo(AFI->getVarArgsGPRSize(), 16) : 0;
+
+  auto CSStackSize = AFI->getCalleeSavedStackSize() + GPRSaveSize;
   bool CombineSPBump = shouldCombineCSRLocalStackBump(MF, NumBytes);
 
   if (!CombineSPBump && CSStackSize != 0)
@@ -956,12 +962,6 @@
          "Odd number of callee-saved regs to spill!");
   int Offset = AFI->getCalleeSavedStackSize();
 
-  unsigned GPRSaveSize = AFI->getVarArgsGPRSize();
-  const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
-  bool IsWin64 = Subtarget.isCallingConvWin64(MF.getFunction()->getCallingConv());
-  if (IsWin64)
-    Offset -= alignTo(GPRSaveSize, 16);
-
   for (unsigned i = 0; i < Count; ++i) {
     RegPairInfo RPI;
     RPI.Reg1 = CSI[i].getReg();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35919.108362.patch
Type: text/x-patch
Size: 3379 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170726/584421b2/attachment.bin>