[PATCH] D25590: Add initial support for WebAssembly binary format

Derek Schuff via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 28 15:00:34 PDT 2016


dschuff added inline comments.


================
Comment at: lib/Object/WasmObjectFile.cpp:440
+    ECase(DATA);
+    default:
+      return object_error::invalid_file_type;
----------------
`#undef ECase`?


================
Comment at: test/tools/llvm-readobj/file-headers.test:374
+WASM: Format: WASM
+WASM: Arch: wasm32
+WASM: AddressSize: 32bit
----------------
Could be `WASM-NEXT:`


================
Comment at: test/tools/llvm-readobj/sections.test:497
+WASM: Sections [
+WASM:   Section {
+WASM:     Type: TYPE (0x1)
----------------
also `WASM-NEXT`


================
Comment at: tools/llvm-readobj/WasmDumper.cpp:62
+  }
+  void printRelocations() override { assert(0); }
+  void printSymbols() override { assert(0); }
----------------
`llvm_unreachable()`


================
Comment at: tools/obj2yaml/wasm2yaml.cpp:40
+      case wasm::WASM_SEC_TYPE: {
+        WasmYAML::TypeSection* TypeSec = new WasmYAML::TypeSection;
+        for (const auto &FunctionSig : Obj.types()) {
----------------
these `new foo` lines could be use `auto*`


================
Comment at: tools/yaml2obj/yaml2wasm.cpp:43
+
+static int writeUint64(raw_ostream &OS, uint64_t Value) {
+  char Data[sizeof(Value)];
----------------
could writeUint{32,64} be a template?


================
Comment at: tools/yaml2obj/yaml2wasm.cpp:45
+  char Data[sizeof(Value)];
+  if (!sys::IsLittleEndianHost)
+    sys::swapByteOrder(Value);
----------------
could you just use `endian::byte_swap<uint64_t, little>(Value)`?


https://reviews.llvm.org/D25590





More information about the llvm-commits mailing list