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

AOT compilation: update to weval 0.2.12. #939

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

cfallin
Copy link
Contributor

@cfallin cfallin commented Sep 3, 2024

This includes a fix for irreducible control flow; see this PR in waffle.

@cfallin cfallin force-pushed the cfallin/update-weval-0.2.11 branch 2 times, most recently from aeea425 to 88e7480 Compare September 3, 2024 21:46
@cfallin
Copy link
Contributor Author

cfallin commented Sep 3, 2024

@guybedford @elliottt I've intentionally avoided a submodule update here as we have the open issue with the new headers implementation in StarlingMonkey interacting with weval somehow in debug builds; the version of weval pulled in by the package.json here is used by the js-compute-runtime toplevel driver, separately from the version of weval that the StarlingMonkey submodule uses.

@cfallin
Copy link
Contributor Author

cfallin commented Sep 3, 2024

Hmm, this is triggering some issues deep in the Wasm backend; I need to investigate further.

cfallin and others added 2 commits September 3, 2024 16:55
This includes a fix for irreducible control flow; see [this
PR](cfallin/waffle#13) in waffle.
@guybedford
Copy link
Member

@guybedford @elliottt I've intentionally avoided a submodule update here as we have the open issue with the new headers implementation in StarlingMonkey interacting with weval somehow in debug builds;

For what it is worth, the new headers work landed already in js-compute-runtime without any error, so we are already on the latest StarlingMonkey here. But fine to maintain separate versioning as well!

@cfallin cfallin changed the title AOT compilation: update to weval 0.2.11. AOT compilation: update to weval 0.2.12. Sep 4, 2024
@cfallin
Copy link
Contributor Author

cfallin commented Sep 4, 2024

I've pushed a new version of weval (0.2.12) with some additional fixes that I believe should pass tests now (let's see how CI does); analogous update in StarlingMonkey at bytecodealliance/StarlingMonkey#141.

For what it is worth, the new headers work landed already in js-compute-runtime without any error, so we are already on the latest StarlingMonkey here. But fine to maintain separate versioning as well!

Ah, interesting, and we're passing WPT here still -- I had been worried about hitting the same issue as bytecodealliance/StarlingMonkey#114 but I guess for whatever reason we don't see that here? In any case, I'm happy to do a separate PR to update to latest StarlingMonkey once above PR lands too, or land it first and update this one, either way.

@guybedford guybedford merged commit 9c31435 into main Sep 4, 2024
23 checks passed
@guybedford guybedford deleted the cfallin/update-weval-0.2.11 branch September 4, 2024 02:53
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