Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update embedding and execution specs for memory64 #74

Merged
merged 3 commits into from
Sep 23, 2024

Conversation

bvisness
Copy link
Collaborator

@bvisness bvisness commented Aug 2, 2024

Memory and table operations in the embedding section have been updated to use u64 instead of u32, reflecting the changes to limits. Various operations in the execution section were also updated to reflect idxtype's inclusion in memtype and tabletype.

Additionally, I have switched the storage in memory and table instances to sequences instead of vectors. This is because vectors are restricted to a maximum length of 2^32, which is incompatible with i64 storage.

Finally, I have decided not to try and address #67 in this PR or the one to follow. I'll submit a separate PR to rename "index type" to "offset type" across the board after all other spec changes are done, and then we can decide if we actually want to do it or not.

See also #75, which updates the JS API.

document/core/syntax/conventions.rst Outdated Show resolved Hide resolved
document/core/exec/modules.rst Outdated Show resolved Hide resolved
Vectors in the spec are syntactically limited to a maximum size of 2^32.
This is incompatible with memory64, so the storage has been changed to
use sequences instead of vectors. The execution semantics are
unaffected.
Memory and table operations in the embedding section have been updated
to use u64 instead of u32, reflecting the changes to `limits`. Various
operations in the execution section were also updated to reflect
`idxtype`'s inclusion in `memtype` and `tabletype`, as well as the
supporting validation rules.
@bvisness bvisness requested a review from rossberg August 7, 2024 15:27
Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for the -1 you removed, see comment.

Btw, please never force-push to a branch under review. That breaks the GH review workflow, such as relative diffs.

document/core/valid/types.rst Show resolved Hide resolved
@bvisness
Copy link
Collaborator Author

@sbc100 Are we good to merge? I'd like to get this in so I can start working on #67.

@sbc100
Copy link
Member

sbc100 commented Sep 23, 2024

Sorry, I missed that you were waiting on me here. I added you has a collaborator so you can land these kind of thing yourself going forward.

@sbc100 sbc100 merged commit cd8bba6 into WebAssembly:main Sep 23, 2024
7 checks passed
@bvisness bvisness deleted the embedding-spec branch October 11, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants