-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
feat(compiler): Cleaner wasm output for low-level wasm types #1158
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work 🙏 I think we have 2 follow-ups to this:
- Please create an issue to add a NEAR smoketest to our CI - you can assign it to me, but it'll block dropping Node 14 support
- We need to document, or file an issue, or write a fix for binaryen. I'm not sure the best way to track this.
Smoketest issue is #1160. I'm not really sure how to track the binaryen issue either. Maybe a promise that I'll do it? 😛 I'm really still just trying to provide a good repro for them with binaryen.ml—I have a small Grain program that repros, but I would feel bad submitting that to them. |
Excellent news (in a way, we may have to abandon our stack-hack with tuples)—there's no Binaryen bug at all. They added an optimization that lifts identical code from the arms of |
Called this out over in WebAssembly/binaryen#4526 |
1692a4f
to
44ddf1e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(a,b)[0]
🙅♂️
44ddf1e
to
618840f
Compare
This avoids an issue in Binaryen v102 where spurious multivalue types appear because of our use of tuples (by using far fewer tuples).