Skip to content

[wasm][interpreter] Defer managed calli cookie resolution to execution time#125455

Open
radekdoulik wants to merge 7 commits intodotnet:mainfrom
radekdoulik:clr-wasm-improve-calli-cookie-resolution
Open

[wasm][interpreter] Defer managed calli cookie resolution to execution time#125455
radekdoulik wants to merge 7 commits intodotnet:mainfrom
radekdoulik:clr-wasm-improve-calli-cookie-resolution

Conversation

@radekdoulik
Copy link
Member

On portable entry point platforms, calli instructions targeting managed code previously required the compiler to compute a signature cookie at compile time via GetCookieForInterpreterCalliSig. This caused assertion failures for delegate shuffle thunks and required
a hard-coded missingCookies table in the WASM build generator to pre-generate interp-to-native stubs for every possible managed calli signature.

Implements #121222

Changes

Compiler (compiler.cpp, compiler.h, interpretershared.h):

  • For managed callis on FEATURE_PORTABLE_ENTRYPOINTS platforms, store the calli IL signature token in a data item instead of computing the cookie at compile time. Allocate two consecutive data items: one for the sig token (immutable) and one for caching the
    resolved cookie.
  • Unmanaged callis (PInvoke) continue to compute the cookie at compile time.
  • Add CalliFlags::DeferredCookie flag to signal the executor that the cookie needs runtime resolution.
  • For CORINFO_CALL_CODE_POINTER / CORINFO_VIRTUALCALL_LDVIRTFTN paths, pass NULL cookie since these targets always have a MethodDesc and take the CALL_INTERP_METHOD fast path.

Executor (interpexec.cpp):

  • At execution time, calli targets with a MethodDesc (interpreted methods, FCalls) are dispatched via CALL_INTERP_METHOD using TryGetMethodDesc.
  • For JIT helper portable entry points (no MethodDesc) with DeferredCookie, resolve the cookie lazily from the sig token via GetSigFromToken + MetaSig + GetCookieForCalliSig, then cache it in the adjacent data item slot.
  • Use VolatileLoadWithoutBarrier/VolatileStoreWithoutBarrier for the cache slot, matching the existing dispatch token cache pattern.
  • Wrap GetSigFromToken with IfFailThrow for proper error handling.

Portable entry points (precode_portable.cpp, precode_portable.hpp):

  • Add TryGetMethodDesc — returns nullptr instead of asserting when the PE has no MethodDesc.

Generator cleanup (ManagedToNativeGenerator.cs, callhelpers-interp-to-managed.cpp):

  • Remove the hard-coded missingCookies array (40 signatures) from the WASM build task.
  • Remove 40 corresponding pre-generated interp-to-native stubs (~288 lines) that are no longer needed.

@radekdoulik radekdoulik added this to the Future milestone Mar 11, 2026
@radekdoulik radekdoulik added the arch-wasm WebAssembly architecture label Mar 11, 2026
@radekdoulik radekdoulik requested a review from maraf as a code owner March 11, 2026 20:26
Copilot AI review requested due to automatic review settings March 11, 2026 20:26
@radekdoulik radekdoulik requested review from janvorli and removed request for janvorli March 11, 2026 20:27
@dotnet-policy-service
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

radekdoulik commented Mar 11, 2026

To consider: with the lazy approach, is the cache check overhead in execution phase ok? Is there better way how to cache the cookie? Also note that the unmanaged path is currently not deferring the cookie resolution to execution phase to avoid the cache check overhead.

Copy link
Contributor

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 changes how the interpreter handles calli signature cookies on portable entry point platforms (notably WASM): instead of requiring the compiler to resolve the cookie up-front, it defers cookie resolution to execution time (and caches it), avoiding compile-time asserts and eliminating the need for the WASM generator’s hard-coded “missing cookie” pre-generation list.

Changes:

  • Interpreter compiler now encodes managed calli sites with a signature-token + runtime cookie cache slot (via a new CalliFlags::DeferredCookie).
  • Interpreter executor (interpexec.cpp) routes portable-entry-point targets that have a MethodDesc through CALL_INTERP_METHOD, and lazily resolves/caches cookies for portable entry points without a MethodDesc.
  • WASM build/gen cleanup removes the missingCookies list and removes the corresponding pre-generated interp-to-managed thunks/stubs.

Reviewed changes

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

Show a summary per file
File Description
src/tasks/WasmAppBuilder/coreclr/ManagedToNativeGenerator.cs Removes the temporary missingCookies signature list from thunk/stub generation inputs.
src/coreclr/vm/wasm/callhelpers-interp-to-managed.cpp Removes previously pre-generated signature thunk helpers and their entries from g_wasmThunks.
src/coreclr/vm/precode_portable.hpp Adds PortableEntryPoint::TryGetMethodDesc API for non-asserting lookup.
src/coreclr/vm/precode_portable.cpp Implements TryGetMethodDesc (returns nullptr rather than asserting).
src/coreclr/vm/interpexec.cpp Adds runtime cookie resolution/caching path for deferred managed calli signatures on portable entry points.
src/coreclr/interpreter/inc/interpretershared.h Adds CalliFlags::DeferredCookie to signal runtime resolution.
src/coreclr/interpreter/compiler.h Extends EmitCalli to accept a deferredCookie parameter (defaulted).
src/coreclr/interpreter/compiler.cpp Emits deferred-cookie data items for managed calli on portable entry points; keeps eager cookies for unmanaged calli.

#ifdef FEATURE_PORTABLE_ENTRYPOINTS
// On platforms with portable entrypoints, managed calli targets with a MethodDesc
// are dispatched via CALL_INTERP_METHOD at execution time, which derives the cookie
// from the MethodDesc. For targets without a MethodDesc (JIT helper PEs), the cookie
Copy link
Member

Choose a reason for hiding this comment

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

How many different JIT helper PEs that get called via CALLI are there? Would it be easier to just turn them into FCalls with MethodDescs to avoid this special casing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure how many there are in libraries. The case I was hitting is through ActivatorCache::CreateUninitializedObject where the _pfnAllocator function pointer is set to newobj allocator.

I asked copilot to find more places like this and it found 2 more places, in BoxCache and CreateUninitializedCache - both have calli to newobj allocator as well.

I think user code cannot have function pointer to something without method desc, so it should be hit just by code in our libraries.

So if I understand your idea, it would mean turning the newobj allocators to FCalls and add assert in that path, so that we find it out in case there are more scenarios or when we create new such scenario in libraries.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's the idea. Since it is all about newobj allocators, you may be able to use one dummy FCall for all of them. E.g. replace portableEntryPoint->Init((void*)pfnHelper) in getHelperFtnStatic with:

if (ftnNum >= CORINFO_HELP_NEWFAST && ftnNum <= CORINFO_HELP_NEWSFAST_ALIGN8_FINALIZE)
{
    // CoreLib calls newobj helpers via calli. Give these helper a MethodDesc to allow interpreter to find the method signature.
    portableEntryPoint->Init((void*)pfnHelper, CoreLibBinder::GetMethod(METHOD__RUNTIME_HELPERS__NEWOBJ_DUMMY);
}   
else
{
    portableEntryPoint->Init((void*)pfnHelper);
}

Copilot AI review requested due to automatic review settings March 13, 2026 09:50
Copy link
Contributor

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

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

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants