ILC: track concrete dependencies of open generic methods

[sbomer]

When RyuJit inlines a generic callee into a callsite where some of the generic arguments are concrete, the code generation may require concrete dependencies of the inlinee. We need to ensure that the scanner tracks such dependencies statically so that they are available for codegen.

For example:

using System;
using System.Runtime.CompilerServices;

class Program
{
    static void Main()  
    {
        var r = typeof(Program).GetMethod(nameof(Caller)).MakeGenericMethod(GetStringType()).Invoke(null, []);
    }

    static Type GetStringType() => typeof(string);

    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    static string Callee<T, U>() => new Container<T>().ToString();

    public static string Caller<T>() => Callee<object, T>();
}

class Container<T>;

Here the concrete instantiation Caller<string> is not discovered statically - but when Callee<object, T>() is inlined into the canonical code for Caller<__Canon>, this results in a concrete reference to Container<object> from Caller<__Canon>, so we need to keep any generic dictionary dependencies of Callee<object, T>.

This change adds additional tracking of open generic methods to ensure that any concrete dependencies are preserved during scanning. It expands the tracking done by ShadowConcreteMethodNode (now ShadowGenericMethodNode) to include non-concrete (not fully instantiated) generics. There are three entry points where we needed additional tracking for open generics:

• When reflecting over a generic method
• When encountering a GVM (this was the case in the original issue)
• When we see a direct call to a generic method

Now when ShadowGenericMethodNode is used to instantiate generic dependencies of runtime-determined nodes, the resulting instantiation may not be concrete. We handle this by attempting the instantiation, and if the result is concrete we track the dependency as normal. If the result is not concrete, then we either track it as another ShadowGenericMethodNode (if the result is a generic method) or ignore it.

Fixes #120847

[dotnet-policy-service]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR enhances the ILC (IL Compiler) to track concrete dependencies of open generic methods to prevent compilation failures when RyuJit inlines generic callees into callsites with concrete generic arguments. The key change is expanding the scope of dependency tracking from only concrete generic methods to include partially instantiated (open) generic methods that may still have concrete dependencies.

• Renamed ShadowConcreteMethodNode to ShadowGenericMethodNode to reflect its expanded purpose
• Added tracking for open generic methods in three critical entry points: reflection access, generic virtual method (GVM) calls, and direct method calls
• Enhanced ReadyToRunGenericHelperNode.InstantiateDependencies to handle non-concrete instantiations by attempting substitution and tracking shadow generic methods when results remain canonical

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
ShadowGenericMethodNode.cs	Renamed from ShadowConcreteMethodNode.cs; removed assertion that restricted usage to concrete methods only, allowing tracking of open generic methods
ReadyToRunGenericHelperNode.cs	Added logic to handle non-concrete instantiations by checking substitution results and tracking shadow generic methods when instantiations remain canonical
ReflectionInvokeMapNode.cs	Added tracking of shadow generic methods for shared generic instantiations accessed via reflection
GenericVirtualMethodImplNode.cs	Added shadow generic method dependency tracking for generic virtual method implementations
GenericDictionaryNode.cs	Updated references from ShadowConcreteMethod to ShadowGenericMethod
NodeFactory.cs	Renamed method and hashtable from ShadowConcreteMethod to ShadowGenericMethod; updated CanonicalEntrypoint method
ILImporter.Scanner.cs	Changed direct call tracking to use ShadowGenericMethod instead of CanonicalEntrypoint
MetadataManager.cs	Updated type cast from ShadowConcreteMethodNode to ShadowGenericMethodNode
ILScanner.cs	Updated type cast from ShadowConcreteMethodNode to ShadowGenericMethodNode
RyuJitCompilation.cs	Updated type cast from ShadowConcreteMethodNode to ShadowGenericMethodNode
ILCompiler.Compiler.csproj	Updated file reference from ShadowConcreteMethodNode.cs to ShadowGenericMethodNode.cs
ILCompiler.ReadyToRun.csproj	Updated file reference from ShadowConcreteMethodNode.cs to ShadowGenericMethodNode.cs
Generics.cs	Added five comprehensive test cases covering GVM inlining with different inheritance patterns and reflection scenarios

[MichalStrehovsky]

I would want to keep these assert - they prevent nonsensical graphs.

I think we should split this class into multiple classes - one that tracks fully instantiated things (can keep calling it ShadowConcreteMethod) - this one can still assert all these things. And add one that is not fully specialized. There may be a common base type. I think separate node would also result in more readable dependency graphs.

[sbomer]

Split into ShadowConcreteMethodNode and ShadowNonConcreteMethodNode (with shared base ShadowMethodNode)

[MichalStrehovsky]

I wonder if we could move the logic in this if block into the individual GenericLookupResult.GetTarget overrides - maybe add a bool isNotConcreteInstantiation parameter and have it return null if the result would be canonical?

[sbomer]

I had that idea too, but hesitated because I was afraid the GetTarget change would be too viral. It doesn't look as bad as I thought.