[r2r] Include generic type instantiations from GenericLookupSignature for discovery of virtual methods#129882
Open
BrzVlad wants to merge 2 commits into
Open
[r2r] Include generic type instantiations from GenericLookupSignature for discovery of virtual methods#129882BrzVlad wants to merge 2 commits into
BrzVlad wants to merge 2 commits into
Conversation
… for discovery of virtual methods
Normally, we detect the usage of a type in the application via the TypeFixupSignature and then we use this type to determine which virtual methods on the type need compilation (via supporting GVMDependenciesNode and VirtualMethodUseNode).
Consider the following scenario:
```
public class TestA<T, U>
{
public virtual void TestMethod(T item) => Console.WriteLine($"{item} / {typeof(U)}");
}
public class TestB<T, U>
{
public TestA<T, U> TestCreate() => new TestA<T, U>();
}
TestB<string, int> obj = new TestB<string, int>();
obj.TestCreate().TestMethod("hello");
```
When compiling `TestB<Canon,int32>.TestCreate` we need to create an instance of `TestA<T,int32>`. Because we are compiling a shared version we need to embed a call to a generic lookup helper in order to obtain the actual T. This is done with a GenericLookupSignature fixup, which contains the type `TestA<Canon,int32>` that needs resolving. When creating this node, we add the InheritedVirtualMethodsNode as a dependency which facilitates discovery of virtual methods on this type.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ReadyToRun dependency analysis so that when a type handle is obtained via a GenericLookupSignature (generic dictionary lookup), the compiler still records the type usage needed to discover and compile inherited virtual methods for that instantiated type. It also adds a focused regression test covering the scenario.
Changes:
- Add
InheritedVirtualMethodsdependency tracking forGenericLookupSignaturewhen_fixupKind == ReadyToRunFixupKind.TypeHandle. - Add a new VirtualMethodGenerics test case that reaches a generic type instantiation only via a
GenericLookupSignaturefixup. - Extend
R2RTestSuiteswith a new[Fact]that validates the relevant virtual method is compiled.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs | Adds dependency analysis to record generic-lookup type usage for virtual dispatch discovery (mirrors TypeFixupSignature’s behavior for this scenario). |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/VirtualMethodGenerics/GenericLookup.cs | New embedded test-case source driving a generic instantiation through a GenericLookupSignature-only path. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCases/R2RTestSuites.cs | Adds an xUnit test validating TestA\2<__Canon,int>.TestMethod` is compiled as expected. |
Comment on lines
+141
to
+144
| // In shared generic code, newobj uses a generic dictionary lookup for the type handle | ||
| // rather than a direct READYTORUN_FIXUP_TypeHandle (TypeFixupSignature). Mirror the | ||
| // creation of InheritedVirtualMethodsNode as its done in TypeFixupSignature, so we | ||
| // scan the virtual methods on this type for dependency analysis. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Normally, we detect the usage of a type in the application via the TypeFixupSignature and then we use this type to determine which virtual methods on the type need compilation (via supporting GVMDependenciesNode and VirtualMethodUseNode).
Consider the following scenario:
When compiling
TestB<Canon,int32>.TestCreatewe need to create an instance ofTestA<T,int32>. Because we are compiling a shared version we need to embed a call to a generic lookup helper in order to obtain the actual T. This is done with a GenericLookupSignature fixup, which contains the typeTestA<Canon,int32>that needs resolving. When creating this node, we add the InheritedVirtualMethodsNode as a dependency which facilitates discovery of virtual methods on this type.Fixes perf on linq sorting, ex:
LinqBenchmarks.Order00LinqMethodX(~20x improvement)