[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