Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[wasm] Fix long branches in jiterpreter #106030

Merged
merged 4 commits into from
Aug 7, 2024
Merged

Conversation

kg
Copy link
Member

@kg kg commented Aug 6, 2024

@kg kg added the arch-wasm WebAssembly architecture label Aug 6, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 6, 2024
@kg
Copy link
Member Author

kg commented Aug 6, 2024

This change is correct but doesn't fix it, unfortunately. Will have to dig in further.

@am11 am11 added area-Codegen-Interpreter-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 6, 2024
Copy link
Contributor

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

@kg kg changed the title [wasm] maybe fix long branches in jiterp [wasm] Fix long branches in jiterpreter Aug 6, 2024
@kg
Copy link
Member Author

kg commented Aug 6, 2024

OK, the root cause was that an enum didn't get updated. But I cleaned up some code related to branches while I was in here, which should make it easier to debug in the future.

@pavelsavara
Copy link
Member

Could you please add flag that would make call loaderHelpers.mono_exit(1, err);, instead of just log warning ?
And then turn on that flag in test-main.js for all unit tests.

similar to withExitOnUnhandledError or withDumpThreadsOnNonZeroExit

Thanks

@kg kg merged commit 9fa8fd5 into dotnet:main Aug 7, 2024
32 of 37 checks passed
@kg
Copy link
Member Author

kg commented Aug 7, 2024

Could you please add flag that would make call loaderHelpers.mono_exit(1, err);, instead of just log warning ? And then turn on that flag in test-main.js for all unit tests.

similar to withExitOnUnhandledError or withDumpThreadsOnNonZeroExit

Thanks

Will do a follow up PR for this.

BrzVlad added a commit to BrzVlad/runtime that referenced this pull request Aug 9, 2024
BrzVlad added a commit that referenced this pull request Aug 9, 2024
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"

This reverts commit 9fa8fd5.

* Revert "[mono][interp] Remove short branches (#105386)"

This reverts commit ac89819.
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants