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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: test/MC/WebAssembly/array-fill.ll:18
 ; CHECK-NEXT:     DataSize:        2
-; CHECK-NEXT:     SymbolInfo:      
-; CHECK-NEXT:       - Name:            gBd
+; CHECK-NEXT:     SymbolInfo:
+; CHECK-NEXT:       - Index:           0
----------------
Shouldn't this be SymbolTable now?


================
Comment at: test/MC/WebAssembly/array-fill.ll:21
+; CHECK-NEXT:         Name:            gBd
+; CHECK-NEXT:         Kind:            DATA
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
----------------
Lets put "Kind" before "Name". i.e. common fields coming first makes sense to me.


================
Comment at: test/MC/WebAssembly/array-fill.ll:25
+; CHECK-NEXT:         DataOffset:      0
+; CHECK-NEXT:         DataSize:        2
 ; CHECK-NEXT:     SegmentInfo:    
----------------
Just `Offset` and `Size` make sense to me.   If you want to be specific then maybe `SegmentOffset`


================
Comment at: test/MC/WebAssembly/unnamed-data.ll:67
+; CHECK-NEXT:         Flags:           [  ]
+; CHECK-NEXT:         SegmentIndex:    2
+; CHECK-NEXT:         DataOffset:      0
----------------
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?


================
Comment at: test/ObjectYAML/wasm/linking_section.yaml:58
+# CHECK-NEXT:        Kind:            FUNCTION
+# CHECK-NEXT:        SymbolIndex:     0
 # CHECK-NEXT:        Flags:           [ BINDING_WEAK ]
----------------
Strange, some tests seem to use use "Index" and others use "SymbolIndex"  are some not up-to-date?


================
Comment at: tools/yaml2obj/yaml2wasm.cpp:150
+    uint32_t SymbolIndex = 0;
+    (void)SymbolIndex;
     for (const WasmYAML::SymbolInfo &Info : Section.SymbolInfos) {
----------------
Wrapping in `ifndef _NDEBUG` is more explicit I think.


Repository:
  rL LLVM

https://reviews.llvm.org/D42495