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

Support block-type parameters #6407

Closed
wingo opened this issue Mar 18, 2024 · 2 comments · Fixed by #7149
Closed

Support block-type parameters #6407

wingo opened this issue Mar 18, 2024 · 2 comments · Fixed by #7149

Comments

@wingo
Copy link
Collaborator

wingo commented Mar 18, 2024

Binaryen does not currently support block / if / loop / try parameters, which were added to the standard a few years ago via the multi-value proposal.

The Hoot Scheme compiler emits blocks with parameters, which works fine in browsers, but we cannot use wasm-opt on our binaries for this reason:

$ bin/wasm-opt --all-features ~/src/guile-hoot/out.wasm
[parse exception: control flow inputs are not supported yet (at 0:83175)]
Fatal: error parsing wasm (try --debug for more info)

See also #5962 and #5950.

@tlively
Copy link
Member

tlively commented Mar 18, 2024

Thankfully once we switch to the new wat parser (and also rewrite the binary parser to use the new IRBuilder utility), it will be easy to add support for block parameters by having the parsers implicitly lower them away and use locals instead.

Unfortunately having more in-depth support for block parameters than that is not really feasible with the structure of Binaryen IR, but it's plausible that in the future we could use an alternative IR to do late-stage optimizations that could reintroduce them before module writing.

@jayphelps
Copy link
Contributor

jayphelps commented Apr 21, 2024

Moonbit compiler also emits these. Btw I much appreciate that you all added error checking for this situation and now emit a clear error when encountered. I was at first using an old version of wasm-opt that didn't emit that error and I was confused what was wrong, but once I updated I was happy to at least see a clear answer.

tlively added a commit that referenced this issue Dec 13, 2024
Since multivalue was standardized, WebAssembly has supported not only
multiple results but also an arbitrary number of inputs on control flow
structures, but until now Binaryen did not support control flow input.
Binaryen IR still has no way to represent control flow input, so lower
it away using scratch locals in IRBuilder. Since both the text and
binary parsers use IRBuilder, this gives us full support for parsing
control flow inputs.

The lowering scheme is mostly simple. A local.set writing the control
flow inputs to a scratch local is inserted immediately before the
control flow structure begins and a local.get retrieving those inputs is
inserted inside the control flow structure before the rest of its body.
The only complications come from ifs, in which the inputs must be
retrieved at the beginning of both arms, and from loops, where branches
to the beginning of the loop must be transformed so their values are
written to the scratch local along the way.

Resolves #6407.
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 a pull request may close this issue.

3 participants