Page MenuHomePhabricator

[WebAssembly] WIP: Add support for reference types
Needs ReviewPublic

Authored by loladiro on Aug 9 2019, 3:26 PM.
This revision needs review, but there are no reviewers specified.



This is a quick-and-dirty straw man implementation of reference-types
support for the WebAssembly backend. anyref value types are represented
as non-integral pointers in addrspace(1) and tables are represented as
external global in addrspace(1) pointing to pointers also in addrspace(1).

The bulk of the work here is just adding all the boiler plate for multiple tables
and the new instruction encodings. The ISel part of this is very primitive
and just the quickest thing I could possibly do to play with this. In its current
state, I've gotten it to emit .o files that I think are correctly formed,
but I haven't been able to test them properly (I think at least some of the
problem is browser bugs I'm looking into).

This isn't ready for review or anything, but I figured
a) This might save somebody else some time in implementing the boiler plate
b) It could serve as a starting point to discuss how to represent this in IR

Event Timeline

loladiro created this revision.Aug 9 2019, 3:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2019, 3:26 PM
loladiro updated this revision to Diff 214467.Aug 9 2019, 3:29 PM

Accidentally only pushed half the changes

Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2019, 3:29 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Harbormaster completed remote builds in B36554: Diff 214467.
fitzgen added a subscriber: fitzgen.Aug 9 2019, 3:38 PM
tlively added a subscriber: tlively.Aug 9 2019, 4:22 PM

Thank you! We also have been floating some ideas about supporting reference types as types in another address space, but haven't actually started implementing it AFAIK. I like the general direction. Let's wait for other people's comments too.

  • Are you planning to land this at some point (after adding some tests and such), or is this purely for discussions for the direction? If it's the latter, maybe I don't need to comment in the code in too much detail.. But anyway I did, so please ignore them if you are not actually planning to land this.
  • Even if not to the extent to cover the whole CL, I think a few test cases still would be helpful for discussions to see how reference types can be represented in .ll files and printed in .s files.
  • There are many places that add anyref but not funcref.
  • Out of curiousity, is Julia planning to use reference types? If so, for which concept?
  • I know it is not finished, but please clang-format if you are gonna land this later.
  • Maybe you've already seen this, but if we end up landing this, we should update the linking spec in the tool convention, because we add a new relocation type.

Why not funcref?


I guess this is for the index to distinguish tables, not elements within the table, right? The name is not very distinguishable with R_WASM_TABLE_INDEX_SLEB and R_WASM_TABLE_INDEX_I32 above, so I guess we need a new name or something


Do we need this? Other symbol kinds (global, function, and events) don't have corresponding VK_WSM_ entries here. VK_WASM_TYPEINDEX is left for some other reason (D58472).


Can't this be other reference types, like FUNCREF?


event -> table


Why should we only allow anyref?


Global -> Table


It looks it assumes at this point we are not PIC. Maybe we can assert in if (isPositionIndependent()) { above that nonzero address space is not supported?


How are we gonna represent other reference types, such as funcref or exnref?


Not sure what this means. Isn't this the type of table itself, not that of the elements?

  • Nit: For these two lines: We usually don't seem to have a whitespace after : for *.td files
  • Is the reason for using offset instead of index here that you plan to use named operand table methods?

I guess the name should be anyref.table.get to follow the wasm backend convention (even though we don't have separate instructions). And because "table.get" part is the constant, we only need to parameterize the prefix. This can be similar to something like here.


I guess rules to translate instructions for non-zero indices have not been implemented here, right?


Real nit: maybe we don't need a newline between reference types


Is this gonna support existing .ll files with the previous data layout?

wingo added a subscriber: wingo.Aug 12 2019, 6:38 AM

Overall direction looks good to me as well.

Ditto that tests would be useful just to make it clearer how this is represented in IR.


We'll almost-surely want to pull this 1 out into a constant, WasmAnyrefAddressSpace or something

x86 uses address spaces starting at 256 and counting up for its architecture-specific address spaces. The docs say "Address spaces 1-255 are currently reserved for user-defined code." so we should consider using 256 here.