[PATCH] D42495: [WebAssembly] Add symbol table to LLVM, 2/2

[Nicholas Wilson via Phabricator via llvm-commits]

ncw marked 5 inline comments as done.
ncw added inline comments.


================
Comment at: test/MC/WebAssembly/unnamed-data.ll:67
+; CHECK-NEXT:         Flags:           [  ]
+; CHECK-NEXT:         SegmentIndex:    2
+; CHECK-NEXT:         DataOffset:      0
----------------
sbc100 wrote:
> How about calling this just "Segment".   We could even list the name here if it has one, rather than the index?
> 
> Since we use -fdata-sections by default the Offset will almost alwasys be zero, so perhaps we can elide it when it is zero (save for Size).. to make the output less verbose?
As discussed, printing the segment name is more fiddly than it's worth given yaml2obj has to go the other way.

I've renamed DataSize -> Size, DataOffset -> Offset, and made Offset default to 0 (optional)


================
Comment at: test/ObjectYAML/wasm/linking_section.yaml:58
+# CHECK-NEXT:        Kind:            FUNCTION
+# CHECK-NEXT:        SymbolIndex:     0
 # CHECK-NEXT:        Flags:           [ BINDING_WEAK ]
----------------
sbc100 wrote:
> Strange, some tests seem to use use "Index" and others use "SymbolIndex"  are some not up-to-date?
Oops, just hadn't updated a couple


================
Comment at: tools/yaml2obj/yaml2wasm.cpp:150
+    uint32_t SymbolIndex = 0;
+    (void)SymbolIndex;
     for (const WasmYAML::SymbolInfo &Info : Section.SymbolInfos) {
----------------
sbc100 wrote:
> Wrapping in `ifndef _NDEBUG` is more explicit I think.
Hm, sorry, I forgot to do that. I just try and avoid ifdefs since they're ugly.


Repository:
  rL LLVM

https://reviews.llvm.org/D42495