-
Notifications
You must be signed in to change notification settings - Fork 5k
[wasm][coreclr] Interpret everything on wasm #115788
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
[wasm][coreclr] Interpret everything on wasm #115788
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.
Pull Request Overview
This PR ensures that on the WebAssembly (WASM) target, the runtime uses the interpreter exclusively and skips loading or invoking the JIT.
- Wraps JIT-loading calls in
#ifndef TARGET_WASM
to disable JIT on WASM - Statically links interpreter startup and
getJit
entry points for WASM incodeman.cpp
- Defines
INTERP_API
for symbol visibility and forces interpretation of all methods ineeinterp.cpp
when building for WASM
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/coreclr/vm/jitinterface.cpp | Add TARGET_WASM guards to bypass JIT loading and runtime paths |
src/coreclr/vm/codeman.cpp | Conditionally include interpreter headers and stub JIT startup/getJit |
src/coreclr/interpreter/interpretershared.h | Define INTERP_API macro for cross-platform symbol visibility |
src/coreclr/interpreter/eeinterp.cpp | Force doInterpret = true on WASM builds |
Comments suppressed due to low confidence (2)
src/coreclr/vm/jitinterface.cpp:13365
- The preprocessor directive
#elif
without a condition is invalid here. Replace it with#else
or#elif !defined(TARGET_WASM)
to properly handle the non-WASM branch.
#elif // !TARGET_WASM
src/coreclr/vm/codeman.cpp:1781
- This
#elif
directive is missing an expression and will not compile. Use#else
to cover the non-WASM case.
#elif
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
The armel build was broken in main too and was fixed with #115767 |
Could you please fix the merge conflicts? |
Do not try to setup the jit and use it, instead use the interpreter
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
d2c3c65
to
fc1f6b2
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.
Nits
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.
LGTM once the CI is green
Do not try to setup the jit and use it, instead use the interpreter