Skip to content

[R2R] Expand discovery of non-generic virtual methods on generic types - #128652

Merged
BrzVlad merged 5 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged

[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom
BrzVlad:feature-r2r-generics-non-gvm

Conversation

@BrzVlad

@BrzVladBrzVlad commented May 27, 2026•
edited
Loading

Copy link
Copy Markdown
Member

We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.

This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.

Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7

Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI review requested due to automatic review settings May 27, 2026 15:48
@github-actionsgithub-actionsBot added the area-crossgen2-coreclr only use for closed issues label May 27, 2026
@BrzVlad

BrzVlad commented May 27, 2026

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

azure-pipelinesBot commented May 27, 2026

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

CopilotAI reviewed May 27, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).

Changes:

  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).

Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs Outdated
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs Outdated
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
This was referenced May 28, 2026
Open
Open
@BrzVlad
BrzVladforce-pushed the feature-r2r-generics-non-gvm branch from 68f23ac to f924912CompareJune 8, 2026 16:22
This was referenced Jun 8, 2026
Closed
Closed
BrzVlad added 2 commits June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI review requested due to automatic review settings June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed the feature-r2r-generics-non-gvm branch from f924912 to ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad

BrzVlad commented Jun 9, 2026

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines

azure-pipelinesBot commented Jun 9, 2026

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

CopilotAI reviewed Jun 9, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22

  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.

Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad marked this pull request as ready for review June 9, 2026 05:26
@build-analysisbuild-analysisBot mentioned this pull request Jun 9, 2026
Open
3 tasks
@davidwrighton
davidwrighton self-requested a review June 9, 2026 16:33
davidwrighton
davidwrighton requested changes Jun 11, 2026
View reviewed changes

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

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.

CopilotAI review requested due to automatic review settings June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed the feature-r2r-generics-non-gvm branch from 5db44ea to c22b438CompareJune 12, 2026 19:39
@BrzVlad

BrzVlad commented Jun 12, 2026

Copy link
Copy Markdown
MemberAuthor

Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.

Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?

CopilotAI reviewed Jun 12, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22

  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).

Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs Outdated
This was referenced Jun 12, 2026
Closed
Closed
@BrzVlad

BrzVlad commented Jun 17, 2026

Copy link
Copy Markdown
MemberAuthor

@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?

MichalStrehovsky
MichalStrehovsky reviewed Jun 22, 2026
View reviewed changes

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

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad

BrzVlad commented Jun 23, 2026

Copy link
Copy Markdown
MemberAuthor

Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.

This needs reapproval since I pushed after review.

@build-analysisbuild-analysisBot mentioned this pull request Jun 23, 2026
Closed
CopilotAI review requested due to automatic review settings June 23, 2026 18:55
CopilotAI reviewed Jun 23, 2026
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22

  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).

Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad merged commit 59bffd4 into dotnet:mainJun 25, 2026
112 checks passed
@dotnet-maestrodotnet-maestroBot mentioned this pull request Jun 26, 2026
Merged
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-preview7 milestone Jun 26, 2026
eiriktsarpalis pushed a commit that referenced this pull request Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects

None yet

Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
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;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
[R2R] Expand discovery of non-generic virtual methods on generic types by BrzVlad · Pull Request #128652 · dotnet/runtime · GitHub
Skip to content
[R2R] Expand discovery of non-generic virtual methods on generic types - #128652
Merged
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged
[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
May 27, 2026•

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[R2R] Expand discovery of non-generic virtual methods on generic types by BrzVlad · Pull Request #128652 · dotnet/runtime · GitHub
Skip to content
[R2R] Expand discovery of non-generic virtual methods on generic types - #128652
Merged
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged
[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
May 27, 2026•

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[R2R] Expand discovery of non-generic virtual methods on generic types by BrzVlad · Pull Request #128652 · dotnet/runtime · GitHub
Skip to content
[R2R] Expand discovery of non-generic virtual methods on generic types - #128652
Merged
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged
[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
May 27, 2026•

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
[R2R] Expand discovery of non-generic virtual methods on generic types by BrzVlad · Pull Request #128652 · dotnet/runtime · GitHub
Skip to content
[R2R] Expand discovery of non-generic virtual methods on generic types - #128652
Merged
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged
[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
May 27, 2026•

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[R2R] Expand discovery of non-generic virtual methods on generic types by BrzVlad · Pull Request #128652 · dotnet/runtime · GitHub
Skip to content
[R2R] Expand discovery of non-generic virtual methods on generic types - #128652
Merged
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged
[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
May 27, 2026•

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
[R2R] Expand discovery of non-generic virtual methods on generic types by BrzVlad · Pull Request #128652 · dotnet/runtime · GitHub
Skip to content
[R2R] Expand discovery of non-generic virtual methods on generic types - #128652
Merged
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Jun 25, 2026
Merged
[R2R] Expand discovery of non-generic virtual methods on generic types#128652
BrzVlad merged 5 commits into
dotnet:mainfrom 
BrzVlad:feature-r2r-generics-non-gvm
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
May 27, 2026•

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky

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

edited

Loading

Copy link


Copy Markdown

Member
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests: https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()

CopilotAI
review requested due to automatic review settings
May 27, 2026 15:48
@github-actionsgithub-actionsBot
added
the 
area-crossgen2-coreclr
only use for closed issues
label
May 27, 2026
@BrzVlad
BrzVlad
commented
May 27, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
May 27, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
May 27, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
This PR extends crossgen2/ReadyToRun virtual-dispatch dependency discovery so that non-generic virtual method slots on generic type instantiations can trigger compilation of the correct concrete implementations (in addition to the existing GVM (generic virtual method) mechanism).
Changes:
  • Introduces a new VirtualMethodUseNode marker and a corresponding cache on NodeFactory.
  • Marks non-GVM virtual slots as “used” from MethodFixupSignature for VirtualEntry fixups.
  • Adds conditional static dependencies to InheritedVirtualMethodsNode to compile per-type implementations when a corresponding slot is marked used, and broadens type discovery in TypeFixupSignature to enqueue the node.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
FileDescription
src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csprojAdds the new VirtualMethodUseNode source file to the project.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/VirtualMethodUseNode.csNew dependency-analysis marker node representing a used non-GVM virtual slot.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.csAdds a node cache + factory method for VirtualMethodUseNode.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.csExpands type discovery to add InheritedVirtualMethodsNode when virtual slots/interfaces are present.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.csMarks non-GVM virtual slots as used for VirtualEntry fixups to enable conditional compilation.
src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.csAdds conditional static dependencies to compile implementations based on VirtualMethodUseNode conditions (class + interface paths).
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated

Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs

This was referenced May 28, 2026

Open


Open

@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
68f23ac to
f924912CompareJune 8, 2026 16:22

This was referenced Jun 8, 2026

Closed


Closed

BrzVlad
added 2 commits
June 9, 2026 08:01
@BrzVlad
[R2R] Expand discovery of non-generic virtual methods on generic types
bc2314a
We determine the full set of used generic methods by tracking all types from the app via InheritedVirtualMethodsNode and GVM callsites via GVMDependencies node. The GVMDependencies node then tries to resolve the method on all types from the app, which are a dynamic dependency for this node. This only handles generic methods so, non-generic methods on generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional dependencies to InheritedVirtualMethodsNode. For a generic type, we determine all virtual/interface method slots and the resolved target method. Then we add a conditional dependency that says that the resolved method is included if the slot defining method is used in the app. We detect if the slot defining method is used by creating a VirtualMethodUseNode at callsites.
@BrzVlad
Avoid code duplication of VirtualMethodUseNode
ff3d0a6
It serves the same purpose both for R2R and NativeAOT
CopilotAI
review requested due to automatic review settings
June 9, 2026 05:01
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
f924912 to
ff3d0a6CompareJune 9, 2026 05:01
@BrzVlad
BrzVlad
commented
Jun 9, 2026

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-coreclr crossgen2 outerloop
@azure-pipelines
azure-pipelinesBot
commented
Jun 9, 2026

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
CopilotAI
reviewed
Jun 9, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • Changing VirtualMethodUseNode to public introduces new public surface area in compiler infrastructure. Unless this is intentionally a supported API, it should stay internal (and ideally be hidden behind a base return type) to avoid committing to it long-term and to avoid requiring API-approval tracking.
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
@BrzVlad
BrzVlad
marked this pull request as ready for review
June 9, 2026 05:26
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 9, 2026

Open

3 tasks
@davidwrighton
davidwrighton
self-requested a review
June 9, 2026 16:33
davidwrighton
davidwrighton
requested changes
Jun 11, 2026
View reviewed changes
@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.

I like the work, and it looks correct, but we need to utilize the test framework in ILCompiler.ReadyToRun.Tests to make the change long-term robust especially if we find that this regresses crossgen2 performance in some way by adding so many new dependencies. You've written a nice test file, which should be checked into ILCompiler.ReadyToRun.Tests and should have validation that the expected methods are being compiled. Talk to @jtschuster for details on how to use that test framework if you have questions.
CopilotAI
review requested due to automatic review settings
June 12, 2026 19:39
@BrzVlad
BrzVladforce-pushed
the
feature-r2r-generics-non-gvm
branch
from
5db44ea to
c22b438CompareJune 12, 2026 19:39
@BrzVlad
BrzVlad
commented
Jun 12, 2026

Copy link


Copy Markdown

MemberAuthor
Added a set of tests, checking that the relevant instantiations are being compiled. I've added their GVM counterpart as well, since my other PR didn't add tests like these.
Note that for size/compilation time we have some pipelines on mobile, testing some sample apps. Not sure if there is something else that tracks this for r2r on desktop ?
CopilotAI
reviewed
Jun 12, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • VirtualMethodUseNode was changed from internal to public, which introduces new public API surface in the ILCompiler toolset. This PR doesn't link an api-approved issue, so this should stay non-public (or go through API review).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Outdated


This was referenced Jun 12, 2026

Closed


Closed

@BrzVlad
BrzVlad
commented
Jun 17, 2026

Copy link


Copy Markdown

MemberAuthor
@davidwrighton@MichalStrehovsky Any thoughts so we can move forward with this ?
MichalStrehovsky
MichalStrehovsky
reviewed
Jun 22, 2026
View reviewed changes
@MichalStrehovskyMichalStrehovsky
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.

Product part LGTM, but David should have a look since I haven't looked at the ILCompiler.ReadyToRun.Tests part.
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
@BrzVlad
BrzVlad
commented
Jun 23, 2026

Copy link


Copy Markdown

MemberAuthor
Did a final validation of change before merging. For full framework composite r2r, size increase is 2% with compilation time increase of 10%. Given the compilation type was more than I was expecting, I brought back the TypeHasGVMSlots filter so that these newly added nodes don't take part in dynamic dependency analysis. This reduced the r2r compilation regression from 10% to 5% which is more reasonable.
This needs reapproval since I pushed after review.
@build-analysisbuild-analysisBot
mentioned this pull request
Jun 23, 2026

Closed

CopilotAI
review requested due to automatic review settings
June 23, 2026 18:55
CopilotAI
reviewed
Jun 23, 2026
View reviewed changes
CopilotAI
left a comment


Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/Compiler/DependencyAnalysis/VirtualMethodUseNode.cs:22
  • This changes VirtualMethodUseNode from internal to public, which introduces new public API surface in the tool assemblies. Per repo policy, new public APIs require an api-approved issue; alternatively, keep this type internal and avoid exposing it from public APIs (e.g., have NodeFactory.VirtualMethodUse return a base type like DependencyNodeCore).
Comment thread.../tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunCodegenNodeFactory.cs
Comment thread...ols/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
Comment thread...r/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/InheritedVirtualMethodsNode.cs
Comment thread...tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
Comment threadsrc/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/R2RResultChecker.cs
@BrzVlad
BrzVlad
merged commit 59bffd4
into
dotnet:mainJun 25, 2026

112 checks passed

@dotnet-maestrodotnet-maestroBot
mentioned this pull request
Jun 26, 2026
 Merged

@dotnet-milestone-botdotnet-milestone-botBot
added this to the 11.0-preview7 milestone
Jun 26, 2026
eiriktsarpalis pushed a commit
that referenced
this pull request
Jul 15, 2026
@BrzVlad@eiriktsarpalis
[R2R] Expand discovery of non-generic virtual methods on generic types (
6797186
#128652)
We determine the full set of used generic methods by tracking all types
from the app via InheritedVirtualMethodsNode and GVM callsites via
GVMDependencies node. The GVMDependencies node then tries to resolve the
method on all types from the app, which are a dynamic dependency for
this node. This only handles generic methods so, non-generic methods on
generic types are not resolved with this mechansim.
This PR completes the support. This is done by adding conditional
dependencies to InheritedVirtualMethodsNode. For a generic type, we
determine all virtual/interface method slots and the resolved target
method. Then we add a conditional dependency that says that the resolved
method is included if the slot defining method is used in the app. We
detect if the slot defining method is used by creating a
VirtualMethodUseNode at callsites.
Methods failing to compile before, from this set of tests:
https://gist.github.com/BrzVlad/7ea5987ec494705ac7b4b6aaf1325fe7
```
Test1Base`1[int]:Test1Method()
Test2C`1[int]:Test2Method()
ITest3WithDim`1[int]:ITest3Base.Test3Method()
Test4A`1[int]:ITest4<T>.Test4Method()
Test5B`1[int]:Test5Method()
Test6B`1[int]:Test6Method()
ITest7`1[int]:Test7Method()
```
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 27, 2026
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@davidwrightondavidwrightondavidwrighton approved these changes

Assignees

@BrzVladBrzVlad

Labels

area-crossgen2-coreclronly use for closed issues

Projects


None yet


Milestone

11.0-preview7

Development

Successfully merging this pull request may close these issues.

4 participants

@BrzVlad@davidwrighton@MichalStrehovsky