Skip to content

[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

Merged
merged 13 commits into from
May 26, 2025

Conversation

radekdoulik
Copy link
Member

Do not try to setup the jit and use it, instead use the interpreter

@Copilot Copilot AI review requested due to automatic review settings May 20, 2025 15:51
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 20, 2025
@radekdoulik radekdoulik added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 20, 2025
@radekdoulik radekdoulik added this to the Future milestone May 20, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 in codeman.cpp
  • Defines INTERP_API for symbol visibility and forces interpretation of all methods in eeinterp.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

@jakobbotsch jakobbotsch added area-CodeGen-Interpreter-coreclr and removed area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI labels May 20, 2025
Copy link
Contributor

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

@radekdoulik
Copy link
Member Author

The armel build was broken in main too and was fixed with #115767

@jkotas
Copy link
Member

jkotas commented May 22, 2025

Could you please fix the merge conflicts?

radekdoulik and others added 6 commits May 23, 2025 13:59
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>
@radekdoulik radekdoulik force-pushed the wasm-coreclr-use-interp-on-wasm branch from d2c3c65 to fc1f6b2 Compare May 23, 2025 12:07
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nits

Copy link
Member

@jkotas jkotas left a 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

@radekdoulik radekdoulik merged commit 12eba0a into dotnet:main May 26, 2025
109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants