[PATCH] D57938: [WebAssembly] Update MC for bulk memory

[Wouter van Oortmerssen via Phabricator via llvm-commits]

aardappel added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:673
+      return expect(AsmToken::EndOfStatement, "EOL");
+    }
+
----------------
sbc100 wrote:
> tlively wrote:
> > @aardappel, I wanted to test that this directive would properly round trip through the assembler and disassembler, but I wasn't able to get the assembler to handle a data section properly. Is this something that you would expect to work at the moment?
> There are still issues with roundtriping in its full generality.   Open any bug you find and we can incrementally improve things.
Nope, was actually just going to look into data sections.


================
Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:673
+      return expect(AsmToken::EndOfStatement, "EOL");
+    }
+
----------------
aardappel wrote:
> sbc100 wrote:
> > tlively wrote:
> > > @aardappel, I wanted to test that this directive would properly round trip through the assembler and disassembler, but I wasn't able to get the assembler to handle a data section properly. Is this something that you would expect to work at the moment?
> > There are still issues with roundtriping in its full generality.   Open any bug you find and we can incrementally improve things.
> Nope, was actually just going to look into data sections.
Also, parsing of segments is in WasmAsmParser.cpp, so this code may make more sense there (if its about the container format, not the instruction set)


================
Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:290
+  int64_t expectInt() {
+    if (!Lexer.is(AsmToken::Integer)) {
+      error("Expected integer, got: ", Lexer.getTok());
----------------
Call `expect` here instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57938