[PATCH] D15555: [WebAssembly] Experimental ELF writer support

[JF Bastien via llvm-commits]

jfb added a comment.

A few minor comments, looks good overall since it'll allow us to try out ELF linking (instead of the hackier bitcode linking).


================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyAsmBackend.cpp:51
@@ +50,3 @@
+
+  unsigned getNumFixupKinds() const override { return 1; }
+
----------------
Do like the other architectures, and `return WebAssembly::Fixups::NumTargetFixupKinds` from `WebAssemblyFixupKinds.h`? e.g. AMDGPU has `fixup_si_rodata`.

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyELFObjectWriter.cpp:36
@@ +35,3 @@
+
+// FIXME: EM_NONE as a temporary hack.
+WebAssemblyELFObjectWriter::WebAssemblyELFObjectWriter(bool Is64Bit,
----------------
Looks like generic-abi at googlegroups.com is the place to contact. Maybe leave it in the FIXME, and we'll email when we have a better idea of what we're going for?

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp:42
@@ +41,3 @@
+
+  ~WebAssemblyMCCodeEmitter() {}
+
----------------
`override`

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp:44
@@ +43,3 @@
+
+  // getBinaryCodeForInstr - TableGen'erated function for getting the
+  // binary encoding for an instruction.
----------------
New style drops the function prefix in the comment? (here and below)

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp:50
@@ +49,3 @@
+
+  // getMachineOpValue - Return binary encoding of operand. If the machin
+  // operand requires relocation, record the relocation and return zero.
----------------
Typo "machine"


Repository:
  rL LLVM

http://reviews.llvm.org/D15555