[PATCH] D120229: [WebAssembly] Update WebAssemblyAsmTypeCheck for table.get

Tue Feb 22 05:37:07 PST 2022

pmatos added a comment.

Thanks for the patch. In addition to what @sbc100 wrote, some further comments inline.

================
Comment at: lld/test/wasm/funcref.s:23
+# CHECK-NEXT:          ReturnTypes:
+# CHECK-NEXT:            - FUNCREF
----------------
sbc100 wrote:
> I think this test probably belongs in `llvm/test/MC/WebAssembly/` since this change doesn't look like its related to the linker.
+1 to the comment from @Sbc100 and also maybe change the filename to something more descriptive. For example, `tableget-funcref-typecheck.s`.

================
Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp:195
+    break;
+  case wasm::WASM_SYMBOL_TYPE_FUNCTION:
+  case wasm::WASM_SYMBOL_TYPE_DATA:
----------------
If we are doing a `getTable` here, the only valid case here is `WASM_SYMBOL_TYPE_TABLE`. I think the others can be removed and the `switch` simplified to an `if`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120229