-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[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_WASMto disable JIT on WASM - Statically links interpreter startup and
getJitentry points for WASM incodeman.cpp - Defines
INTERP_APIfor symbol visibility and forces interpretation of all methods ineeinterp.cppwhen 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
#elifwithout a condition is invalid here. Replace it with#elseor#elif !defined(TARGET_WASM)to properly handle the non-WASM branch.
#elif // !TARGET_WASM
src/coreclr/vm/codeman.cpp:1781
- This
#elifdirective is missing an expression and will not compile. Use#elseto 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 <[email protected]>
Co-authored-by: Jan Kotas <[email protected]>
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