[WebAssembly] Put __stack_pointer in the offset field of loads and stores?

[sunfish]

Summary

Currently prologue/epilogue code accesses __stack_pointer like this:

i32.const       $push10=, __stack_pointer
i32.load        $push11=, 0($pop10)

The patch here changes it to this:

i32.const       $push10=, 0
i32.load        $push11=, __stack_pointer($pop10)

It's not obvious which way is better though. Currently regular load isel prefers the second form in general, though that's arbitrary too. Thoughts?

[dschuff]

Would the latter have smaller encoding if we have an opcode table that includes i32.const 0? I suppose i32.const __stack_pointer would also end up in the opcode table, but since 0 would likely still be in there anyway, we'd save one slot. I'll ask Ben when he comes in if V8 would have any appreciable codegen difference either way.

[dschuff]

(I guess that assumes the "advanced" opcode table which encodes whole instructions not just opcodes)

[dschuff]

Ben says he thought the second form would be more efficient, at least when bounds checking is used. Also I think having the prolog/epilog code match the general isel code is probably better anyway (and if we want to change it, change both), so that if engines implement optimizations based on common patterns, they will get everything at once with just one pattern. So maybe we should just land this.

lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp

static void writeSPToMemory(unsigned SrcReg, MachineFunction &MF,
                             MachineBasicBlock::iterator &InsertAddr,
                             MachineBasicBlock::iterator &InsertStore,
                             DebugLoc DL) {
   const char *ES = "__stack_pointer";
   auto *SPSymbol = MF.createExternalSymbolName(ES);
   MachineRegisterInfo &MRI = MF.getRegInfo();
   const TargetRegisterClass *PtrRC =
       MRI.getTargetRegisterInfo()->getPointerRegClass(MF);
-  unsigned SPAddr = MRI.createVirtualRegister(PtrRC);
+  unsigned Zero = MRI.createVirtualRegister(PtrRC);
   unsigned Drop = MRI.createVirtualRegister(PtrRC);
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
 
-  BuildMI(MBB, InsertAddr, DL, TII->get(WebAssembly::CONST_I32), SPAddr)
-      .addExternalSymbol(SPSymbol);
+  BuildMI(MBB, InsertAddr, DL, TII->get(WebAssembly::CONST_I32), Zero)
+      .addImm(0);
   auto *MMO = new MachineMemOperand(MachinePointerInfo(MF.getPSVManager()
                                         .getExternalSymbolCallEntry(ES)),
                                     MachineMemOperand::MOStore, 4, 4);
   BuildMI(MBB, InsertStore, DL, TII->get(WebAssembly::STORE_I32), Drop)
-      .addImm(0)
-      .addReg(SPAddr)
+      .addExternalSymbol(SPSymbol)
+      .addReg(Zero)
       .addImm(2)  // p2align
       .addReg(SrcReg)
       .addMemOperand(MMO);
 }
 
 MachineBasicBlock::iterator
 WebAssemblyFrameLowering::eliminateCallFramePseudoInstr(
     MachineFunction &MF, MachineBasicBlock &MBB,
void WebAssemblyFrameLowering::emitPrologue(MachineFunction &MF,
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
   auto &MRI = MF.getRegInfo();
 
   auto InsertPt = MBB.begin();
   DebugLoc DL;
 
   const TargetRegisterClass *PtrRC =
       MRI.getTargetRegisterInfo()->getPointerRegClass(MF);
-  unsigned SPAddr = MRI.createVirtualRegister(PtrRC);
+  unsigned Zero = MRI.createVirtualRegister(PtrRC);
   unsigned SPReg = MRI.createVirtualRegister(PtrRC);
   const char *ES = "__stack_pointer";
   auto *SPSymbol = MF.createExternalSymbolName(ES);
-  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), SPAddr)
-      .addExternalSymbol(SPSymbol);
+  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), Zero)
+      .addImm(0);
   auto *LoadMMO = new MachineMemOperand(MachinePointerInfo(MF.getPSVManager()
                                             .getExternalSymbolCallEntry(ES)),
                                         MachineMemOperand::MOLoad, 4, 4);
   // Load the SP value.
   BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::LOAD_I32),
           StackSize ? SPReg : (unsigned)WebAssembly::SP32)
-      .addImm(0)       // offset
-      .addReg(SPAddr)  // addr
+      .addExternalSymbol(SPSymbol)
+      .addReg(Zero)    // addr
       .addImm(2)       // p2align
       .addMemOperand(LoadMMO);
 
   if (StackSize) {
     // Subtract the frame size
     unsigned OffsetReg = MRI.createVirtualRegister(PtrRC);
     BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), OffsetReg)
         .addImm(StackSize);