[PATCH] D12461: WebAssembly: generate load/store

[JF Bastien via llvm-commits]

jfb added inline comments.

================
Comment at: lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:103
@@ -91,1 +102,3 @@
+SmallString<32>
+WebAssemblyAsmPrinter::OpcodeName(const WebAssemblyInstrInfo *TII,
                                   const MachineInstr *MI) {
----------------
sunfish wrote:
> Is there a reason why OpcodeName has to be a member function?
Name lookup was sad because I have two `toString` methods, but I changed the `Type` version to take in `hasAddr64` and moved everything back to `static`, so it's all good.

================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrMemory.td:33
@@ +32,3 @@
+// Basic load.
+def LOAD_I32_ : I<(outs Int32:$dst), (ins Int32:$addr),
+                  [(set Int32:$dst, (load Int32:$addr))]>;
----------------
sunfish wrote:
> Nit: Why the trailing underscore in the name?
I'm playing tricks with the name printing: it truncates out the last underscore and everything after it, so this prints as `load_i32`. We can come up with another scheme (maybe pass in a string), but this was pretty simple to start with.

================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrMemory.td:62
@@ +61,3 @@
+def LOAD_ZX_I32_I64_ : I<(outs Int64:$dst), (ins Int32:$addr),
+                         [(set Int64:$dst, (zextloadi32 Int32:$addr))]>;
+
----------------
sunfish wrote:
> I'm ok with expanding things out like this, but several other things in the WebAssembly tablegen files are aggressively factored out into multiclasses. Is there a reason these aren't multiclasses too?
Maybe I'm lacking imagination: InstrFormats-style multiclasses would only be used once each here? There's one load per type, and the `#_I32` concatenation has to be textual IIUC. Similarly for extending loads? Or maybe tablegen has extra tricks I don't know :-)

I was thinking that `HasAddr64` should be done with a multiclass for `Ptr:$addr` and `require`, but I figure this should be a separate patch.


http://reviews.llvm.org/D12461