[AArch64] Implement support for windows style vararg functions

[mstorsjo]

Summary

Pass parameters properly in calls to such functions (pass all floats in integer registers), and handle va_start properly (allocate stack immediately below the arguments on the stack, to save the register arguments into a single continuous array).

[mstorsjo]

Added an initial test, fixed va_copy to copy the right amount of bytes, fixed the va_list pointer if the first variable argument is on the stack.

[t.p.northover]

lib/Target/AArch64/AArch64ISelLowering.cpp

2889 ↗

(On Diff #105525)

I don't understand this line, particularly the GPRSaveSize & 15. I'd expected to see -GPRSaveSize since the args are on the stack just before the canonical frame address (i.e. SP on function entry).

I assume the masking is a crude version of alignTo? I don't think that's going to be right. You need to make sure the stack is aligned in FrameLowering, but you're still going to want the FrameIndex that gets put into a va_list is able to point an odd number of registers below SP on entry if necessary.

Things look equally bad when I dump the MIR (e.g. from your first test-case):

[...]
fi#-1: size=56, align=8, fixed, at location [SP+8]
[...]
STRXui %X1<kill>, <fi#-1>, 0; mem:ST8[FixedStack-1+8]

So LLVM thinks X1 is going at the incoming SP+8 rather than (as actually happens) incoming SP-56 (I think).

So I think something is going horribly wrong during FrameLowering and LLVM is losing track of where SP actually is compared to function entry. I'll try to take a look again later, but I've not spotted the issue yet.

2901–2902 ↗

(On Diff #105525)

I think this is lying about the location isn't it? They're not all being stored at GPRIdx.

[t.p.northover]

I've thought some more and it seems pretty clear now that the difference is exactly that extra GPRSaveSize in FrameLowering. That needs to be incorporated into LLVM's view of the stack size and layout.

I think just using the correct offset in CreateFixedObject will do it for the size (though this is a pretty hazy memory from when I looked in detail multiple years ago so take it with a pinch of salt). That leaves the layout as the real problem: you can probably skip the emitFrameOffset but you need to convince FrameLowering to leave a gap before storing the callee-saved registers instead.

[t.p.northover]

OK, I now see that this actually works. You're essentially redefining the CFA (canonical frame address -- where LLVM bases its SP calculations) to the aligned SP after the extra GPRs have been pushed.

It's not thoroughly broken, but it is confusing and as far as I'm aware unprecedented (32-bit ARM always does this kind of jig to save varargs GPRs but doesn't mangle the CFA in the process). I still think switching back to the original CFA along roughly the lines I proposed earlier is the best way to implement this.

[mstorsjo]

Thanks for taking a look! Do you have a more concrete suggestion on how to make frame lowering leave a gap at the top, that doesn't mess up the layout like this does? This was a result of trying to take inspiration from the same code for arm, with a lot of blind guesswork and trial and error. If not, I can try to get back to trying things later today.

[mstorsjo]

Things look equally bad when I dump the MIR (e.g. from your first test-case):

What parameters do you use to produce this MIR dump btw? I found http://llvm.org/docs/MIRLangRef.html but llc -stop-after=machine-cp doesn't really give me the things you quoted.

[mstorsjo]

Avoiding the extra manual stack adjustment, using CreateFixedObject properly (or at least more properly than before).

[rnk]

lib/Target/AArch64/AArch64ISelLowering.cpp

4591–4593 ↗

(On Diff #106091)

IMO it would be nice to make this match with two ifs. Now that we have 3 cases, the old ternary conditional doesn't look right.

[rnk]

This seems like the right approach to me. It's like the Win x64 "shadow slots" are allocated on the callee side instead of the caller side. We might want to consider implementing /homeparams at some point across the Windows-supporting backends, which will require more FrameLowering surgery.

[mstorsjo]

Fixed the style issue pointed out by Reid, adjusted the test case and flipped the immutable parameter to CreateFixedObject.

[t.p.northover]

Thanks for updating the patch. Looks really good to me, just one trivial comment about a comment.

lib/Target/AArch64/AArch64ISelLowering.cpp

2833–2834 ↗

(On Diff #106308)

Minor nit (just change and commit), but that's peripheral to why Win64 needs registers to be saved. The key point is something along the lines of "Win64 also passes variadic arguments in registers".

llvm/trunk/lib/Target/AArch64/AArch64CallingConvention.td

CCIfType<[f64], CCAssignToRegWithShadow<[D0, D1, D2, D3, D4, D5, D6, D7],
                                           [Q0, Q1, Q2, Q3, Q4, Q5, Q6, Q7]>>,
   CCIfType<[v1i64, v2i32, v4i16, v8i8, v1f64, v2f32, v4f16],
       CCAssignToRegWithShadow<[D0, D1, D2, D3, D4, D5, D6, D7],
                               [Q0, Q1, Q2, Q3, Q4, Q5, Q6, Q7]>>,
   CCIfType<[f128, v2i64, v4i32, v8i16, v16i8, v4f32, v2f64, v8f16],
       CCAssignToReg<[Q0, Q1, Q2, Q3, Q4, Q5, Q6, Q7]>>
 ]>;
 
+// Vararg functions on windows pass floats in integer registers
+def CC_AArch64_Win64_VarArg : CallingConv<[
+  CCIfType<[f16, f32],    CCPromoteToType<f64>>,
+  CCIfType<[f64], CCBitConvertToType<i64>>,
+  CCDelegateTo<CC_AArch64_AAPCS>
+]>;
+
 
 // Darwin uses a calling convention which differs in only two ways
 // from the standard one at this level:
 //     + i128s (i.e. split i64s) don't need even registers.
 //     + Stack slots are sized as needed rather than being at least 64-bit.
 def CC_AArch64_DarwinPCS : CallingConv<[
   CCIfType<[iPTR], CCBitConvertToType<i64>>,
   CCIfType<[v2f32], CCBitConvertToType<v2i32>>,

llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp

bool AArch64FastISel::fastSelectInstruction(const Instruction *I) {
   case Instruction::AtomicCmpXchg:
     return selectAtomicCmpXchg(cast<AtomicCmpXchgInst>(I));
   }
 
   // fall-back to target-independent instruction selection.
   return selectOperator(I, I->getOpcode());
   // Silence warnings.
   (void)&CC_AArch64_DarwinPCS_VarArg;
+  (void)&CC_AArch64_Win64_VarArg;
 }
 
 namespace llvm {
 
 FastISel *AArch64::createFastISel(FunctionLoweringInfo &FuncInfo,
                                         const TargetLibraryInfo *LibInfo) {
   return new AArch64FastISel(FuncInfo, LibInfo);
 }
 
 } // end namespace llvm

llvm/trunk/lib/Target/AArch64/AArch64FrameLowering.cpp

 //
 // |                                   | Higher address
 // |-----------------------------------|
 // |                                   |
 // | arguments passed on the stack     |
 // |                                   |
 // |-----------------------------------|
 // |                                   |
+// | (Win64 only) varargs from reg     |
+// |                                   |
+// |-----------------------------------|
+// |                                   |
 // | prev_fp, prev_lr                  |
 // | (a.k.a. "frame record")           |
 // |-----------------------------------| <- fp(=x29)
 // |                                   |
 // | other callee-saved registers      |
 // |                                   |
 // |-----------------------------------|
 // |.empty.space.to.make.part.below....|
static void computeCalleeSaveRegisterPairs(
   unsigned Count = CSI.size();
   (void)CC;
   // MachO's compact unwind format relies on all registers being stored in
   // pairs.
   assert((!produceCompactUnwindFrame(MF) ||
           CC == CallingConv::PreserveMost ||
           (Count & 1) == 0) &&
          "Odd number of callee-saved regs to spill!");
-  unsigned Offset = AFI->getCalleeSavedStackSize();
+  int Offset = AFI->getCalleeSavedStackSize();
+
+  unsigned GPRSaveSize = AFI->getVarArgsGPRSize();
+  const AArch64Subtarget &Subtarget = MF.getSubtarget<AArch64Subtarget>();
+  if (Subtarget.isTargetWindows())
+    Offset -= alignTo(GPRSaveSize, 16);
 
   for (unsigned i = 0; i < Count; ++i) {
     RegPairInfo RPI;
     RPI.Reg1 = CSI[i].getReg();
 
     assert(AArch64::GPR64RegClass.contains(RPI.Reg1) ||
            AArch64::FPR64RegClass.contains(RPI.Reg1));
     RPI.IsGPR = AArch64::GPR64RegClass.contains(RPI.Reg1);

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h

private:
   SDValue LowerSELECT_CC(ISD::CondCode CC, SDValue LHS, SDValue RHS,
                          SDValue TVal, SDValue FVal, const SDLoc &dl,
                          SelectionDAG &DAG) const;
   SDValue LowerJumpTable(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerConstantPool(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerBlockAddress(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerAAPCS_VASTART(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerDarwin_VASTART(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerWin64_VASTART(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerVASTART(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerVACOPY(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerVAARG(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFRAMEADDR(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerINSERT_VECTOR_ELT(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerEXTRACT_VECTOR_ELT(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerSCALAR_TO_VECTOR(SDValue Op, SelectionDAG &DAG) const;

llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp

case CallingConv::WebKit_JS:
     return CC_AArch64_WebKit_JS;
   case CallingConv::GHC:
     return CC_AArch64_GHC;
   case CallingConv::C:
   case CallingConv::Fast:
   case CallingConv::PreserveMost:
   case CallingConv::CXX_FAST_TLS:
   case CallingConv::Swift:
+    if (Subtarget->isTargetWindows() && IsVarArg)
+      return CC_AArch64_Win64_VarArg;
     if (!Subtarget->isTargetDarwin())
       return CC_AArch64_AAPCS;
     return IsVarArg ? CC_AArch64_DarwinPCS_VarArg : CC_AArch64_DarwinPCS;
   }
 }
 
 CCAssignFn *
 AArch64TargetLowering::CCAssignFnForReturn(CallingConv::ID CC) const {
SDValue AArch64TargetLowering::LowerFormalArguments(
 
   // varargs
   AArch64FunctionInfo *FuncInfo = MF.getInfo<AArch64FunctionInfo>();
   if (isVarArg) {
     if (!Subtarget->isTargetDarwin()) {
       // The AAPCS variadic function ABI is identical to the non-variadic
       // one. As a result there may be more arguments in registers and we should
       // save them for future reference.
+      // Win64 variadic functions also pass arguments in registers, but all float
+      // arguments are passed in integer registers.
       saveVarArgRegisters(CCInfo, DAG, DL, Chain);
     }
 
     // This will point to the next argument passed via stack.
     unsigned StackOffset = CCInfo.getNextStackOffset();
     // We currently pass all varargs at 8-byte alignment.
     StackOffset = ((StackOffset + 7) & ~7);
     FuncInfo->setVarArgsStackIndex(MFI.CreateFixedObject(4, StackOffset, true));
static const MCPhysReg GPRArgRegs[] = { AArch64::X0, AArch64::X1, AArch64::X2,
                                           AArch64::X3, AArch64::X4, AArch64::X5,
                                           AArch64::X6, AArch64::X7 };
   static const unsigned NumGPRArgRegs = array_lengthof(GPRArgRegs);
   unsigned FirstVariadicGPR = CCInfo.getFirstUnallocated(GPRArgRegs);
 
   unsigned GPRSaveSize = 8 * (NumGPRArgRegs - FirstVariadicGPR);
   int GPRIdx = 0;
   if (GPRSaveSize != 0) {
+    if (Subtarget->isTargetWindows())
+      GPRIdx = MFI.CreateFixedObject(GPRSaveSize, -(int)GPRSaveSize, false);
+    else
       GPRIdx = MFI.CreateStackObject(GPRSaveSize, 8, false);
 
     SDValue FIN = DAG.getFrameIndex(GPRIdx, PtrVT);
 
     for (unsigned i = FirstVariadicGPR; i < NumGPRArgRegs; ++i) {
       unsigned VReg = MF.addLiveIn(GPRArgRegs[i], &AArch64::GPR64RegClass);
       SDValue Val = DAG.getCopyFromReg(Chain, DL, VReg, MVT::i64);
       SDValue Store = DAG.getStore(
           Val.getValue(1), DL, Val, FIN,
-          MachinePointerInfo::getStack(DAG.getMachineFunction(), i * 8));
+          Subtarget->isTargetWindows()
+              ? MachinePointerInfo::getFixedStack(DAG.getMachineFunction(),
+                                                  GPRIdx,
+                                                  (i - FirstVariadicGPR) * 8)
+              : MachinePointerInfo::getStack(DAG.getMachineFunction(), i * 8));
       MemOps.push_back(Store);
       FIN =
           DAG.getNode(ISD::ADD, DL, PtrVT, FIN, DAG.getConstant(8, DL, PtrVT));
     }
   }
   FuncInfo->setVarArgsGPRIndex(GPRIdx);
   FuncInfo->setVarArgsGPRSize(GPRSaveSize);
 
-  if (Subtarget->hasFPARMv8()) {
+  if (Subtarget->hasFPARMv8() && !Subtarget->isTargetWindows()) {
     static const MCPhysReg FPRArgRegs[] = {
         AArch64::Q0, AArch64::Q1, AArch64::Q2, AArch64::Q3,
         AArch64::Q4, AArch64::Q5, AArch64::Q6, AArch64::Q7};
     static const unsigned NumFPRArgRegs = array_lengthof(FPRArgRegs);
     unsigned FirstVariadicFPR = CCInfo.getFirstUnallocated(FPRArgRegs);
 
     unsigned FPRSaveSize = 16 * (NumFPRArgRegs - FirstVariadicFPR);
     int FPRIdx = 0;
SDValue AArch64TargetLowering::LowerDarwin_VASTART(SDValue Op,
   SDLoc DL(Op);
   SDValue FR = DAG.getFrameIndex(FuncInfo->getVarArgsStackIndex(),
                                  getPointerTy(DAG.getDataLayout()));
   const Value *SV = cast<SrcValueSDNode>(Op.getOperand(2))->getValue();
   return DAG.getStore(Op.getOperand(0), DL, FR, Op.getOperand(1),
                       MachinePointerInfo(SV));
 }
 
+SDValue AArch64TargetLowering::LowerWin64_VASTART(SDValue Op,
+                                                  SelectionDAG &DAG) const {
+  AArch64FunctionInfo *FuncInfo =
+      DAG.getMachineFunction().getInfo<AArch64FunctionInfo>();
+
+  SDLoc DL(Op);
+  SDValue FR = DAG.getFrameIndex(FuncInfo->getVarArgsGPRSize() > 0
+                                     ? FuncInfo->getVarArgsGPRIndex()
+                                     : FuncInfo->getVarArgsStackIndex(),
+                                 getPointerTy(DAG.getDataLayout()));
+  const Value *SV = cast<SrcValueSDNode>(Op.getOperand(2))->getValue();
+  return DAG.getStore(Op.getOperand(0), DL, FR, Op.getOperand(1),
+                      MachinePointerInfo(SV));
+}
+
 SDValue AArch64TargetLowering::LowerAAPCS_VASTART(SDValue Op,
                                                 SelectionDAG &DAG) const {
   // The layout of the va_list struct is specified in the AArch64 Procedure Call
   // Standard, section B.3.
   MachineFunction &MF = DAG.getMachineFunction();
   AArch64FunctionInfo *FuncInfo = MF.getInfo<AArch64FunctionInfo>();
   auto PtrVT = getPointerTy(DAG.getDataLayout());
   SDLoc DL(Op);
MemOps.push_back(DAG.getStore(
       Chain, DL, DAG.getConstant(-FPRSize, DL, MVT::i32), VROffsAddr,
       MachinePointerInfo(SV, 28), /* Alignment = */ 4));
 
   return DAG.getNode(ISD::TokenFactor, DL, MVT::Other, MemOps);
 }
 
 SDValue AArch64TargetLowering::LowerVASTART(SDValue Op,
                                             SelectionDAG &DAG) const {
-  return Subtarget->isTargetDarwin() ? LowerDarwin_VASTART(Op, DAG)
-                                     : LowerAAPCS_VASTART(Op, DAG);
+  if (Subtarget->isTargetWindows())
+    return LowerWin64_VASTART(Op, DAG);
+  else if (Subtarget->isTargetDarwin())
+    return LowerDarwin_VASTART(Op, DAG);
+  else
+    return LowerAAPCS_VASTART(Op, DAG);
 }
 
 SDValue AArch64TargetLowering::LowerVACOPY(SDValue Op,
                                            SelectionDAG &DAG) const {
   // AAPCS has three pointers and two ints (= 32 bytes), Darwin has single
   // pointer.
   SDLoc DL(Op);
-  unsigned VaListSize = Subtarget->isTargetDarwin() ? 8 : 32;
+  unsigned VaListSize =
+      Subtarget->isTargetDarwin() || Subtarget->isTargetWindows() ? 8 : 32;
   const Value *DestSV = cast<SrcValueSDNode>(Op.getOperand(3))->getValue();
   const Value *SrcSV = cast<SrcValueSDNode>(Op.getOperand(4))->getValue();
 
   return DAG.getMemcpy(Op.getOperand(0), DL, Op.getOperand(1),
                        Op.getOperand(2),
                        DAG.getConstant(VaListSize, DL, MVT::i32),
                        8, false, false, false, MachinePointerInfo(DestSV),
                        MachinePointerInfo(SrcSV));
bool AArch64TargetLowering::isIntDivCheap(EVT VT, AttributeList Attr) const {
   // sequence can be performed in vector form.
   bool OptSize =
       Attr.hasAttribute(AttributeList::FunctionIndex, Attribute::MinSize);
   return OptSize && !VT.isVector();
 }
 
 unsigned
 AArch64TargetLowering::getVaListSizeInBits(const DataLayout &DL) const {
-  if (Subtarget->isTargetDarwin())
+  if (Subtarget->isTargetDarwin() || Subtarget->isTargetWindows())
     return getPointerTy(DL).getSizeInBits();
 
   return 3 * getPointerTy(DL).getSizeInBits() + 2 * 32;
 }

llvm/trunk/test/CodeGen/AArch64/win64_vararg.ll

+; RUN: llc < %s -mtriple=aarch64-pc-win32 | FileCheck %s
+
+define void @pass_va(i32 %count, ...) nounwind {
+entry:
+; CHECK: sub     sp, 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: bl      other_func
+; CHECK: ldr     x30, [sp], #80
+; CHECK: ret
+  %ap = alloca i8*, align 8
+  %ap1 = bitcast i8** %ap to i8*
+  call void @llvm.va_start(i8* %ap1)
+  %ap2 = load i8*, i8** %ap, align 8
+  call void @other_func(i8* %ap2)
+  ret void
+}
+
+declare void @other_func(i8*) local_unnamed_addr
+
+declare void @llvm.va_start(i8*) nounwind
+declare void @llvm.va_copy(i8*, i8*) nounwind
+
+; CHECK-LABEL: f9:
+; CHECK: sub     sp, sp, #16
+; CHECK: add     x8, sp, #24
+; CHECK: add     x0, sp, #24
+; CHECK: str     x8, [sp, #8]
+; CHECK: add     sp, sp, #16
+; CHECK: ret
+define i8* @f9(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64 %a6, i64 %a7, i64 %a8, ...) nounwind {
+entry:
+  %ap = alloca i8*, align 8
+  %ap1 = bitcast i8** %ap to i8*
+  call void @llvm.va_start(i8* %ap1)
+  %ap2 = load i8*, i8** %ap, align 8
+  ret i8* %ap2
+}
+
+; CHECK-LABEL: f8:
+; CHECK: sub     sp, sp, #16
+; CHECK: add     x8, sp, #16
+; CHECK: add     x0, sp, #16
+; CHECK: str     x8, [sp, #8]
+; CHECK: add     sp, sp, #16
+; CHECK: ret
+define i8* @f8(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64 %a6, i64 %a7, ...) nounwind {
+entry:
+  %ap = alloca i8*, align 8
+  %ap1 = bitcast i8** %ap to i8*
+  call void @llvm.va_start(i8* %ap1)
+  %ap2 = load i8*, i8** %ap, align 8
+  ret i8* %ap2
+}
+
+; CHECK-LABEL: f7:
+; CHECK: sub     sp, sp, #16
+; CHECK: add     x8, sp, #8
+; CHECK: add     x0, sp, #8
+; CHECK: stp     x8, x7, [sp], #16
+; CHECK: ret
+define i8* @f7(i64 %a0, i64 %a1, i64 %a2, i64 %a3, i64 %a4, i64 %a5, i64 %a6, ...) nounwind {
+entry:
+  %ap = alloca i8*, align 8
+  %ap1 = bitcast i8** %ap to i8*
+  call void @llvm.va_start(i8* %ap1)
+  %ap2 = load i8*, i8** %ap, align 8
+  ret i8* %ap2
+}
+
+; CHECK-LABEL: copy1:
+; CHECK: sub     sp, sp, #80
+; CHECK: add     x8, sp, #24
+; CHECK: stp     x6, x7, [sp, #64]
+; CHECK: stp     x4, x5, [sp, #48]
+; CHECK: stp     x2, x3, [sp, #32]
+; CHECK: stp     x8, x1, [sp, #16]
+; CHECK: str     x8, [sp, #8]
+; CHECK: add     sp, sp, #80
+; CHECK: ret
+define void @copy1(i64 %a0, ...) nounwind {
+entry:
+  %ap = alloca i8*, align 8
+  %cp = alloca i8*, align 8
+  %ap1 = bitcast i8** %ap to i8*
+  %cp1 = bitcast i8** %cp to i8*
+  call void @llvm.va_start(i8* %ap1)
+  call void @llvm.va_copy(i8* %cp1, i8* %ap1)
+  ret void
+}