This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Use intrinsics for table.get/set instructions
ClosedPublic

Authored by pmatos on Sep 22 2022, 6:32 AM.

Details

Summary

Initial table.get/set implementation would match and lower combinations
of GEP+load/store to table.get/set instructions. However, this is error
prone due to potential combinations of GEP+load/store we don't implement,
and load/store optimizations. By changing the code to using intrinsics, we
avoid both issues and simplify the code.

New builtins implemented:

  • @llvm.wasm.table.get.externref
  • @llvm.wasm.table.get.funcref
  • @llvm.wasm.table.set.externref
  • @llvm.wasm.table.set.funcref

Diff Detail

Event Timeline

pmatos created this revision.Sep 22 2022, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 22 2022, 6:32 AM
pmatos requested review of this revision.Sep 22 2022, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 22 2022, 6:32 AM
asb accepted this revision.Sep 26 2022, 1:57 AM

This change LGTM, thanks!

Needing one intrinsic per GC type is non-ideal, but the longer term plan to move to a 'wasmref' type (with associated type metadata) once we've fleshed out that prototype and go through necessary RFCs should remove that issue.

I wondered a bit about the properties for the intrinsic, and now I've reminded myself that intrinsics are not speculatable by default I think they're fine as defined.

llvm/include/llvm/IR/IntrinsicsWebAssembly.td
55

Nit: this line seems to have unwanted whitespace (a bunch of spaces)

This revision is now accepted and ready to land.Sep 26 2022, 1:57 AM
tlively accepted this revision.Sep 26 2022, 11:39 AM

LGTM!

pmatos marked an inline comment as done.Sep 27 2022, 12:16 AM