[PATCH] D15344: [WebAssembly] WIP for SP lowering.

[Derek Schuff via llvm-commits]

dschuff added inline comments.

================
Comment at: lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp:84
@@ +83,3 @@
+  auto *SPSymbol = MF.getFunction()->getParent()
+      ->getNamedGlobal("SP");
+  BuildMI(MBB, InsertPt, DL, TII->get(WebAssembly::CONST_I32), SPReg)
----------------
sunfish wrote:
> Instead of using getNamedGlobal on the Module, which requires a declaration in LLVM IR, we can use MF.createExternalSymbolName (and addExternalSymbol below), which creates an ExternalSymbol, which is a very lightweight thing that doesn't require any up-front setup.
> 
> Also, "SP" is in the C/C++ user namespace. What do you think about "__stack_pointer"?
Done. One consequence is that the MachinePointerInfo no longer has the reference to the stack pointer, but that's probably OK.

================
Comment at: lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp:145
@@ -74,1 +144,3 @@
+      .addMemOperand(MMO);
+
 }
----------------
sunfish wrote:
> This code doesn't yet set up FP; can you assert !hasFP(MF) for now?
> 
> Can you add TODO comments for:
>  - Do ".setMIFlag(MachineInstr::FrameSetup)" on emitted instructions
>  - Emit TargetOpcode::CFI_INSTRUCTION instructions
> 
hasFP() currently includes MFI->hasCalls(), which seems wrong, at least for wasm. I took that out.

Added the former TODO to EmitPrologue and the latter to the top with the other TODOs.




http://reviews.llvm.org/D15344