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

Typed continuations: nocont and cont basic heap types #6468

Merged
merged 6 commits into from
Apr 4, 2024

Conversation

frank-emrich
Copy link
Contributor

@frank-emrich frank-emrich commented Apr 3, 2024

This PR is part of a series that adds basic support for the typed continuations/wasmfx proposal.

This particular types adds cont and nocont as top and bottom types for continuation types, completely analogous to func and nofunc for function types (also: exn and noexn).

@tlively
Copy link
Member

tlively commented Apr 3, 2024

The isFunction / isSignature distinction is definitely something I would like to make better, perhaps by removing isFunction, but I haven't looked into it in detail. We also don't have different versions of isStruct or isArray that include the struct and array heap types, so we're already inconsistent about the API structure. All this is to say I think it would be fine (and even preferable) to undo the renaming of isContinuation.

@frank-emrich
Copy link
Contributor Author

Fair enough, I'm happy to keep the old name. I've undone the renaming and updated the PR description.

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

LGTM, but for more test coverage it might be a good idea to add continuation types to TestHeapTypeRelations in test/gtest/type-builder.cpp as well.

@frank-emrich
Copy link
Contributor Author

Good call, I actually caught a place I had forgotten to update this way!

Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Excellent!

@tlively tlively merged commit 365f12e into WebAssembly:main Apr 4, 2024
14 checks passed
@frank-emrich
Copy link
Contributor Author

Just FYI, this was the last missing piece to add basic support for the core of the proposal to binaryen. Only resume.throw and barrier instructions are missing, which aren't a high priority for us at this point.

I'd therefore prioritize implementing validation for the already added instructions first. Can the new wat/wast parser now be used for spec tests, or is that going to be the case soon-ish? I'm asking because I'm hoping to re-use our existing spec tests when adding validation, rather than having to transform them into binaryen's text format.

@tlively
Copy link
Member

tlively commented Apr 4, 2024

No support for running spec tests with the new parser yet, but I hope to be done with that within a couple weeks.

@tlively
Copy link
Member

tlively commented Apr 29, 2024

@frank-emrich, the new parser is now used for top-level modules in wasm-shell, so you should try running your spec tests now. It's possible you'll run into some other limitations, but at least there should be fewer now. I plan to keep working toward being able to run the full upstream test suite.

@frank-emrich
Copy link
Contributor Author

That's great, thanks for letting me know!

@gkdn gkdn mentioned this pull request Aug 31, 2024
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.

2 participants