[PATCH] D99191: [WebAssembly][MC] Record limit constraints for table sizes

[Andy Wingo via Phabricator via llvm-commits]

wingo marked 3 inline comments as done.
wingo added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:860
         return true;
-      auto Type = parseType(TypeName);
-      if (!Type)
-        return error("Unknown type in .tabletype directive: ", TypeTok);
+      auto ElemType = parseType(ElemTypeName);
+      if (!ElemType)
----------------
sbc100 wrote:
> Using auto all over the place makes this harder to review and goes against llvm policy.   I guess its pre-existing code so you don't need to change it here.. but I wish it this had a type name
Yeah I sympathize.  I'll add an explicit type for ElemType here.


================
Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:874
       WasmSym->setType(wasm::WASM_SYMBOL_TYPE_TABLE);
-      WasmSym->setTableType(Type.getValue());
+      wasm::WasmTableType Type = {uint8_t(ElemType.getValue()), Limits};
+      WasmSym->setTableType(Type);
----------------
sbc100 wrote:
> All these casts make me this that WasmTableType should have ValType rather than uint8_t
Yeah.  Will need to change in any case once we get typed function references.  There are some related cases that should probably change at the same time, like `BinaryFormat/Wasm.h:WasmGlobalType::Type`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99191