JIT: Devirtualize shared generic virtual methods#123323
JIT: Devirtualize shared generic virtual methods#123323hez2010 wants to merge 53 commits intodotnet:mainfrom
Conversation
github-actions
bot
added
the
area-CodeGen-coreclr
dotnet-policy-service
bot
added
the
community-contribution
There was a problem hiding this comment.
Pull request overview
This PR enables JIT devirtualization for shared generic virtual methods (GVM) that don't require runtime lookups. Previously, all shared GVMs were blocked from devirtualization due to concerns about having the right generic context. This change unblocks devirtualization when the instantiating stub doesn't need a runtime lookup, by checking for the presence of a GT_RUNTIMELOOKUP node before proceeding.
Changes:
- Introduced
needsMethodContextflag to track when a method context is needed for devirtualization - For shared generic methods, obtain the instantiating stub and set
needsMethodContext = true - Unified handling of array interface and generic virtual method devirtualization paths
- Added runtime lookup check in the JIT to bail out when a lookup is needed but context is unavailable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/vm/jitinterface.cpp | Added logic to detect shared generic methods and obtain instantiating stubs, unified array and GVM devirtualization handling |
| src/coreclr/jit/importercalls.cpp | Updated assertions to allow GVM in AOT scenarios, added runtime lookup check to prevent devirtualization when context is unavailable |
Co-authored-by: Copilot <[email protected]>
|
Failures seem to be caused by missing context during spmi replay. Otherwise all tests are passing. |
hez2010
changed the title
hez2010
force-pushed
the
gvm-devirt-shared
branch
from
bf5b09a to
c8b7f69
Compare
src/coreclr/jit/importercalls.cpp
Outdated
| GenTree* instParam = | ||
| getLookupTree(lookupToken, &dvInfo.instParamLookup, GTF_ICON_METHOD_HDL, compileTimeHandle); |
There was a problem hiding this comment.
lookupToken is unused by getLookupTree. I have missed removing it in my change. Can you remove it here and clean up the uses?
|
The output of this program is wrong on coreclr with this PR: using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
public class Program
{
public static void Main()
{
Test<string>();
}
[MethodImpl(MethodImplOptions.NoInlining)]
private static void Test<T>()
{
Derived<T> foo = new();
foo.Test<object>();
}
public class Base<T>
{
public virtual void Test<U>()
{
Console.WriteLine(typeof(T).FullName);
Console.WriteLine(typeof(U).FullName);
}
}
public class Derived<T> : Base<List<T>>
{
}
}I am not sure that using the token that we get from the base will work. Under any circumstances it is not the right thing to instantiate the base class type parameters with. Some changes to the signature will be needed to apply the proper modifications coming from the inheritance hierarchy. |
|
The VM uses a concept called Substitution to make that sort of thing work, but that's at the metadata level, and not typically directly used in this portion of the runtime. For overrides is typically done by starting from an actual |
|
Notably, to make this all work, you may need to encode 2 different types in the as part of |
|
I'm also not entirely sure which scenarios you are trying to optimize here. The nicest examples I see above are entirely removing the virtual dispatch, but I can imagine scenarios where you could be using devirtualization to remove just the virtual lookup operation. This isn't commonly all that useful on regular virtual dispatch, or even in interface dispatch as the dispatch mechanisms are actually quite fast, but in GVM dispatch there is a hashtable involved, which could be removed to somewhat good effect. |
Yeah. I'm not actually sure how we can make this work. We won't have the exact object type available during either The only thing I can think of is a special version of the runtime lookup helper that extracts the exact type from the object and then finds the overriding method table that way. Maybe I am missing something. |
|
I could see something like... there is an explicit call to newobj in the method being compiler followed by a use of a GVM on that object. In that sort of situation, you could use the token associated with the newobj to define the exact type and then work from there. Similarly, if there is a type which is sealed in a signature, you could do similar tricks, even if you couldn't see the exact type. This is why I'm trying to figure out which scenarios are expected to be optimizable with this change. |
|
The hope would be that most GVMs where we know the (possibly shared) exact type would be optimizable by devirtualization. We should be able to compute the exact target and hence avoid the virtual resolution and even do inlining of it, but it seems like computing the instantiation argument is very difficult when the JIT does not know the unshared exact type. (In many cases we know exact unshared types, e.g. under guarded devirtualization that would be the case.) Perhaps this PR should should focus on the exact unshared types to start out with. Obviously computing the instantiation argument for those is much simpler since no runtime lookup is required. |
|
Thanks for the explanation. I'm going to remove the part that tries to compute the runtime lookup and instead only focus on cases where we know the exact type in this PR. |
The scenario I was thinking about is not the lookup itself. It's that by turning the GVM call into a directly call, it enables the JIT to inline the callee despite it still needs a runtime lookup. |
|
I think it is impossible in general to represent the instantiation argument as a runtime lookup because the JIT does not guarantee that exact shared types have unique instantiations. Consider using System;
using System.Runtime.CompilerServices;
public class Program
{
public static void Main()
{
CallTest<string, object>(true);
}
[MethodImpl(MethodImplOptions.NoInlining)]
private static void CallTest<T, U>(bool b)
{
Base foo = GetBase<T, U>(b);
foo.Test<object>();
}
private static Base GetBase<T, U>(bool b)
{
if (b)
{
return new Derived<T>();
}
else
{
return new Derived<U>();
}
}
public class Base
{
public virtual void Test<U>()
{
Console.WriteLine(typeof(U).FullName);
}
}
public class Derived<T> : Base
{
public override void Test<U>()
{
Console.WriteLine(typeof(T).FullName);
Console.WriteLine(typeof(U).FullName);
}
}
}After inlining However, it is not possible to compute the instantiation argument to pass to A few alternatives:
(3) might still be beneficial because we'll be able to inline the target code and in many cases the instantiation argument completely disappears after that. But I would probably start with (1) and perhaps (2). |
|
It turns out that the R2R issue was not runtime lookup, it was caused by that a method needs to perform runtime lookup being skipped so that it ends up using a non-exact instantiation stub. I have pushed a commit to bail on shared method table, and only allow runtime lookup for the method instantiation. |
| public virtual Delegate Foo<U>() | ||
| { | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(U)); | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List<U>)); | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List<List<U>>)); | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List<List<List<U>>>)); | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List<List<List<List<U>>>>)); | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List<List<List<List<List<U>>>>>)); | ||
| RuntimeLookupDelegateGenericVirtual.s_list.Add(typeof(List<List<List<List<List<List<U>>>>>>)); | ||
| return Foo<U>; | ||
| } |
There was a problem hiding this comment.
These method-group returns/assignments won't compile as written: method-group conversion only works to a specific delegate type, not to System.Delegate. return Foo<U>; requires an explicit conversion (e.g., cast to Func<Delegate> and then upcast), or change the API to use a concrete delegate type instead of Delegate.
|
|
||
| var test2 = new Derived(); | ||
| Delegate m1 = test2.Foo<List<T>>(); | ||
| Delegate m2 = test2.Foo<List<List<T>>>; |
There was a problem hiding this comment.
Delegate m2 = test2.Foo<List<List<T>>>; is a method group and can't be implicitly converted to System.Delegate. If the intent is to compare delegates, explicitly create a concrete delegate instance (e.g., cast to Func<Delegate> or invoke the method and compare the returned delegates, depending on the scenario you're trying to exercise).
| Delegate m2 = test2.Foo<List<List<T>>>; | |
| Delegate m2 = test2.Foo<List<List<T>>>(); |
4 participants
Previously in #122023, we hit an issue with GVM devirtualization when the devirtualized target is a shared generic method. GVM calls are imported with a runtime lookup that is specific to the base method. After devirtualization, the call requires the instantiation argument for the implementing method, and the existing lookup cannot be reused.
This PR unblocks devirtualization for shared generic targets by ensuring the call receives the correct instantiation parameter for the devirtualized method:
The multiple ad-hoc flags in
dvInfonow have been unified into a singleinstParamLookupWhen the target does not require a runtime lookup, we already know the exact generic context. We pass the instantiating stub as the inst param (shared with the existing array interface devirtualization path).
Store the instantiating stub (when necessary) directly in the exactContext, and devirtualizedMethod now can never be an instantiating stub. Remove the unnecessary
getInstantiatedEntryroundtrip.When the target requires a runtime lookup, we now introduced a new
DictionaryEntryKind::DevirtualizedMethodDescSlot, and pass it to theinstParamLookupso that later the VM knows that it needs to encode the class token from the devirtualized method instead of the original token. And in this case, the devirtualized methodpDevirtMDwill be passed as template method.Also due to the
instParamLookupchange I implement the support for R2R as well.NativeAOT still needs extra work in JIT to enable GVM devirts.
Example:
Codegen diff:
Another example that involves runtime lookup:
Codegen diff:
Contributes to #112596