Skip to content

[wasm][R2R] Implement JIT_PollGC rare path - #131653

Open
pavelsavara wants to merge 10 commits into
dotnet:mainfrom
pavelsavara:wasm-r2r-pollgc
Open

[wasm][R2R] Implement JIT_PollGC rare path#131653
pavelsavara wants to merge 10 commits into
dotnet:mainfrom
pavelsavara:wasm-r2r-pollgc

Conversation

@pavelsavara

Copy link
Copy Markdown
Member

Summary

Implements the WebAssembly JIT_PollGC helper, which was a PORTABILITY_ASSERT stub.

Problem

JIT_PollGC (CORINFO_HELP_POLL_GC) is emitted by RyuJIT/crossgen2 at GC-safe poll points (loop back-edges, call sites). On wasm it was unimplemented:

FCIMPL0(void, JIT_PollGC)
{
    PORTABILITY_ASSERT("JIT_PollGC is not implemented on wasm");
}

Once CoreLib itself is R2R-compiled for browser-wasm, its class constructors' GC polls call this helper during startup and abort the runtime (observed at System.AppContext.Setup → a CoreLib cctor).

Fix

Implement the helper to match the other architectures, whose JIT_PollGC asm stub tail-calls the managed Thread.PollGC. That managed helper's only effect is a p/invoke round-trip through the empty ThreadNative_PollGC QCall — the GC transition parks the thread at a safe point and the pinvoke rare path performs the actual suspension.

This change reproduces that round-trip directly with the existing inline-pinvoke helpers:

  • Fast path: return immediately unless g_TrapReturningThreads is set (no GC / ThreadAbort / debugger suspend pending) — the common case.
  • Rare path: JIT_PInvokeBeginImpl pushes an InlinedCallFrame seeded from the R2R caller's stack pointer and switches to preemptive mode, so a pending GC can suspend and walk this thread's R2R frames at the safe point. The end sequence mirrors JIT_PInvokeEnd: return to cooperative mode and, when a GC/abort is still pending, JIT_PInvokeEndRarePath performs the actual suspension (RareDisablePreemptiveGC), services any ThreadAbort, and pops the frame.

Notes / testing

  • Builds cleanly (clr.runtime, browser-wasm, Debug).
  • The fast path (which unblocks CoreLib R2R startup) is straightforward and low-risk.
  • The rare path only executes during an actual GC with g_TrapReturningThreads set, which is hard to trigger deterministically on single-threaded wasm; it has not been exercised at runtime. A GC-stress run (or forcing g_TrapReturningThreads) would be worthwhile before relying on it.

Note

This change and description were prepared with the assistance of GitHub Copilot.

The wasm JIT_PollGC helper was a PORTABILITY_ASSERT stub, so any GC poll emitted by crossgen2 into R2R code (e.g. a CoreLib class constructor at startup) aborted the runtime.

Implement it to match the other architectures, whose asm stub tail-calls the managed Thread.PollGC. That helper's only effect is a p/invoke round-trip through the empty ThreadNative_PollGC QCall: the GC transition parks the thread at a safe point and the pinvoke rare path performs the suspension. The fast path returns immediately when no GC/ThreadAbort/suspend is pending; the rare path reuses the inline-pinvoke helpers (JIT_PInvokeBeginImpl seeds an InlinedCallFrame from the R2R caller's stack pointer and goes preemptive so a pending GC can walk the R2R frames, then JIT_PInvokeEndRarePath performs RareDisablePreemptiveGC, services any ThreadAbort, and pops the frame).
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Jul 31, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Implements the WebAssembly VM JIT_PollGC helper (used at GC-safe poll points) so it no longer aborts at runtime, by adding a fast-path check on g_TrapReturningThreads and a rare-path transition sequence intended to allow GC/thread-suspend to proceed safely.

Changes:

  • Replace the wasm JIT_PollGC PORTABILITY_ASSERT stub with a functional implementation.
  • Add a fast path that returns immediately when no trap/suspend is pending.
  • Add a rare path that sets up an InlinedCallFrame and performs the GC-mode transition/suspension handshake.

Comment thread src/coreclr/vm/wasm/helpers.cpp
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 31, 2026 16:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/coreclr/vm/wasm/helpers.cpp:808

  • JIT_PollGC currently takes the inline-pinvoke rare path whenever g_TrapReturningThreads != 0, but it doesn’t replicate Thread.PollGC’s additional guard (CatchAtSafePoint). On other architectures the rare path tail-calls Thread.PollGC, which returns without doing the transition when the current thread isn’t actually in a catch-at-safe-point state. Without this check, wasm may execute JIT_PInvokeEndRarePath (and potentially block/suspend) in cases where other architectures would do nothing.
    // Fast path: nothing to do unless a GC (or ThreadAbort / debugger suspend) is pending.
    if (!g_TrapReturningThreads)
        return;

    // Rare path. On other architectures JIT_PollGC's asm stub tail-calls the managed Thread.PollGC,

Comment thread src/coreclr/vm/wasm/helpers.cpp Outdated
Comment thread src/coreclr/vm/wasm/helpers.cpp Outdated
Comment thread src/coreclr/vm/wasm/helpers.cpp Outdated
Copilot AI review requested due to automatic review settings August 4, 2026 14:47
@pavelsavara

Copy link
Copy Markdown
Member Author
image

@pavelsavara

pavelsavara commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

You may want to do a one-off test run without the g_TrapReturningThreads fast path to get proper coverage of the slow path.

I did, but on ST system, this doesn't do anything. There is no GC thread to kick in.
I guess we will fix this once we have MT and something is setting g_TrapReturningThreads outside of VM startup.

Should we rather leave the rare path implementation empty until then ?

@pavelsavara
pavelsavara requested a review from jkotas August 4, 2026 14:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/coreclr/vm/wasm/helpers.cpp:816

  • JIT_PollGCRarePath constructs an InlinedCallFrame as a normal C++ local and then calls JIT_PInvokeBeginImpl, which placement-news an InlinedCallFrame into that same storage. If InlinedCallFrame has a non-trivial constructor (e.g., under FEATURE_INTERPRETER), this results in starting the object lifetime twice and is undefined behavior. Also, the wasm implementation currently always performs the pinvoke transition whenever g_TrapReturningThreads != 0, but the canonical path on other architectures tail-calls Thread.PollGC, which first gates the transition on CatchAtSafePoint() (see Thread.CoreCLR.cs). Adding the CatchAtSafePoint() gate here keeps semantics aligned and avoids unnecessary rare-path work for cases where the global is non-zero but the thread shouldn't trap.
EXTERN_C void JIT_PollGCRarePath(void* sp)
{
    InlinedCallFrame inlinedCallFrame;
    JIT_PInvokeBeginImpl(sp, &inlinedCallFrame);

Comment thread src/coreclr/vm/wasm/helpers.cpp Outdated
"i32.load %[g_TrapReturningThreads]\n"
"if\n"
" global.get __stack_pointer\n"
" local.set 1\n" /* save previous __stack_pointer into the unused pep local */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The FCall infrastructure stores the __stack_pointer in the callersStackPointer local.

#define WASM_WRAPPER_FUNC_INITIAL \
{ \
asm ("local.get 0\n" /* Capture callersStackPointer onto the stack for calling _IMPL function*/ \
"local.get 0\n" /* Capture callersStackPointer onto the stack for setting the __stack_pointer */ \
"global.get __stack_pointer\n" /* Get current value of stack global */ \
"local.set 0\n" /* Store current stack global into callersStackPointer local */ \
"global.set __stack_pointer\n" /* Set stack global to the initial value of callersStackPointer, which is the current stack pointer for the interpreter call */
#define WASM_WRAPPER_FUNC_EPILOG(_method) \
"call %0\n" /* Call the actual implementation function*/ \
"local.get 0\n" /* Load the original stack pointer from stack Arg local */ \
"global.set __stack_pointer\n" /* Restore the original stack pointer to the stack global */ \
"return" :: "i" (_method ## _IMPL)); \

Is there an advantage in doing one or the other way - does the wasm JIT generate better code for one vs. the other?

@pavelsavara pavelsavara Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

local.0 and local.1 are both identical scratch variables.

I don't know if the V8 JIT can re-order the if g_TrapReturningThreads before global.get __stack_pointer if we use WASM_WRAPPER_FUNC_INITIAL. I estimate that it can. Is that what you are asking ?

I slightly prefer the hand rolled asm block over the WASM_WRAPPER_FUNC_INITIAL reuse.

@jkotas

jkotas commented Aug 6, 2026

Copy link
Copy Markdown
Member

Should we rather leave the rare path implementation empty until then ?

This will be used by managed debugger break-in. The debugger break-in is going to set the flag that triggers the rare path that will end up in calling into the debugger to report that we have stopped.

pavelsavara and others added 2 commits August 6, 2026 13:29
Copilot AI review requested due to automatic review settings August 6, 2026 11:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/vm/wasm/helpers.cpp
Copilot AI review requested due to automatic review settings August 6, 2026 11:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/coreclr/vm/wasm/helpers.cpp:816

  • JIT_PollGCRarePath duplicates the JIT_PInvokeEnd logic (disable preemptive GC, check g_TrapReturningThreads, then either JIT_PInvokeEndRarePath() or pop the frame). This creates a divergence risk if the P/Invoke end sequence changes (debug asserts, additional bookkeeping, etc.). Consider delegating to JIT_PInvokeEnd after JIT_PInvokeBeginImpl instead of re-implementing the end sequence here.
    Thread* pThread = (Thread*)inlinedCallFrame.m_pThread;
    pThread->m_fPreemptiveGCDisabled.StoreWithoutBarrier(1);
    if (g_TrapReturningThreads)
    {
        JIT_PInvokeEndRarePath();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-VM-coreclr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants