[PATCH] D42233: [WebAssembly] Better support for WASM Object format

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: lib/MC/WasmObjectWriter.cpp:848
+      writeAlignedDataSegment(Section, Segment);
+    }
+    else
----------------
drop the  the { } ?   (and maybe move the comment out side the `if` if it makes it more readable?) 


================
Comment at: lib/MC/WasmObjectWriter.cpp:867
+
+    raw_null_ostream NullStream;    
+    unsigned MemIndexLen = encodeULEB128(0, NullStream); 
----------------
The indentation here looks funny.  Can you run clang-format on this patch ? 


================
Comment at: lib/Object/WasmObjectFile.cpp:1121
   RelocRef.d.a = Ref.d.a;
   RelocRef.d.b = Sec.Relocations.size();
+
----------------
Can this line move to the `else` of the `if` below?


================
Comment at: test/MC/WebAssembly/bss.ll:72
 ; CHECK-NEXT:         Content:         '00'
-; CHECK-NEXT:       - SectionOffset:  30 
 ; CHECK-NEXT:         MemoryIndex:     0
----------------
Revert this change?


================
Comment at: test/tools/llvm-objdump/wasm.txt:14
 # CHECK-NEXT:  8 DATA          0000001a 0000000000000000 DATA 
-# CHECK-NEXT:  9 name          0000002b 0000000000000000 
-# CHECK-NEXT: 10 reloc.CODE    00000017 0000000000000000 
-# CHECK-NEXT: 11 linking       00000016 0000000000000000 
+# CHECK-NEXT:  9               00000014 0000000000000000 DATA 
+# CHECK-NEXT: 10 name          0000002b 0000000000000000 
----------------
We should probably give this section (actually its the one and only segment, right?) a name.

Also, perhaps we should omit the data section itself from the list, now that its sub-segments are themselves lists?


https://reviews.llvm.org/D42233