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 Binaryen and utilize new ExpressionRunner API #1237

Merged
merged 6 commits into from
Apr 26, 2020
Merged

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Apr 22, 2020

This uses Binaryen's new ExpressionRunner that we have been waiting for to continue work on #1111. Overall this works well and untouched output is much more true to the source now since we no longer have to precompute conditions, but can instead drop them once we evaluated their outcome.

But: There's one remaining issue as can be seen on the failing std/typedarray test, where it fails to evaluate expressions like

(block
 (drop
  (local.get $0)
 )
 (i32.const 0)
)

due to bailing early when encountering a local.get with an unknown value, here breaking statically precomputed instanceof. A bit unfortunate, but going to look into this.

@dcodeIO
Copy link
Member Author

dcodeIO commented Apr 22, 2020

Binaryen PR: WebAssembly/binaryen#2787

@dcodeIO
Copy link
Member Author

dcodeIO commented Apr 23, 2020

With the new issue mentioned at the WG meeting fixed meanwhile, this is waiting for the PR above again. However, there have been objections on what the interpreter should do or not, so there's some uncertainty there.

@dcodeIO
Copy link
Member Author

dcodeIO commented Apr 25, 2020

Implemented a workaround now that mimics the PR linked above by suppressing certain drops to be emitted. Affected code is not really untouched, like before. Even though not optimal, it somewhat allows us to use the new runner API today.

@dcodeIO dcodeIO merged commit 61b9079 into master Apr 26, 2020
@dcodeIO dcodeIO deleted the expressionrunner branch May 14, 2020 21:12
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.

1 participant