[PATCH] D13119: [WebAssembly] Switch to a more traditional assembly syntax

[JF Bastien via llvm-commits]

jfb added a comment.

This looks great. Only a few minor things, otherwise lgtm.


================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:127
@@ +126,3 @@
+
+  return '@' + utostr(TargetRegisterInfo::virtReg2Index(RegNo) + NumArgs);
+}
----------------
Comment that wasm arguments and locals are in the same number space, with args coming first (and there are no vaargs).

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:190
@@ -261,1 +189,3 @@
+
   if (!Rt->isVoidTy() || !F->arg_empty()) {
+    SmallString<128> Str;
----------------
Is this check needed? It seems like the below code does the right thing without it.

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:229
@@ +228,3 @@
+    // TODO: wasm64
+    OS << "i32.const " << toSymbol(MI->getOperand(1).getGlobal()->getName());
+    break;
----------------
Should we just drop GLOBAL since it's not in the design anymore?

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:256
@@ +255,3 @@
+      if (NeedComma)
+        OS << ',';
+      OS << ' ';
----------------
Move `NeedComma = true;` to here?

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:286
@@ +285,3 @@
+  if (NumDefs != 0) {
+    Str.clear();
+    OS << "\t" "set_local "
----------------
Create a new string here instead?

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:289
@@ +288,3 @@
+       << regToString(MI->getOperand(0).getReg()) << ", "
+          "@pop";
+    OutStreamer->EmitRawText(OS.str());
----------------
`@pop` is a bit confusing since it's more of an internal opcode not an identifier (which we use `@` for). Maybe `%pop` or something (since we're already using `$` and `#` looks like a comment)?


Repository:
  rL LLVM

http://reviews.llvm.org/D13119