-
Notifications
You must be signed in to change notification settings - Fork 21
Implement table64 extension #51
Comments
Looks good, although I wouldn't consider wabt a blocker for phase 4 if we have support in Binaryen. |
My understanding of ocaml is shaky at best but by my read the current implementation of the binary encoding of a table type is to use the third bit of the prefix byte is used to indicate whether the index type is i64 (where if it's 0 then it's i32, as-is today). This makes sense for memories where the second bit was already claimed for the Given the shared-everything-threads proposal it seems like we'll get a |
Yes, the idea is tie match the flags that we use for memories. For example, in wabt we have: https://github.com/WebAssembly/wabt/blob/356931a867c7d642bc282fff46a1c95ab0e843f3/include/wabt/binary.h#L24-L29 and these flags are used for both memories and tables. It makes sense to me to re-use these, even though we don't currently have shared tables. Are there downsides to doing this that you are thinking of? |
I don't think there's any downside to doing this, no. In Wasmtime we don't parse limits in the same way between tables/memories because for so long the encoding of tables/memories has diverged - for example for quite some times memories can be shared or 64-bit but tables can't be. Even with the table64 extension and a hypothetical shared table type the custom-page-sizes proposal still has different parsing of the flags byte which won't ever get mapped to tables. Basically while the parsing of limits was trivially the same between memories/tables I don't think we should take it as a design guideline that they must match for all future proposals as well. The 64-bit-ness and shared-ness makes sense to match since we might as well, though. |
Unless there is a strong reason to do otherwise (like running out of bits), I think it's preferable to keep them in sync. (We have been doing the same for elem/data segments.) |
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
Tests is still very limited. Hopefully we can use the upstream spec tests soon and avoid having to write our own tests for `.set/.set/.fill/etc`. See WebAssembly/memory64#51
See WebAssembly/memory64#51 Includes workaround for #2422
See WebAssembly/memory64#51 Includes workaround for #2422
See WebAssembly/memory64#51 Includes workaround for #2422
See WebAssembly/memory64#51 Includes workaround for #2422
See WebAssembly/memory64#51 Includes workaround for #2422
The SpiderMonkey implementation has landed in Nightly: https://bugzilla.mozilla.org/show_bug.cgi?id=1893643 Aside from the concerns laid out in #68, our implementation is ready for phase 4. |
Fantastic. Thanks for all the work on this. I think #68 is now the only blocker. |
@bvisness is there a command line flag I need to pass to spidermonkey to enable memory64? I'm trying to run some tests in emscripten CI. |
In the shell it should get enabled with |
Hmm.. I can't seem to make -P work:
|
Now that #46 is decided we need get this extension implemented in various places:
External places:
The text was updated successfully, but these errors were encountered: