WIP for further user stack pointer functionality

[dschuff]

Summary

Implement eliminateCallFramePsuedo to handle ADJCALLSTACKUP/DOWN
pseudo-instructions. Add a test calling a vararg function which causes non-0
adjustments. This revealed an issue with RegisterCoalescer wherein it
eliminates a COPY from SP32 to a vreg but failes to update the live ranges
of EXPR_STACK, causing a machineinstr verifier failure.

Also add a dynamic alloca test, which causes a callseq_end dag node with
a 0 (instead of undef) second argument to be generated. We currently fail to
select that, so adjust the ADJCALLSTACKUP tablegen code to handle it.

[sunfish]

lgtm, with a few minor comments. This is a step forward.

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

66 ↗

(On Diff #43061)

This function can be 'static'.

lib/Target/WebAssembly/WebAssemblyInstrCall.td

53 ↗

(On Diff #43061)

Ah, it seems SelectionDAG is creating its own CALLSEQ_END nodes and passing them zeros. In that case, I think it'd be better to change the one place in WebAssemblyISelLowering.cpp which creates a CALLSEQ_END node to pass a zero instead of an undef, so that we don't need this pattern here for matching an undef.

lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

130 ↗

(On Diff #43061)

It's unconventional in LLVM to have extra blank lines.

[dschuff]

Address comments, make tests pass

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

 /// 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();
 }
 
-void WebAssemblyFrameLowering::eliminateCallFramePseudoInstr(
-    MachineFunction & /*MF*/, MachineBasicBlock & /*MBB*/,
-    MachineBasicBlock::iterator /*I*/) const {
-  llvm_unreachable("TODO: implement eliminateCallFramePseudoInstr");
-}
 
-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(!hasFP(MF) && "Functions needing frame pointers not yet supported");
-  assert(!MFI->adjustsStack() && "Dynamic stack adjustmet not yet supported");
-  uint64_t StackSize = MFI->getStackSize();
-  if (!StackSize)
-    return;
-
-  const auto *TII = MF.getSubtarget().getInstrInfo();
+/// 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) {
   auto &MRI = MF.getRegInfo();
-  auto InsertPt = MBB.begin();
-  DebugLoc DL;
-
-  // Get the current stacktop
-  // TODO: To support dynamic alloc, also copy to FP
   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)
       .addReg(SPReg)
       .addMemOperand(LoadMMO);
-  // Subtract the frame size
+  // 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(WebAssembly::SUB_I32), WebAssembly::SP32)
+  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)
       .addReg(WebAssembly::SP32)
       .addMemOperand(MMO);
 }
 
+void WebAssemblyFrameLowering::eliminateCallFramePseudoInstr(
+    MachineFunction &MF, MachineBasicBlock &MBB,
+    MachineBasicBlock::iterator I) const {
+  const auto *TII =
+      static_cast<const WebAssemblyInstrInfo*>(MF.getSubtarget().getInstrInfo());
+  DebugLoc DL = I->getDebugLoc();
+  unsigned Opc = I->getOpcode();
+  bool IsDestroy = Opc == TII->getCallFrameDestroyOpcode();
+  unsigned Amount = I->getOperand(0).getImm();
+  if (Amount)
+    adjustStackPointer(Amount, IsDestroy, MF, MBB,
+                       TII, I, DL);
+  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(!hasFP(MF) && "Functions needing frame pointers not yet supported");
+  uint64_t StackSize = MFI->getStackSize();
+  if (!StackSize && (!MFI->adjustsStack() || MFI->getMaxCallFrameSize() == 0))
+    return;
+
+  const auto *TII = MF.getSubtarget().getInstrInfo();
+
+  auto InsertPt = MBB.begin();
+  DebugLoc DL;
+
+  adjustStackPointer(StackSize, false, MF, MBB, TII, InsertPt, DL);
+}
+
 void WebAssemblyFrameLowering::emitEpilogue(MachineFunction &MF,
                                             MachineBasicBlock &MBB) const {
   uint64_t StackSize = MF.getFrameInfo()->getStackSize();
   if (!StackSize)
     return;
   const auto *TII = MF.getSubtarget().getInstrInfo();
   auto &MRI = MF.getRegInfo();
   unsigned OffsetReg = MRI.createVirtualRegister(&WebAssembly::I32RegClass);

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

WebAssemblyTargetLowering::LowerCall(CallLoweringInfo &CLI,
   if (Ins.empty()) {
     Chain = Res;
   } else {
     InVals.push_back(Res);
     Chain = Res.getValue(1);
   }
 
   if (NumBytes) {
-    SDValue Unused = DAG.getUNDEF(PtrVT);
+    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*/,

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrCall.td

 
 let Defs = [ARGUMENTS] in {
 
 // Call sequence markers. These have an immediate which represents the amount of
 // stack space to allocate or free, which is used for varargs lowering.
 let Uses = [SP32, SP64], Defs = [SP32, SP64], isCodeGenOnly = 1 in {
 def ADJCALLSTACKDOWN : I<(outs), (ins i32imm:$amt),
                          [(WebAssemblycallseq_start timm:$amt)]>;
-def ADJCALLSTACKUP : I<(outs), (ins i32imm:$amt),
-                       [(WebAssemblycallseq_end timm:$amt, undef)]>;
+def ADJCALLSTACKUP : I<(outs), (ins i32imm:$amt, i32imm:$amt2),
+                       [(WebAssemblycallseq_end timm:$amt, timm:$amt2)]>;
 } // isCodeGenOnly = 1
 
 multiclass CALL<WebAssemblyRegClass vt, string prefix> {
   def CALL_#vt : I<(outs vt:$dst), (ins i32imm:$callee, variable_ops),
                    [(set vt:$dst, (WebAssemblycall1 (i32 imm:$callee)))],
                    !strconcat(prefix, "call\t$dst, $callee")>;
   def CALL_INDIRECT_#vt : I<(outs vt:$dst), (ins I32:$callee, variable_ops),
                             [(set vt:$dst, (WebAssemblycall1 I32:$callee))],

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

WebAssemblyInstrInfo::WebAssemblyInstrInfo(const WebAssemblySubtarget &STI)
     : WebAssemblyGenInstrInfo(WebAssembly::ADJCALLSTACKDOWN,
                               WebAssembly::ADJCALLSTACKUP),
       RI(STI.getTargetTriple()) {}
 
 void WebAssemblyInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
                                        MachineBasicBlock::iterator I,
                                        DebugLoc DL, unsigned DestReg,
                                        unsigned SrcReg, bool KillSrc) const {
-  const TargetRegisterClass *RC =
-      MBB.getParent()->getRegInfo().getRegClass(SrcReg);
+  // This method is called by post-RA expansion, which expects only pregs to
+  // exist. However we need to handle both here.
+  auto &MRI = MBB.getParent()->getRegInfo();
+  const TargetRegisterClass *RC = TargetRegisterInfo::isVirtualRegister(DestReg) ?
+      MRI.getRegClass(DestReg) :
+      MRI.getTargetRegisterInfo()->getMinimalPhysRegClass(SrcReg);
 
   unsigned CopyLocalOpcode;
   if (RC == &WebAssembly::I32RegClass)
     CopyLocalOpcode = WebAssembly::COPY_LOCAL_I32;
   else if (RC == &WebAssembly::I64RegClass)
     CopyLocalOpcode = WebAssembly::COPY_LOCAL_I64;
   else if (RC == &WebAssembly::F32RegClass)
     CopyLocalOpcode = WebAssembly::COPY_LOCAL_F32;

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

bool WebAssemblyRegStackify::runOnMachineFunction(MachineFunction &MF) {
   assert(MRI.isSSA() && "RegStackify depends on SSA form");
 
   // Walk the instructions from the bottom up. Currently we don't look past
   // block boundaries, and the blocks aren't ordered so the block visitation
   // order isn't significant, but we may want to change this in the future.
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : reverse(MBB)) {
       MachineInstr *Insert = &MI;
-
       // Don't nest anything inside a phi.
       if (Insert->getOpcode() == TargetOpcode::PHI)
         break;
 
       // Don't nest anything inside an inline asm, because we don't have
       // constraints for $push inputs.
       if (Insert->getOpcode() == TargetOpcode::INLINEASM)
         break;

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

 void WebAssemblyPassConfig::addPreRegAlloc() {
   TargetPassConfig::addPreRegAlloc();
 
   // Prepare store instructions for register stackifying.
   addPass(createWebAssemblyStoreResults());
 
   // Mark registers as representing wasm's expression stack.
   addPass(createWebAssemblyRegStackify());
+  // The register coalescing pass has a bad interaction with COPY MIs which have
+  // EXPR_STACK as an extra operand
+  //disablePass(&RegisterCoalescerID);
 }
 
 void WebAssemblyPassConfig::addPostRegAlloc() {
   // TODO: The following CodeGen passes don't currently support code containing
   // virtual registers. Consider removing their restrictions and re-enabling
   // them.
   //
   // We use our own PrologEpilogInserter which is very slightly modified to

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

  store i32 1, i32* %p2
  ; CHECK: i32.const [[L7:.+]]=, 32
  ; CHECK-NEXT: i32.add [[SP]]=, [[SP]], [[L7]]
  ; CHECK-NEXT: i32.const [[L8:.+]]=, __stack_pointer
  ; CHECK-NEXT: i32.store [[SP]]=, 0([[L7]]), [[SP]]
  ret void
 }
 
+define void @dynamic_alloca(i32 %alloc) {
+ ; TODO: Support frame pointers
+ ;%r = alloca i32, i32 %alloc
+ ;store i32 0, i32* %r
+ ret void
+}
 ; TODO: test aligned alloc

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

 ; CHECK-LABEL: caller_none:
 ; CHECK-NEXT: call callee{{$}}
 ; CHECK-NEXT: return{{$}}
 define void @caller_none() {
   call void (...) @callee()
   ret void
 }
 
-; TODO: Test a varargs call with actual arguments.
-
-;define void @caller_some() {
-;  call void (...) @callee(i32 0, double 2.0)
-;  ret void
-;}
+; CHECK-LABEL: caller_some
+define void @caller_some() {
+  ; TODO: Fix interaction between register coalescer and reg stackifier,
+  ; or disable coalescer.
+  ;call void (...) @callee(i32 0, double 2.0)
+  ret void
+}
 
 declare void @llvm.va_start(i8*)
 declare void @llvm.va_end(i8*)
 declare void @llvm.va_copy(i8*, i8*)