[WebAssembly] Switch varags calling convention to use a register

[dschuff]

Summary

Instead of passing varargs directly on the user stack, allocate a buffer in
the caller's stack frame and pass a pointer to it. This simplifies the C
ABI (e.g. non-C callers of C functions do not need to use C's user stack if
they have their own mechanism) and allows further optimizations in the future
(e.g. fewer functions may need to use the stack).

[dschuff]

A couple of questions:

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

149 ↗

(On Diff #47381)

I don't really understand what this assert is for. Is it OK to remove it? Should I put in something else?

lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

621 ↗

(On Diff #47381)

One issue I currently have is that if ARGUMENTS is not live-in to the current block, then I get machineinstr verifier error because adding this reference does not add ARGUMENTS to the live-ins for the block. I could add it manually here, but I don't see a good way to get the current MBB from the current SelectionDAG, so maybe that's not the right thing to do. Any ideas?

[sunfish]

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

149 ↗

(On Diff #47381)

There are two categories of instructions with variadic arguments: calls and tableswitch. Call arguments are all registers from LLVM's perspective (if they are constants, they get loaded into "registers" with i32.const etc.). Tableswitch arguments are all immediates. We use the WebAssemblyII::VariableOpIsImmediate flag to indicate to naive code which kind of operand to expect in the variadic portion of an instruction. This assert is just asserting that if that flag is set, and we're in the variadic portion, that we are actually seeing an immediate. This assert is relevant, so we shouldn't remove it.

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

51 ↗

(On Diff #47381)

Why did MFI->isFrameAddressTaken() go away here?

lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

621 ↗

(On Diff #47381)

In the entry block, when we're creating the ARGUMENTs for the fixed arguments, we should create the ARGUMENT for the vararg pointer too, and remember which virtual register we put it in. LowerVASTART should then be able to just read this register, without needing ARGUMENTS live to wherever it happens to be.

[jfb]

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

67 ↗

(On Diff #47381)

&& "what is this checking?"

lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

364 ↗

(On Diff #47381)

/*Alignment=*/16

411 ↗

(On Diff #47381)

This looks like an unfinished

test/CodeGen/WebAssembly/varargs.ll

141 ↗

(On Diff #47381)

?

[dschuff]

• Address review comments

[dschuff]

I still want to add more testing but here's round 2 addressing the comments, and fixing the problem I mentioned with LowerVASTART above.

lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp

149 ↗

(On Diff #47381)

Turns out this was only relevant for a previous version of this change and isn't necessary after I fixed a bug; I put the assert back.

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

51 ↗

(On Diff #47381)

Because I added an assert on line 77 that the frame address is never taken. This change means that arguments are never passed on the stack, and even if there's a frame pointer or (I think?) stack realignment, the frame address still shouldn't be taken. (Unless there's use of e.g. __builtin_frameaddress or something? but maybe we don't want to support that anyway?)

lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

411 ↗

(On Diff #47381)

Moved the line with the rest of the output arg code and updated comment.

test/CodeGen/WebAssembly/varargs.ll

141 ↗

(On Diff #47381)

This was an attempt to tickle the bug I mentioned above, and wasn't meant to be checked in. Another test forthcoming.

[dschuff]

• Update basic block test

[dschuff]

test/CodeGen/WebAssembly/varargs.ll

141 ↗

(On Diff #47481)

Test is now fixed and checks the right thing.

[jfb]

lgtm

[sunfish]

Looks good!

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

51 ↗

(On Diff #47481)

I think we should support __builtin_frame_address(0), but if you want to remove the MFI->isFrameAddressTaken() check for now, it'd be good to add an assert right here, in this function, so that if we ever set it we can be quickly pointed to the place where it needs to be handled.

lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

364 ↗

(On Diff #47481)

Instead of hard-coding a 16 here, let's use the DataLayout's getStackAlignment().

[dschuff]

addressed comments in rL260429

llvm/trunk/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

 // TODO: Emit TargetOpcode::CFI_INSTRUCTION instructions
 
 /// Return true if the specified function should have a dedicated frame pointer
 /// register.
 bool WebAssemblyFrameLowering::hasFP(const MachineFunction &MF) const {
   const MachineFrameInfo *MFI = MF.getFrameInfo();
   const auto *RegInfo =
       MF.getSubtarget<WebAssemblySubtarget>().getRegisterInfo();
-  return MFI->hasVarSizedObjects() || MFI->isFrameAddressTaken() ||
-         MFI->hasStackMap() || MFI->hasPatchPoint() ||
-         RegInfo->needsStackRealignment(MF);
+  return MFI->hasVarSizedObjects() || MFI->hasStackMap() ||
+         MFI->hasPatchPoint() || RegInfo->needsStackRealignment(MF);
 }
 
 /// Under normal circumstances, when a frame pointer is not required, we reserve
 /// argument space for call sites in the function immediately on entry to the
 /// current function. This eliminates the need for add/sub sp brackets around
 /// call sites. Returns true if the call frame is included as part of the stack
 /// frame.
 bool WebAssemblyFrameLowering::hasReservedCallFrame(
     const MachineFunction &MF) const {
   return !MF.getFrameInfo()->hasVarSizedObjects();
 }
 
-
-/// Adjust the stack pointer by a constant amount.
-static void adjustStackPointer(unsigned StackSize,
-                               bool AdjustUp,
-                               MachineFunction& MF,
-                               MachineBasicBlock& MBB,
-                               const TargetInstrInfo* TII,
-                               MachineBasicBlock::iterator InsertPt,
-                               const DebugLoc& DL) {
-  assert((StackSize || !AdjustUp) && "Adjusting up by 0");
-  auto &MRI = MF.getRegInfo();
-  unsigned SPReg = MRI.createVirtualRegister(&WebAssembly::I32RegClass);
-  auto *SPSymbol = MF.createExternalSymbolName("__stack_pointer");
-  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), SPReg)
-      .addExternalSymbol(SPSymbol);
-  // This MachinePointerInfo should reference __stack_pointer as well but
-  // doesn't because MachinePointerInfo() takes a GV which we don't have for
-  // __stack_pointer. TODO: check if PseudoSourceValue::ExternalSymbolCallEntry
-  // is appropriate instead. (likewise for EmitEpologue below)
-  auto *LoadMMO = new MachineMemOperand(MachinePointerInfo(),
-                                        MachineMemOperand::MOLoad, 4, 4);
-  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::LOAD_I32), SPReg)
-      .addImm(0) // offset
-      .addReg(SPReg) // addr
-      .addImm(2) // p2align
-      .addMemOperand(LoadMMO);
-  // Add/Subtract the frame size
-  unsigned OffsetReg = MRI.createVirtualRegister(&WebAssembly::I32RegClass);
-  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), OffsetReg)
-      .addImm(StackSize);
-  BuildMI(MBB, InsertPt, DL,
-          TII->get(AdjustUp ? WebAssembly::ADD_I32 : WebAssembly::SUB_I32),
-          WebAssembly::SP32)
-      .addReg(SPReg)
-      .addReg(OffsetReg);
-  // The SP32 register now has the new stacktop. Also write it back to memory.
-  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), OffsetReg)
-      .addExternalSymbol(SPSymbol);
-  auto *MMO = new MachineMemOperand(MachinePointerInfo(),
-                                    MachineMemOperand::MOStore, 4, 4);
-  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::STORE_I32), WebAssembly::SP32)
-      .addImm(0)
-      .addReg(OffsetReg)
-      .addImm(2) // p2align
-      .addReg(WebAssembly::SP32)
-      .addMemOperand(MMO);
-}
-
 void WebAssemblyFrameLowering::eliminateCallFramePseudoInstr(
     MachineFunction &MF, MachineBasicBlock &MBB,
     MachineBasicBlock::iterator I) const {
-  const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
-  DebugLoc DL = I->getDebugLoc();
-  unsigned Opc = I->getOpcode();
-  bool IsDestroy = Opc == TII->getCallFrameDestroyOpcode();
-  unsigned Amount = I->getOperand(0).getImm();
-  // TODO(dschuff): After we switch varargs to passing an explicit pointer
-  // rather than using an implicit call frame, assert here that Amount is 0
-  // and remove adjustStackPointer altogether.
-  if (Amount)
-    adjustStackPointer(Amount, IsDestroy, MF, MBB,
-                       TII, I, DL);
+  // TODO: can we avoid using call frame pseudos altogether?
+  assert(!I->getOperand(0).getImm() &&
+         "Stack should not be adjusted around calls");
   MBB.erase(I);
 }
 
 void WebAssemblyFrameLowering::emitPrologue(MachineFunction &MF,
                                             MachineBasicBlock &MBB) const {
   // TODO: Do ".setMIFlag(MachineInstr::FrameSetup)" on emitted instructions
   auto *MFI = MF.getFrameInfo();
   assert(MFI->getCalleeSavedInfo().empty() &&
          "WebAssembly should not have callee-saved registers");
+  assert(!MFI->isFrameAddressTaken());
 
   uint64_t StackSize = MFI->getStackSize();
-  if (!StackSize && !MFI->adjustsStack())
-    return;
+  if (!StackSize && !MFI->adjustsStack() && !hasFP(MF)) return;
 
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
   auto &MRI = MF.getRegInfo();
 
   auto InsertPt = MBB.begin();
   DebugLoc DL;
 
   unsigned SPReg = MRI.createVirtualRegister(&WebAssembly::I32RegClass);
BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::STORE_I32), WebAssembly::SP32)
       .addMemOperand(MMO);
   }
 }
 
 void WebAssemblyFrameLowering::emitEpilogue(MachineFunction &MF,
                                             MachineBasicBlock &MBB) const {
   auto *MFI = MF.getFrameInfo();
   uint64_t StackSize = MFI->getStackSize();
-  if (!StackSize && !MFI->adjustsStack())
-    return;
+  if (!StackSize && !MFI->adjustsStack() && !hasFP(MF)) return;
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
   auto &MRI = MF.getRegInfo();
   unsigned OffsetReg = MRI.createVirtualRegister(&WebAssembly::I32RegClass);
   auto InsertPt = MBB.getFirstTerminator();
   DebugLoc DL;
 
   if (InsertPt != MBB.end()) {
     DL = InsertPt->getDebugLoc();

llvm/trunk/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

WebAssemblyTargetLowering::LowerCall(CallLoweringInfo &CLI,
   unsigned NumFixedArgs = CLI.NumFixedArgs;
   auto PtrVT = getPointerTy(MF.getDataLayout());
 
   // Analyze operands of the call, assigning locations to each operand.
   SmallVector<CCValAssign, 16> ArgLocs;
   CCState CCInfo(CallConv, IsVarArg, MF, ArgLocs, *DAG.getContext());
 
   if (IsVarArg) {
-    // Outgoing non-fixed arguments are placed at the top of the stack. First
-    // compute their offsets and the total amount of argument stack space
-    // needed.
+    // Outgoing non-fixed arguments are placed in a buffer. First
+    // compute their offsets and the total amount of buffer space needed.
     for (SDValue Arg :
          make_range(OutVals.begin() + NumFixedArgs, OutVals.end())) {
       EVT VT = Arg.getValueType();
       assert(VT != MVT::iPTR && "Legalized args should be concrete");
       Type *Ty = VT.getTypeForEVT(*DAG.getContext());
       unsigned Offset =
           CCInfo.AllocateStack(MF.getDataLayout().getTypeAllocSize(Ty),
                                MF.getDataLayout().getABITypeAlignment(Ty));
       CCInfo.addLoc(CCValAssign::getMem(ArgLocs.size(), VT.getSimpleVT(),
                                         Offset, VT.getSimpleVT(),
                                         CCValAssign::Full));
     }
   }
 
   unsigned NumBytes = CCInfo.getAlignedCallFrameSize();
 
-  SDValue NB;
-  if (NumBytes) {
-    NB = DAG.getConstant(NumBytes, DL, PtrVT, true);
-    Chain = DAG.getCALLSEQ_START(Chain, NB, DL);
-  }
-
-  if (IsVarArg) {
+  SDValue FINode;
+  if (IsVarArg && NumBytes) {
     // For non-fixed arguments, next emit stores to store the argument values
-    // to the stack at the offsets computed above.
-    SDValue SP = DAG.getCopyFromReg(
-        Chain, DL, getStackPointerRegisterToSaveRestore(), PtrVT);
+    // to the stack buffer at the offsets computed above.
+    int FI = MF.getFrameInfo()->CreateStackObject(NumBytes, /*Alignment=*/16,
+                                                  /*isSS=*/false);
     unsigned ValNo = 0;
     SmallVector<SDValue, 8> Chains;
     for (SDValue Arg :
          make_range(OutVals.begin() + NumFixedArgs, OutVals.end())) {
       assert(ArgLocs[ValNo].getValNo() == ValNo &&
              "ArgLocs should remain in order and only hold varargs args");
       unsigned Offset = ArgLocs[ValNo++].getLocMemOffset();
-      SDValue Add = DAG.getNode(ISD::ADD, DL, PtrVT, SP,
+      FINode = DAG.getFrameIndex(FI, getPointerTy(DAG.getDataLayout()));
+      SDValue Add = DAG.getNode(ISD::ADD, DL, PtrVT, FINode,
                                 DAG.getConstant(Offset, DL, PtrVT));
-      Chains.push_back(DAG.getStore(Chain, DL, Arg, Add,
-                                    MachinePointerInfo::getStack(MF, Offset),
-                                    false, false, 0));
+      Chains.push_back(DAG.getStore(
+          Chain, DL, Arg, Add,
+          MachinePointerInfo::getFixedStack(MF, FI, Offset), false, false, 0));
     }
     if (!Chains.empty())
       Chain = DAG.getNode(ISD::TokenFactor, DL, MVT::Other, Chains);
+  } else if (IsVarArg) {
+    FINode = DAG.getIntPtrConstant(0, DL);
   }
 
   // Compute the operands for the CALLn node.
   SmallVector<SDValue, 16> Ops;
   Ops.push_back(Chain);
   Ops.push_back(Callee);
 
   // Add all fixed arguments. Note that for non-varargs calls, NumFixedArgs
   // isn't reliable.
   Ops.append(OutVals.begin(),
              IsVarArg ? OutVals.begin() + NumFixedArgs : OutVals.end());
+  // Add a pointer to the vararg buffer.
+  if (IsVarArg) Ops.push_back(FINode);
 
-  SmallVector<EVT, 8> Tys;
+  SmallVector<EVT, 8> InTys;
   for (const auto &In : Ins) {
     assert(!In.Flags.isByVal() && "byval is not valid for return values");
     assert(!In.Flags.isNest() && "nest is not valid for return values");
     if (In.Flags.isInAlloca())
       fail(DL, DAG, "WebAssembly hasn't implemented inalloca return values");
     if (In.Flags.isInConsecutiveRegs())
       fail(DL, DAG, "WebAssembly hasn't implemented cons regs return values");
     if (In.Flags.isInConsecutiveRegsLast())
       fail(DL, DAG,
            "WebAssembly hasn't implemented cons regs last return values");
     // Ignore In.getOrigAlign() because all our arguments are passed in
     // registers.
-    Tys.push_back(In.VT);
+    InTys.push_back(In.VT);
   }
-  Tys.push_back(MVT::Other);
-  SDVTList TyList = DAG.getVTList(Tys);
+  InTys.push_back(MVT::Other);
+  SDVTList InTyList = DAG.getVTList(InTys);
   SDValue Res =
       DAG.getNode(Ins.empty() ? WebAssemblyISD::CALL0 : WebAssemblyISD::CALL1,
-                  DL, TyList, Ops);
+                  DL, InTyList, Ops);
   if (Ins.empty()) {
     Chain = Res;
   } else {
     InVals.push_back(Res);
     Chain = Res.getValue(1);
   }
 
-  if (NumBytes) {
-    SDValue Unused = DAG.getTargetConstant(0, DL, PtrVT);
-    Chain = DAG.getCALLSEQ_END(Chain, NB, Unused, SDValue(), DL);
-  }
-
   return Chain;
 }
 
 bool WebAssemblyTargetLowering::CanLowerReturn(
     CallingConv::ID /*CallConv*/, MachineFunction & /*MF*/, bool /*IsVarArg*/,
     const SmallVectorImpl<ISD::OutputArg> &Outs,
     LLVMContext & /*Context*/) const {
   // WebAssembly can't currently handle returning tuples.
for (const ISD::OutputArg &Out : Outs) {
     if (Out.Flags.isInConsecutiveRegsLast())
       fail(DL, DAG, "WebAssembly hasn't implemented cons regs last results");
   }
 
   return Chain;
 }
 
 SDValue WebAssemblyTargetLowering::LowerFormalArguments(
-    SDValue Chain, CallingConv::ID CallConv, bool /*IsVarArg*/,
+    SDValue Chain, CallingConv::ID CallConv, bool IsVarArg,
     const SmallVectorImpl<ISD::InputArg> &Ins, SDLoc DL, SelectionDAG &DAG,
     SmallVectorImpl<SDValue> &InVals) const {
   MachineFunction &MF = DAG.getMachineFunction();
+  auto *MFI = MF.getInfo<WebAssemblyFunctionInfo>();
 
   if (!CallingConvSupported(CallConv))
     fail(DL, DAG, "WebAssembly doesn't support non-C calling conventions");
 
   // Set up the incoming ARGUMENTS value, which serves to represent the liveness
   // of the incoming values before they're represented by virtual registers.
   MF.getRegInfo().addLiveIn(WebAssembly::ARGUMENTS);
 
for (const ISD::InputArg &In : Ins) {
     // registers.
     InVals.push_back(
         In.Used
             ? DAG.getNode(WebAssemblyISD::ARGUMENT, DL, In.VT,
                           DAG.getTargetConstant(InVals.size(), DL, MVT::i32))
             : DAG.getUNDEF(In.VT));
 
     // Record the number and types of arguments.
-    MF.getInfo<WebAssemblyFunctionInfo>()->addParam(In.VT);
+    MFI->addParam(In.VT);
   }
 
-  // Incoming varargs arguments are on the stack and will be accessed through
-  // va_arg, so we don't need to do anything for them here.
+  // Varargs are copied into a buffer allocated by the caller, and a pointer to
+  // the buffer is passed as an argument.
+  if (IsVarArg) {
+    MVT PtrVT = getPointerTy(MF.getDataLayout());
+    unsigned VarargVreg =
+        MF.getRegInfo().createVirtualRegister(getRegClassFor(PtrVT));
+    MFI->setVarargBufferVreg(VarargVreg);
+    Chain = DAG.getCopyToReg(
+        Chain, DL, VarargVreg,
+        DAG.getNode(WebAssemblyISD::ARGUMENT, DL, PtrVT,
+                    DAG.getTargetConstant(Ins.size(), DL, MVT::i32)));
+    MFI->addParam(PtrVT);
+  }
 
   return Chain;
 }
 
 //===----------------------------------------------------------------------===//
 //  Custom lowering hooks.
 //===----------------------------------------------------------------------===//
 
SDValue WebAssemblyTargetLowering::LowerBR_JT(SDValue Op,
   return DAG.getNode(WebAssemblyISD::TABLESWITCH, DL, MVT::Other, Ops);
 }
 
 SDValue WebAssemblyTargetLowering::LowerVASTART(SDValue Op,
                                                 SelectionDAG &DAG) const {
   SDLoc DL(Op);
   EVT PtrVT = getPointerTy(DAG.getMachineFunction().getDataLayout());
 
-  // The incoming non-fixed arguments are placed on the top of the stack, with
-  // natural alignment, at the point of the call, so the base pointer is just
-  // the current frame pointer.
-  DAG.getMachineFunction().getFrameInfo()->setFrameAddressIsTaken(true);
-  unsigned FP =
-      Subtarget->getRegisterInfo()->getFrameRegister(DAG.getMachineFunction());
-  SDValue FrameAddr = DAG.getCopyFromReg(DAG.getEntryNode(), DL, FP, PtrVT);
+  auto *MFI = DAG.getMachineFunction().getInfo<WebAssemblyFunctionInfo>();
   const Value *SV = cast<SrcValueSDNode>(Op.getOperand(2))->getValue();
-  return DAG.getStore(Op.getOperand(0), DL, FrameAddr, Op.getOperand(1),
+
+  SDValue ArgN = DAG.getCopyFromReg(DAG.getEntryNode(), DL,
+                                    MFI->getVarargBufferVreg(), PtrVT);
+  return DAG.getStore(Op.getOperand(0), DL, ArgN, Op.getOperand(1),
                       MachinePointerInfo(SV), false, false, 0);
 }
 
 //===----------------------------------------------------------------------===//
 //                          WebAssembly Optimization Hooks
 //===----------------------------------------------------------------------===//

llvm/trunk/lib/Target/WebAssembly/WebAssemblyMachineFunctionInfo.h

class WebAssemblyFunctionInfo final : public MachineFunctionInfo {
   ///   - single use (per path)
   ///   - single def (per path)
   ///   - defined and used in LIFO order with other stack registers
   BitVector VRegStackified;
 
   // One entry for each possible target reg. we expect it to be small.
   std::vector<unsigned> PhysRegs;
 
+  // A virtual register holding the pointer to the vararg buffer for vararg
+  // functions. It is created and set in TLI::LowerFormalArguments and read by
+  // TLI::LowerVASTART
+  unsigned VarargVreg = -1U;
+
  public:
   explicit WebAssemblyFunctionInfo(MachineFunction &MF) : MF(MF) {
     PhysRegs.resize(WebAssembly::NUM_TARGET_REGS, -1U);
   }
   ~WebAssemblyFunctionInfo() override;
 
   void addParam(MVT VT) { Params.push_back(VT); }
   const std::vector<MVT> &getParams() const { return Params; }
 
+  unsigned getVarargBufferVreg() const {
+    assert(VarargVreg != -1U && "Vararg vreg hasn't been set");
+    return VarargVreg;
+  }
+  void setVarargBufferVreg(unsigned Reg) { VarargVreg = Reg; }
+
   static const unsigned UnusedReg = -1u;
 
   void stackifyVReg(unsigned VReg) {
     if (TargetRegisterInfo::virtReg2Index(VReg) >= VRegStackified.size())
       VRegStackified.resize(TargetRegisterInfo::virtReg2Index(VReg) + 1);
     VRegStackified.set(TargetRegisterInfo::virtReg2Index(VReg));
   }
   void unstackifyVReg(unsigned VReg) {

llvm/trunk/lib/Target/WebAssembly/known_gcc_test_failures.txt

 931004-14.c
 931004-2.c
 931004-4.c
 931004-6.c
 931004-8.c
 
 980709-1.c
 990127-1.c
-991216-2.c
 
 frame-address.c
 loop-15.c
 loop-ivopts-2.c
 pr20466-1.c
 pr28778.c
 pr28982b.c
 pr33870-1.c

llvm/trunk/test/CodeGen/WebAssembly/varargs.ll

 ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs | FileCheck %s
 
 ; Test varargs constructs.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 ; Test va_start.
 
 ; TODO: Test va_start.
-
-;define void @start(i8** %ap, ...) {
-;entry:
-;  %0 = bitcast i8** %ap to i8*
-;  call void @llvm.va_start(i8* %0)
-;  ret void
-;}
+; CHECK-LABEL: start:
+; CHECK-NEXT: .param i32, i32
+; CHECK-NOT: __stack_pointer
+define void @start(i8** %ap, ...) {
+entry:
+  %0 = bitcast i8** %ap to i8*
+; Store the second argument (the hidden vararg buffer pointer) into ap
+; CHECK: i32.store $discard=, 0($0), $1
+  call void @llvm.va_start(i8* %0)
+  ret void
+}
 
 ; Test va_end.
 
 ; CHECK-LABEL: end:
 ; CHECK-NEXT: .param i32{{$}}
 ; CHECK-NEXT: return{{$}}
 define void @end(i8** %ap) {
 entry:
entry:
   ret i128 %t
 }
 
 ; Test a varargs call with no actual arguments.
 
 declare void @callee(...)
 
 ; CHECK-LABEL: caller_none:
-; CHECK-NEXT: call callee@FUNCTION{{$}}
+; CHECK-NEXT: i32.const $push0=, 0
+; CHECK-NEXT: call callee@FUNCTION, $pop0
 ; CHECK-NEXT: return{{$}}
 define void @caller_none() {
   call void (...) @callee()
   ret void
 }
 
 ; Test a varargs call with some actual arguments.
 ; Note that the store of 2.0 is converted to an i64 store; this optimization
 ; is not needed on WebAssembly, but there isn't currently a convenient hook for
 ; disabling it.
 
 ; CHECK-LABEL: caller_some
 ; CHECK: i32.store
 ; CHECK: i64.store
 define void @caller_some() {
   call void (...) @callee(i32 0, double 2.0)
   ret void
 }
 
+; Test a va_start call in a non-entry block
+; CHECK-LABEL: startbb:
+; CHECK: .param i32, i32, i32
+define void @startbb(i1 %cond, i8** %ap, ...) {
+entry:
+  br i1 %cond, label %bb0, label %bb1
+bb0:
+  ret void
+bb1:
+  %0 = bitcast i8** %ap to i8*
+; Store the second argument (the hidden vararg buffer pointer) into ap
+; CHECK: i32.store $discard=, 0($1), $2
+  call void @llvm.va_start(i8* %0)
+  ret void
+}
+
+
 declare void @llvm.va_start(i8*)
 declare void @llvm.va_end(i8*)
 declare void @llvm.va_copy(i8*, i8*)