Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dd9dfb0-13d5-4a8c-9419-3f808cbd9b8b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dd9dfb0-13d5-4a8c-9419-3f808cbd9b8b
…asm-r2r-delegate-lowering
…asm-r2r-delegate-lowering
…asm-r2r-delegate-lowering
…asm-r2r-delegate-lowering
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dd9dfb0-13d5-4a8c-9419-3f808cbd9b8b
…asm-r2r-delegate-lowering
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/crossgen-contrib |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three moderate review issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Enables browser-Wasm ReadyToRun delegate-constructor lowering with shared DC0/DC1 thunks and portable fixup metadata.
Changes:
- Adds Wasm fixup handling, constructor rooting, and interpreter-transition dependencies.
- Emits shared delegate-constructor thunks with GC argument tracking.
- Adds runtime, IL, and ReadyToRun image coverage.
Review findings:
- Moderate: Image tests do not verify the matching interpreter-thunk keys.
- Moderate: The fallback condition excludes open-static delegates with hidden return buffers.
- Moderate: The closed-static return-buffer test does not invoke the delegate.
| File | Description |
|---|---|
src/tests/readytorun/wasm/WasmInterpreterTransitions/WasmInterpreterTransitions.csproj |
Adds the IL helper project reference. |
src/tests/readytorun/wasm/WasmInterpreterTransitions/WasmInterpreterTransitions.cs |
Adds delegate runtime coverage. |
src/tests/readytorun/wasm/WasmInterpreterTransitions/OpenInstanceDelegateHelper.ilproj |
Defines the helper IL project. |
src/tests/readytorun/wasm/WasmInterpreterTransitions/OpenInstanceDelegateHelper.il |
Provides the open-instance delegate fixture. |
src/coreclr/vm/wasm/helpers.hpp |
Declares return-buffer classification. |
src/coreclr/vm/wasm/helpers.cpp |
Implements Wasm return-buffer classification. |
src/coreclr/vm/wasm/dynamichelpers.cpp |
Adds the Wasm delay-load bridge. |
src/coreclr/vm/prestub.cpp |
Resolves delegate-constructor fixups. |
src/coreclr/vm/precode_portable.hpp |
Defines portable constructor entry-point data. |
src/coreclr/vm/method.hpp |
Declares the constructor R2R publication helper. |
src/coreclr/vm/frames.cpp |
Tracks Wasm object argument roots. |
src/coreclr/vm/cgensys.h |
Declares the typed Wasm helper entry point. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj |
Includes the new compiler components. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunDelegateCtorRootProvider.cs |
Roots delegate constructors. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilationBuilder.cs |
Registers Wasm constructor roots. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs |
Caches constructor thunk nodes. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmImportThunkPortableEntrypoint.cs |
Connects delegate fixups to shared thunks. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/WasmDelegateCtorThunkNode.cs |
Emits DC0 and DC1 Wasm thunks. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/Webcil/WasmDelegateConstructors.cs |
Adds ReadyToRun image fixtures. |
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs |
Validates delegate fixups and thunks. |
src/coreclr/jit/flowgraph.cpp |
Enables the Wasm R2R optimization. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dd9dfb0-13d5-4a8c-9419-3f808cbd9b8b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dd9dfb0-13d5-4a8c-9419-3f808cbd9b8b
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Virtual targets may be incorrectly routed to CtorClosed, and coverage for excluded generic/value-type shapes is incomplete.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
Resolved since last review (1)
| if (!targetMethod.Signature.IsStatic && | ||
| !targetMethod.OwningType.IsValueType && | ||
| !targetMethod.HasInstantiation && | ||
| !targetMethod.OwningType.HasInstantiation && | ||
| delegateInvoke.Signature.Length == targetMethod.Signature.Length) |
| // R2R delegate construction calls CtorClosed directly for bounded closed-instance shapes | ||
| // and uses the general DelegateConstruct helper for open, static, generic, and fallback shapes. |
davidwrighton
left a comment
There was a problem hiding this comment.
Lets use an R2R helper for this case. We'll need to modify the jit interface, but that's ok.
| !targetMethod.OwningType.HasInstantiation && | ||
| delegateInvoke.Signature.Length == targetMethod.Signature.Length) | ||
| { | ||
| delegateCtor = systemDelegate.GetKnownMethod("CtorClosed"u8, null); |
There was a problem hiding this comment.
Returning a MethodDesc to a method in another module shouldn't actually be working. If that IS, its a bug. Notably, it probably breaks the split composite scenario, and should be breaking scenarios where we don't enable any form of cross module inlining. Instead, we should be treating this as a R2R helper.
There was a problem hiding this comment.
Agreed — returning a CoreLib MethodDesc from GetDelegateCtor here is wrong, and the manifest-token workaround only happens to work in the tested configurations. Before implementing, I'd like to confirm the shape:
-
Two new JIT helpers bound to the existing managed implementations (the
DYNAMICJITHELPER(..., METHOD__...)pattern):CORINFO_HELP_DELEGATE_CTOR_CLOSED→Delegate.CtorClosedCORINFO_HELP_DELEGATE_CTOR→Delegate.DelegateConstruct
Both keep the
(this, target, methodPtr)shape, so no shuffle thunk or custom ABI is involved. R2R code reaches them via newREADYTORUN_HELPER_*IDs (R2R minor version bump). -
JIT-EE change: a new callback, e.g.
CorInfoHelpFunc getDelegateCtorHelper(CORINFO_CLASS_HANDLE delegateType, CORINFO_METHOD_HANDLE targetMethod), which the JIT uses on Wasm R2R to emit a helper call instead of retargeting the call to another method. Crossgen2 selects..._CLOSEDfor non-generic closed reference-instance targets and the general helper otherwise; the VM implementation can map fromCOMDelegate::GetDelegateCtor. -
Cleanup: remove the private-token pre-creation, the Crossgen
GetDelegateCtorimplementation, and the entrypoint refresh. Keep rootingCtorClosed/DelegateConstructin CoreLib so the helpers bind to compiled code. -
Tests: add split-composite and no-cross-module-inlining coverage in addition to the existing image and Node/GC-stress tests.
Open questions:
- Would you rather extend an existing callback (for example, have
getReadyToRunDelegateCtorHelperreturn a helper ID) than add a new one? - Is it fine to scope this first step to
CtorClosed+DelegateConstruct, and leave an open-delegate helper (CtorOpenwith the shuffle thunk passed explicitly) as a follow-up for the remaining open-static gap?
Note
This comment was generated with GitHub Copilot.
| private CORINFO_METHOD_STRUCT_* GetDelegateCtor(CORINFO_METHOD_STRUCT_* methHnd, CORINFO_CLASS_STRUCT_* clsHnd, CORINFO_METHOD_STRUCT_* targetMethodHnd, ref DelegateCtorArgs pCtorData) | ||
| { throw new NotImplementedException("GetDelegateCtor"); } | ||
| { | ||
| #if READYTORUN |
There was a problem hiding this comment.
I think all such cases use the partial files here instead of #if.



Summary
READYTORUN_FIXUP_DelegateCtorSystem.Delegate.CtorClosedhelper directly for safe closed reference-instance shapesSystem.Delegate.DelegateConstructhelper for open, static, generic, value-type, virtual, collectible, and other fallback shapesDC0/DC1thunks, delegate portable-entrypoint data, delay-load helper, runtime fixup handling, and Wasm retbuf classifier added by the earlier prototypeDesign
Wasm R2R now falls through to the existing
GetDelegateCtortransformation used by the normal JIT. Crossgen2 implements that callback with a bounded helper selection:CtorClosedwhen the target is a non-static reference-instance method, neither the method nor owner is generic, and the delegate/target explicit arities matchDelegateConstructotherwiseThe call's entrypoint is recomputed after replacing
gtCallMethHnd, avoiding the stale original-constructor entrypoint. The private CoreLib helper references are added to the R2R manifest before metadata token creation is frozen.This deliberately keeps the open/static path on the fully general helper. Selecting
CtorOpendirectly still requires a regular-helper contract for the shuffle thunk and collectible lifetime data; that can be evaluated independently without reintroducing Crossgen-specific constructor ABI structures.Validation
Passed locally on macOS arm64:
clr+libs+host+packsRelease buildDelegateCtorfixups and ordinary imports for bothDelegateConstructandCtorClosedDOTNET_GCStress=0xCgit diff --checkClosed-static retbuf invocation remains #134108's adapter responsibility; this change validates its construction and preserves the existing slow helper behavior.
Performance
100,000 observable constructions, 25 samples per shape:
Allocations remain 2.8 MB per 100,000 constructions.
The layering improvement has a measurable open-static cost versus the specialized
DC1 -> CtorOpenprototype: the direct-helper path is 3.192 ms slower per 100,000 constructions (about 31.9 ns per construction), a 60.2% time increase. Equivalently, the specialized thunk is about 1.60x faster. The direct path still improves open-static construction by approximately 5.1x over the original interpreted-constructor behavior.Closed-instance performance is fully preserved because the new design calls the small
CtorClosedhelper directly: it is 0.174 ms per 100,000 constructions (5.3%) faster than the specializedDC0result and approximately 14.8x faster than the original behavior.Recovering the remaining open-static difference would require a regular
CtorOpenhelper contract that supplies the shuffle-thunk and collectible-lifetime inputs without putting constructor ABI details back into Crossgen.Size
The current
System.Private.CoreLib.wasmis 28,498,503 bytes. The last custom-thunk build measured 28,508,845 bytes; this is 10,342 bytes smaller, though the branch incorporated main updates between those two builds, so this is directional rather than a controlled A/B.Related work
Resolves #134564
Note
This pull request description was generated with GitHub Copilot.