[PATCH] D58660: [WebAssembly] Add support for data sections in the assembler.

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

Great!



================
Comment at: lib/MC/MCParser/WasmAsmParser.cpp:139
+        { ".text", SectionKind::getText() },
+        { ".rodata", SectionKind::getReadOnly() },
+        // TODO: add more types.
----------------
How about `.data` ?


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:656
+      if (CurrentState != DataSection) {
+        // Start section
+      }
----------------
What will happen now if we hit this case?  Should we assert/unreachable?


================
Comment at: test/MC/WebAssembly/basic-assembly.s:87
+
+	.section	.rodata..L.str,"",@
+.L.str:
----------------
Indentation inconsistent with above


================
Comment at: test/MC/WebAssembly/basic-assembly.s:173
+# CHECK-NEXT:	.int8	72
+# CHECK-NEXT:	.asciz	"ello, World!"
+
----------------
To these end up as two different fragments with in the data section?  Just curious how this is preserved through mc.


================
Comment at: test/MC/WebAssembly/data-section.s:2
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+unimplemented-simd128,+nontrapping-fptoint,+exception-handling < %s | FileCheck %s
+# Check that it converts to .o without errors, but don't check any output:
+# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -mattr=+unimplemented-simd128,+nontrapping-fptoint,+exception-handling < %s | obj2yaml | FileCheck -check-prefix=BIN %s
----------------
Command is wrong. We do check!


================
Comment at: test/MC/WebAssembly/data-section.s:11
+.Ltestlabel:
+    end_function
+
----------------
Consistent indention with directives below?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58660/new/

https://reviews.llvm.org/D58660