Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Include unboxing stubs in r2r images - #132787

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs
Sep 2, 2026
Merged

Include unboxing stubs in r2r images#132787
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-unbox-stubs

Conversation

@BrzVlad

@BrzVladBrzVlad commented Aug 26, 2026

Copy link
Copy Markdown
Member

We want to include unbox stubs in the R2R image, so they are not interpreted on ios/wasm. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for methods on valuetypes that can be called through an interface. These unbox stubs are included in the InstanceMethodEntryPoints table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.

NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).

Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.

This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical.

CopilotAI lite review requested due to automatic review settings August 26, 2026 15:08
@BrzVlad
BrzVlad requested review from MichalStrehovsky and davidwrighton and removed request for MichalStrehovsky and CopilotAugust 26, 2026 15:10
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@hez2010

hez2010 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

With this change will we be able to remove the bail-out here?

#if READYTORUN
if(unboxingStub)
{
// We need an unboxing stub that points to an instantiated method but this is not happening in R2R.
info->detail=CORINFO_DEVIRTUALIZATION_DETAIL.CORINFO_DEVIRTUALIZATION_FAILED_CANON;
returnfalse;
}

@davidwrightondavidwrighton 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.

Holistic Review

Motivation: Precompiling unboxing stubs is justified where the runtime cannot generate a usable frameless shuffle thunk, particularly on targets without runtime code generation.

Approach: The current dependency analysis is too broad because it emits a complete R2R method for every reachable unboxing stub, including common cases already handled by a much smaller runtime-generated shuffle thunk.

Summary: ⚠️ Needs Changes. Please restrict precompiled unboxing stubs to targets without runtime code generation and signatures that the runtime cannot adapt with its frameless shuffle-thunk machinery.


Detailed Findings

❌ Emission policy — Avoid precompiling stubs already handled by shuffle thunks

I don't think we should precompile every reachable unboxing stub. On targets that support runtime code generation, the common case is already handled by a much smaller frameless shuffle thunk. Emitting a complete R2R method for those cases increases image size without providing a clear benefit.

I think we should restrict precompiled unboxing stubs to two cases:

  1. The target cannot generate code at runtime. Please use the existing ReadyToRunCompilerContext.TargetAllowsRuntimeCodeGeneration property rather than duplicating the iOS, MacCatalyst, tvOS, Browser, WASI, and Wasm target checks.
  2. The runtime cannot represent the required adaptation as a frameless shuffle thunk. In particular, an instantiating unboxing stub cannot use the current shuffle machinery when adding the hidden instantiation argument requires moving or introducing an argument on the stack.

Crossgen2 should be able to approximate the second condition using its existing ArgIterator. Build the source layout as an unboxing stub, where the hidden instantiation argument is suppressed, and the destination layout as the underlying target method, where that argument is present. If the layouts have different stack-area sizes, or an argument's stack location changes, the runtime would need a framed stub and we should emit the full R2R method.

x86 must bypass this ArgIterator stack-move approximation. Its arguments are generally stack-based, but MakeUnboxingStubWorker uses the architecture-specific EmitUnboxMethodStub, which supports this calling convention without falling back to an IL stub. Accordingly, CanGenerateRuntimeShuffleThunk should return true for x86 before examining argument locations.

I suggest separating "can compile" from "should precompile" so the predicate polarity remains clear:

privatestaticboolCanCompileUnboxingStub(MethodDescmethod){return!method.RequiresInstMethodDescArg()&&!method.IsAsyncCall();}privatestaticboolShouldPrecompileUnboxingStub(MethodDescmethod){if(!CanCompileUnboxingStub(method))returnfalse;ReadyToRunCompilerContextcontext=(ReadyToRunCompilerContext)method.Context;return!context.TargetAllowsRuntimeCodeGeneration||!CanGenerateRuntimeShuffleThunk(method);}privatestaticboolCanGenerateRuntimeShuffleThunk(MethodDescmethod){// The ordinary unboxing stub only adjusts 'this' and tail-jumps.if(!method.RequiresInstMethodTableArg())returntrue;// x86 has a specialized implementation that supports its stack-based ABI.if(method.Context.Target.Architecture==TargetArchitecture.X86)returntrue;(ArgIterator<TypeHandle>source,TransitionBlocktransitionBlock)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:true);(ArgIterator<TypeHandle>destination,_)=GCRefMapBuilder.BuildArgIterator(method.Signature,method.Context,methodRequiresInstArg:method.RequiresInstArg(),isUnboxingStub:false);// GenerateShuffleArrayPortable rejects an instantiating shuffle when// the source and destination stack sizes differ.if(source.SizeOfFrameArgumentArray()!=destination.SizeOfFrameArgumentArray())returnfalse;while(true){intsourceOffset=source.GetNextOffset();intdestinationOffset=destination.GetNextOffset();Debug.Assert((sourceOffset==TransitionBlock.InvalidOffset)==(destinationOffset==TransitionBlock.InvalidOffset));if(sourceOffset==TransitionBlock.InvalidOffset)returntrue;ArgLocDesc?sourceLocation=source.GetArgLoc(sourceOffset);ArgLocDesc?destinationLocation=destination.GetArgLoc(destinationOffset);if(StackLocationChanged(transitionBlock,sourceOffset,sourceLocation,destinationOffset,destinationLocation)){returnfalse;}}staticboolStackLocationChanged(TransitionBlocktransitionBlock,intsourceOffset,ArgLocDesc?sourceLocation,intdestinationOffset,ArgLocDesc?destinationLocation){boolsourceUsesStack=transitionBlock.IsStackArgumentOffset(sourceOffset)||sourceLocationis{m_byteStackSize:>0};booldestinationUsesStack=transitionBlock.IsStackArgumentOffset(destinationOffset)||destinationLocationis{m_byteStackSize:>0};if(sourceUsesStack!=destinationUsesStack)returntrue;if(!sourceUsesStack)returnfalse;// GetArgLoc describes arguments split between registers and the stack.// If either side has such a description, conservatively require the// stack portions to be identical.if(sourceLocation.HasValue||destinationLocation.HasValue){return!sourceLocation.HasValue||!destinationLocation.HasValue||sourceLocation.Value.m_byteStackIndex!=destinationLocation.Value.m_byteStackIndex||sourceLocation.Value.m_byteStackSize!=destinationLocation.Value.m_byteStackSize;}returnsourceOffset!=destinationOffset;}}

The important behavior is:

  • CanGenerateRuntimeShuffleThunk returns false when a stack move is needed.
  • ShouldPrecompileUnboxingStub consequently returns true for that method.
  • Ordinary all-register unboxing stubs remain runtime-generated on JIT-capable targets.
  • x86 uses its specialized runtime stub instead of being classified by its normal stack-based argument convention.

Please also add coverage showing that:

  • A target without runtime code generation emits an otherwise ordinary unboxing stub.
  • A target with runtime code generation does not emit an ordinary/all-register unboxing stub.
  • A target with runtime code generation emits an instantiating unboxing stub when adding the hidden context changes the stack layout.

Note

This review was created by GitHub Copilot.

Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI lite review requested due to automatic review settings August 28, 2026 12:09
@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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 change teaches ReadyToRun (R2R) to precompile and persist unboxing stubs in the R2R image, and updates the runtime lookup logic to distinguish unboxing-stub entries from “normal” entries even when they otherwise share the same signature shape. The PR also bumps the R2R minor version (27.1) and adds R2R test coverage for value-type interface/virtual dispatch scenarios that require unboxing stubs.

Changes:

  • Runtime: extend signature matching and entrypoint selection so unboxing stubs are stored/loaded via InstanceMethodEntryPoints and matched using ENCODE_METHOD_SIG_UnboxingStub.
  • Crossgen2/R2R compiler: generate and root unboxing thunk IL stubs, encode them with the unboxing bit in the signature, and key the instance entrypoint table in a way the runtime can probe.
  • Tests/versioning: bump R2R minor version to 27.1 and add ReadyToRun tests that validate unboxing thunk presence (and runtime-function emission in a GVM case).

Reviewed changes

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

Show a summary per file
FileDescription
src/coreclr/vm/readytoruninfo.cppMakes SigMatchesMethodDesc validate the unboxing-stub flag and routes unboxing stubs through the instance-method entrypoint table lookup.
src/coreclr/vm/prestub.cppFor unboxing stubs, prefers GetPrecompiledR2RCode over runtime stub generation when R2R code is available.
src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.csBumps managed R2R header minor version to 27.1.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/GVMDependenciesNode.csAdds unboxing-stub dependencies for interface GVM scenarios under #if READYTORUN.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.csEnhances boxed-value-type and unboxing-thunk types (mangling/sorting, target resolution helpers, and READYTORUN-specific IL emission details).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/MethodDescExtensions.csTreats compiler-generated unboxing thunks as secondary MethodDescs and maps them back to the “primary” target MethodDesc for metadata identity.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.csAdjusts version-bubble and shared-generic token logic to use the target method when compiling from an unboxing thunk.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojLinks shared boxed-types/unboxing-thunk implementation and INonEmittableType into the R2R compiler build.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/IL/ReadyToRunILProvider.csSupplies IL for unboxing-thunk stubs via ILStubMethod.EmitIL() when needed.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTableManager.csEnsures unboxing thunks are tracked among methods requiring the “instantiated/instance entrypoint table” treatment.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.csRoots unboxing stubs where appropriate and ensures generated-IL tokens are available for unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds policy for when to precompile unboxing stubs and a factory method to materialize the correct thunk (generic vs non-generic).
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.csEncodes the unboxing bit into method signatures and hashes unboxing thunks by their target so the runtime can probe correctly.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional dependencies to include unboxing stubs for value-type virtual/interface dispatch cases.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.csAdds helpers to assert presence/absence of compiled unboxing thunks in produced R2R images.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/NonGVM.csAdds NonGVM test cases exercising value-type interface and object virtual dispatch requiring unboxing thunks.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GVM.csAdds a value-type interface GVM case to validate unboxing thunk generation for GVM scenarios.
src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.csAsserts unboxing thunks are present for the new test cases and that GVM unboxing thunk has runtime functions.
src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csprojSwitches boxed-types implementation to the shared Common file (to share behavior with R2R).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Sorting.csRemoves boxed/unboxing thunk sorting partials (now handled in the shared boxed-types file).
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Mangling.csRemoves boxed/unboxing thunk mangling partials (now handled in the shared boxed-types file).
src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.hBumps native header minor version to 27.1 for NativeAOT runtime consumption.
src/coreclr/inc/readytorun.hBumps READYTORUN_MINOR_VERSION to 0x0001 and documents the 27.1 format change for unboxing stubs.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

@davidwrighton This seems like the only suspicious failure from CI, happens on wasm. Do you recall seeing this outside of my change ? https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_apis/build/builds/1572366/logs/539

I enabled these stubs unconditionally on desktop for my own development convenience and to get some CI testing for them. I was actually considering to switch them off completely on targets that allow jit. They don't contribute to further discoveries of dependencies, I believe they are quick to generate by the jit mechanisms at run-time and I suspect that the jit wouldn't be able to tier them up. So if we load suboptimal code for some unbox stubs, we are stuck with the r2r implementation, when the jit could have generated slightly more efficient code. Given also the detection of shuffle thunks usage seems non-trivial, I'm wondering if it would be an overall better solution to just keep these stubs on iOS/wasm for now.

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

cc @pavelsavara on the r2r failure question linked above

@pavelsavara

pavelsavara commented Aug 28, 2026

Copy link
Copy Markdown
Member

cc @pavelsavara on the r2r failure question linked above

I didn't see that one yet on my PRs

@pavelsavara

Copy link
Copy Markdown
Member

@davidwrighton

Copy link
Copy Markdown
Member

@BrzVlad, add a switch to enable building them all the time then, and update the test suite to pass that switch to the smoke test, and make sure that test has a stub which actually gets used.

@davidwrighton

Copy link
Copy Markdown
Member

Looks to me like there is a need to fix that build though or at least understand it more. Wasm has some places where it has found a few bugs that are much easier to hit on the wasm r2r that are also bugs for the normal crossgen platform in very unusual situations.

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws new InvalidOperationException() with no message in the default switch arm. If this is hit (e.g., due to a caller bug), the exception is not actionable and makes diagnosing signature/table mismatches much harder. Prefer throwing with a clear message that includes the unexpected method.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment threadsrc/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 04:54
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 266c5a1 to 6af3d64CompareSeptember 1, 2026 04:54

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • The default arm in this switch throws a parameterless InvalidOperationException. This makes failures harder to diagnose and goes against the repo guidance to avoid empty exceptions for unreachable paths. Since the switch should be exhaustive when IsUnboxingThunk(method) is true, prefer UnreachableException (or at least an InvalidOperationException with a message) so unexpected thunk types are actionable.
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

CopilotAI review requested due to automatic review settings September 1, 2026 12:13
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from 6af3d64 to ac4ce63CompareSeptember 1, 2026 12:13

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.

🔵 Needs a closer look

Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We want to include unboxing stubs into the R2R image to avoid their interpretation on platforms with no jit. For this we need to create a special method that inherits from ILStubMethod. This needs to have an EmitIL implementation (for the method body) and a MethodSignature so that the method can be encoded/decoded. This new stub method will be added during dependency analysis for virtual methods on valuetypes (methods that are either called through an interface or from normal virtual calls from Object). These unbox stubs are included in the `InstanceMethodEntryPoints` table where they are encoded based on the signature (together with the other methods that not uniquely identified by a method rid). When the runtime needs to load such a stub, it will build the right signature for the unbox stub and lookup in these tables.
NativeAot already has UnboxingThunk and GenericUnboxingThunk for this purpose. These are created per generic definition and r2r inflates them as needed. They are created as method residing on an artificial BoxedValueType, which represents also the first argument they receive. UnboxingThunk simply unboxes the object and then dispatches to the target method. Given the implementation is simple, NativeAOT actually uses some custom assembly thunks for this purpose (UnboxingStubNode), which we don't do yet on r2r to reduce the scope of the change. GenericUnboxingThunk handles the scenario where we need to obtain the method table instantiation from the boxed instance, so that we can pass it to the valuetype method (which expects it explicitly given it doesn't receive an object as this).
Shared generic methods are not yet handled. I believe the runtime currently expects unboxing thunks to not receive any method inst hidden arg, so additional changes might be needed there.
This bumps R2R version because an older runtime, whose SigMatchesMethodDesc doesn't check the unboxing bit, could incorrectly match a normal method with an unbox stub, due to the signatures being otherwise identical. Bumped minor version since major version was just bumped a few days ago and it seems redundant to do it again.
Add r2r option to override this behavior. We use this option in the R2R tests which are checking the presence of the unbox stubs, as well in new smoke tests where we validate that these unbox stubs also work correctly.
CopilotAI review requested due to automatic review settings September 1, 2026 14:10
@BrzVlad
BrzVladforce-pushed the feature-r2r-unbox-stubs branch from ac4ce63 to 2334d68CompareSeptember 1, 2026 14:10

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:353

  • Throwing an empty InvalidOperationException makes failures harder to diagnose (no actionable context about which MethodDesc was unexpected). This is an internal helper, but it can still surface during R2R compilation/debugging; include at least the offending method in the message.
  • Files reviewed: 29/29 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment threadsrc/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h
Comment threadsrc/coreclr/vm/readytoruninfo.cpp Outdated
CopilotAI review requested due to automatic review settings September 1, 2026 16:19

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.

🔵 Needs a closer look

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.BoxedTypes.cs:354

  • GetTargetOfUnboxingThunk throws InvalidOperationException() with no message for the non-thunk case. If this is ever hit (e.g., a caller forgets to guard with IsUnboxingThunk), it will be difficult to diagnose which method triggered it. Include at least the offending MethodDesc in the exception message.
  • Files reviewed: 31/31 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BrzVlad

Copy link
Copy Markdown
MemberAuthor

/ba-g com failure unrelated

@BrzVlad
BrzVlad merged commit 36ef186 into dotnet:mainSep 2, 2026
111 of 113 checks passed
@dotnet-milestone-botdotnet-milestone-botBot added this to the 12.0-preview1 milestone Sep 3, 2026
Sign up for freeto 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.

7 participants

@BrzVlad@hez2010@pavelsavara@davidwrighton@jkotas@MichalStrehovsky