[PATCH] D26722: [WebAssembly] Add skeleton MC support for the Wasm container format

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

Just a couple of drive by comments.



================
Comment at: lib/MC/WasmObjectWriter.cpp:114
+                                                const MCAsmLayout &Layout) {
+  // Section symbols are used as definitions for undefined symbols with matching
+  // names. If there are multiple sections with the same name, the first one is
----------------
It looks like this code was just copied from the ELF object writer. Do you need section symbol support in the wasm object format? Section symbols cause bugs such as http://lists.llvm.org/pipermail/llvm-dev/2016-March/097064.html so you might want to consider not supporting them.


================
Comment at: lib/MC/WasmObjectWriter.cpp:130
+
+  // The presence of symbol versions causes undefined symbols and
+  // versions declared with @@@ to be renamed.
----------------
Same here: is symbol versioning a necessary feature of the wasm object format?


Repository:
  rL LLVM

https://reviews.llvm.org/D26722