[PATCH] D111154: [WebAssembly] Implementation of table.get/set for reftypes in LLVM IR

[Thomas Lively via Phabricator via llvm-commits]

tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.

Nice! Just a couple nits but I think this is good to go.



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1478-1479
+  } else {
+    GA = dyn_cast<GlobalAddressSDNode>(Base->getOperand(0));
+    if (!GA) {
+      // This might be Case 1 above (or an error)
----------------
It would be nice to switch the `if` arms around so the `else` corresponds with the null case.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:72-75
+            GlobalVT->getArrayElementType()->getPointerAddressSpace() ==
+                    WebAssembly::WasmAddressSpace::WASM_ADDRESS_SPACE_FUNCREF
+                ? MVT::funcref
+                : MVT::externref;
----------------
It would be good to explicitly check the externref address space as well so we can error out if we get an unexpected address space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111154