Skip to content

Fix generic lookups and thunks - #129825

Merged
davidwrighton merged 10 commits into
dotnet:mainfrom
davidwrighton:FixGenericLookupsAndThunks
Jun 26, 2026
Merged

Fix generic lookups and thunks#129825
davidwrighton merged 10 commits into
dotnet:mainfrom
davidwrighton:FixGenericLookupsAndThunks

Conversation

@davidwrighton

@davidwrightondavidwrighton commented Jun 24, 2026

Copy link
Copy Markdown
Member

Add support for generic lookups to Wasm R2R code by following the DynamicHelper pattern we use on other platforms. This looks like a quite suitable implementation for Wasm, so I expect that this will remain for the long term.

Add support for WebAssembly .S files, as it was difficult to implement the helpers in inline assembly due to clang limitations.
Tweak Crossgen2 into generating uses of the DynamicHelper logic.

Also add support for precomputed thunks for various runtime helpers which were missing them, and add an assert that will fire even in interpreted runs for missing cases.

Fixes (most) of #129622
Fixes#129821

CopilotAI 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

This PR extends the WebAssembly ReadyToRun (R2R) pipeline to support generic dictionary lookups and related delay-load helper paths using the “DynamicHelper” pattern already used on other architectures, including adding dedicated Wasm .S helpers and updating Crossgen2’s Wasm thunk/signature plumbing accordingly.

Changes:

  • Add new Wasm dynamic helper stubs (in .S) for generic dictionary lookup variants and supporting VM glue to allocate the corresponding portable-entrypoint stub data.
  • Update Wasm delay-load/import thunk generation to support generic lookups via portable entrypoints (signature updates + always generating a thunk).
  • Add missing Wasm portable-entrypoint interpreter-call thunks for additional signatures (e.g., double arg + i32/i64 return, and S8 payload).

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
FileDescription
src/coreclr/vm/wasm/helpers.cppAdds additional portable-entrypoint interpreter-call thunks and updates the portable-entrypoint thunk table; removes Wasm placeholder DelayLoad_Helper stubs.
src/coreclr/vm/wasm/dynamichelpers.SNew Wasm assembly implementations of generic dictionary lookup dynamic helpers (incl. size-check / null-test variants and small constant-offset fast paths).
src/coreclr/vm/wasm/dynamichelpers.cppImplements Wasm DelayLoad_Helper entry and creates portable-entrypoint data for generic dictionary lookup helpers on Wasm.
src/coreclr/vm/wasm/asmconstants.hIntroduces Wasm-specific asm constants/offsets needed by dynamichelpers.S (with debug/fre handling and static-assert hooks).
src/coreclr/vm/readytoruninfo.hAdds GenericDictionaryDynamicHelperStubData_PortableEntryPoint layout used by Wasm helper stubs.
src/coreclr/vm/prestub.cppAllows resolving import sections by RVA when sectionIndex == (DWORD)-1 (used by Wasm delay-load helper), and excludes non-Wasm fixup kinds under TARGET_WASM.
src/coreclr/vm/jitinterface.cppAdds a debug assert ensuring portable-entrypoint targets have actual code under R2R (helps catch missing cases).
src/coreclr/vm/CMakeLists.txtWires new Wasm .S and .cpp sources (and asmconstants header) into the VM build.
src/coreclr/vm/cgensys.hAdjusts DelayLoad_Helper declaration for Wasm to match the new calling convention/signature.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../WasmImportThunkPortableEntrypoint.csUpdates generic-lookup thunk signature shape/string to include the portable-entrypoint parameter.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/.../DelayLoadHelperImport.csAlways generates a Wasm import-thunk portable entrypoint (including for generic lookups).
eng/native/configurecompiler.cmakeEnables ASM for Browser/WASI toolchains so preprocessed Wasm .S files can be assembled by clang-based toolchains.

Comment threadsrc/coreclr/vm/wasm/helpers.cpp
Comment threadsrc/coreclr/vm/wasm/helpers.cpp
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.cpp Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 24, 2026 22:53

CopilotAI 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 12 out of 12 changed files in this pull request and generated 3 comments.

Comment threadsrc/coreclr/vm/wasm/helpers.cpp Outdated
Comment threadsrc/coreclr/vm/cgensys.h
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.cpp
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 24, 2026 23:01

CopilotAI 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 12 out of 12 changed files in this pull request and generated 7 comments.

Comment threadsrc/coreclr/vm/wasm/helpers.cpp Outdated
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
@AndyAyersMS

Copy link
Copy Markdown
Member

Have been testing this locally and it looks good from that standpoint. Still some possibly related failures but it will take me a bit longer to isolate those.

@AndyAyersMSAndyAyersMS left a comment

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.

Changes look reasonable to me, but I confess I didn't dig through all the wasm in detail. From a testing standpoint things are definitely better.

Comment threadsrc/coreclr/vm/prestub.cpp
Comment threadsrc/coreclr/vm/prestub.cpp Outdated
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.cpp
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.cpp
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.cpp
@pavelsavarapavelsavara added the arch-wasm WebAssembly architecture label Jun 26, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara
See info in area-owners.md if you want to be subscribed.

@AndyAyersMS

Copy link
Copy Markdown
Member

Remaining generics-related Pri-1 failures look like a jit issue, we are not passing the instantiation parameter for some calls. PR for this up soon.

CopilotAI review requested due to automatic review settings June 26, 2026 18:59

CopilotAI 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 13 out of 13 changed files in this pull request and generated 7 comments.

Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.S
Comment threadsrc/coreclr/vm/wasm/dynamichelpers.cpp

@AndyAyersMSAndyAyersMS left a comment

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.

re-approving

@davidwrighton

Copy link
Copy Markdown
MemberAuthor

/ba-g Infra and unrelated failures

@davidwrighton
davidwrighton merged commit 0bc7eda into dotnet:mainJun 26, 2026
170 of 173 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-preview7 milestone Jun 28, 2026
eiriktsarpalis pushed a commit that referenced this pull request Jul 15, 2026
Add support for generic lookups to Wasm R2R code by following the
DynamicHelper pattern we use on other platforms. This looks like a quite
suitable implementation for Wasm, so I expect that this will remain for
the long term.
Add support for WebAssembly .S files, as it was difficult to implement
the helpers in inline assembly due to clang limitations.
Tweak Crossgen2 into generating uses of the DynamicHelper logic.
Also add support for precomputed thunks for various runtime helpers
which were missing them, and add an assert that will fire even in
interpreted runs for missing cases.
Fixes (most) of #129622Fixes#129821
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 29, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasmWebAssembly architecturearea-VM-coreclr

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[wasm][coreCLR] R2R generic dictionary lookup traps with null function (zero import cell for GenericLookupSignature)

5 participants

@davidwrighton@AndyAyersMS@AaronRobinsonMSFT@pavelsavara