Fix reflection invoke file format issue for generic methods (#83438)

The file format that mapped reflection metadata to runtime artifacts had a limitation that required generic methods to be always instantiated over concrete types - we could not place canonical method bodies there. The limitation stemmed from the fact that the mapping table was placing a generic dictionary in it.

This switches the table to use NameAndSignature + an array of instantiation arguments.

The rest of this change is just deleting code that was working around the problem. The fixes in NativeLayoutVertexNode and NodeFactory.NativeLayout are fixing analysis holes that were exposed by our ability to be more lazy.

Also saves 0.4% on BasicWebApi, which is always nice.

src/coreclr/nativeaot/Common/src/Internal/Runtime/TypeLoader/ExternalReferencesTable.cs

@@ -56,11 +56,6 @@ public RuntimeTypeHandle GetRuntimeTypeHandleFromIndex(uint index)
             return RuntimeAugments.CreateRuntimeTypeHandle(GetIntPtrFromIndex(index));
         }
 
-        public IntPtr GetGenericDictionaryFromIndex(uint index)
-        {
-            return GetIntPtrFromIndex(index);
-        }
-
         public unsafe IntPtr GetAddressFromIndex(uint index)
         {
             if (index >= _elementsCount)

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.Metadata.cs

@@ -897,14 +897,12 @@ private struct InvokeMapEntryDataEnumerator<TLookupMethodInfo, TDictionaryCompon
             public RuntimeTypeHandle _entryType;
             public IntPtr _methodEntrypoint;
             public uint _dynamicInvokeCookie;
-            public IntPtr _entryDictionary;
             public RuntimeTypeHandle[] _methodInstantiation;
 
             // Computed data
             private bool _hasEntryPoint;
             private bool _isMatchingMethodHandleAndDeclaringType;
             private MethodNameAndSignature _nameAndSignature;
-            private RuntimeTypeHandle[] _entryMethodInstantiation;
 
             public InvokeMapEntryDataEnumerator(
                 TLookupMethodInfo lookupMethodInfo,
@@ -925,10 +923,8 @@ public InvokeMapEntryDataEnumerator(
                 _dynamicInvokeCookie = 0xffffffff;
                 _hasEntryPoint = false;
                 _isMatchingMethodHandleAndDeclaringType = false;
-                _entryDictionary = IntPtr.Zero;
                 _methodInstantiation = null;
                 _nameAndSignature = null;
-                _entryMethodInstantiation = null;
             }
 
             public void GetNext(
@@ -944,10 +940,8 @@ public void GetNext(
                 _entryType = default(RuntimeTypeHandle);
                 _methodEntrypoint = IntPtr.Zero;
                 _dynamicInvokeCookie = 0xffffffff;
-                _entryDictionary = IntPtr.Zero;
                 _methodInstantiation = null;
                 _nameAndSignature = null;
-                _entryMethodInstantiation = null;
 
                 // If the current entry is not a canonical entry of the same canonical form kind we are looking for, then this cannot be a match
                 if (((_flags & InvokeTableFlags.IsUniversalCanonicalEntry) != 0) != (_canonFormKind == CanonicalFormKind.Universal))
@@ -988,17 +982,18 @@ public void GetNext(
                 if ((_flags & InvokeTableFlags.IsGenericMethod) == 0)
                     return;
 
-                if ((_flags & InvokeTableFlags.IsUniversalCanonicalEntry) != 0)
+                if ((_flags & InvokeTableFlags.RequiresInstArg) != 0)
                 {
-                    Debug.Assert((_hasEntryPoint || ((_flags & InvokeTableFlags.HasVirtualInvoke) != 0)) && ((_flags & InvokeTableFlags.RequiresInstArg) != 0));
+                    Debug.Assert(_hasEntryPoint || ((_flags & InvokeTableFlags.HasVirtualInvoke) != 0));
 
                     uint nameAndSigPointerToken = entryParser.GetUnsigned();
                     _nameAndSignature = TypeLoaderEnvironment.Instance.GetMethodNameAndSignatureFromNativeLayoutOffset(_moduleHandle, nameAndSigPointerToken);
                 }
-                else if (((_flags & InvokeTableFlags.RequiresInstArg) != 0) && _hasEntryPoint)
-                    _entryDictionary = extRefTable.GetGenericDictionaryFromIndex(entryParser.GetUnsigned());
-                else
+
+                if ((_flags & InvokeTableFlags.IsUniversalCanonicalEntry) == 0)
+                {
                     _methodInstantiation = GetTypeSequence(ref extRefTable, ref entryParser);
+                }
             }
 
             public bool IsMatchingOrCompatibleEntry()
@@ -1018,15 +1013,7 @@ public bool IsMatchingOrCompatibleEntry()
                     return true;
                 }
 
-                // Generic non-shareable method or abstract methods: check for the canonical equivalency of the method
-                // instantiation arguments that we read from the entry
-                if (((_flags & InvokeTableFlags.RequiresInstArg) == 0) || !_hasEntryPoint)
-                    return _lookupMethodInfo.CanInstantiationsShareCode(_methodInstantiation, _canonFormKind);
-
-                // Generic shareable method: check for canonical equivalency of the method instantiation arguments.
-                // The method instantiation arguments are extracted from the generic dictionary pointer that we read from the entry.
-                Debug.Assert(_entryDictionary != IntPtr.Zero);
-                return GetNameAndSignatureAndMethodInstantiation() && _lookupMethodInfo.CanInstantiationsShareCode(_entryMethodInstantiation, _canonFormKind);
+                return _lookupMethodInfo.CanInstantiationsShareCode(_methodInstantiation, _canonFormKind);
             }
 
             public bool GetMethodEntryPoint(out IntPtr methodEntrypoint, out TDictionaryComponentType dictionaryComponent, out IntPtr rawMethodEntrypoint)
@@ -1057,7 +1044,7 @@ private bool GetDictionaryComponent(out TDictionaryComponentType dictionaryCompo
                 }
 
                 // Dictionary for generic method (either found statically or constructed dynamically)
-                return GetNameAndSignatureAndMethodInstantiation() && _lookupMethodInfo.GetMethodDictionary(_nameAndSignature, out dictionaryComponent);
+                return _lookupMethodInfo.GetMethodDictionary(_nameAndSignature, out dictionaryComponent);
             }
 
             private bool GetMethodEntryPointComponent(TDictionaryComponentType dictionaryComponent, out IntPtr methodEntrypoint)
@@ -1074,27 +1061,6 @@ private bool GetMethodEntryPointComponent(TDictionaryComponentType dictionaryCom
 
                 return true;
             }
-
-            private bool GetNameAndSignatureAndMethodInstantiation()
-            {
-                if (_nameAndSignature != null)
-                {
-                    Debug.Assert(((_flags & InvokeTableFlags.IsUniversalCanonicalEntry) != 0) || (_entryMethodInstantiation != null && _entryMethodInstantiation.Length > 0));
-                    return true;
-                }
-
-                if ((_flags & InvokeTableFlags.IsUniversalCanonicalEntry) != 0)
-                {
-                    // _nameAndSignature should have been read from the InvokeMap entry directly!
-                    Debug.Fail("Universal canonical entries do NOT have dictionary entries!");
-                    return false;
-                }
-
-                RuntimeTypeHandle dummy1;
-                bool success = TypeLoaderEnvironment.Instance.TryGetGenericMethodComponents(_entryDictionary, out dummy1, out _nameAndSignature, out _entryMethodInstantiation);
-                Debug.Assert(success && dummy1.Equals(_entryType) && _nameAndSignature != null && _entryMethodInstantiation != null && _entryMethodInstantiation.Length > 0);
-                return success;
-            }
         }
 
         public bool TryGetMetadataForTypeMethodNameAndSignature(RuntimeTypeHandle declaringTypeHandle, MethodNameAndSignature nameAndSignature, out QMethodDefinition methodHandle)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDictionaryNode.cs

@@ -7,8 +7,6 @@
 using Internal.Text;
 using Internal.TypeSystem;
 
-using CombinedDependencyList = System.Collections.Generic.List<ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.CombinedDependencyListEntry>;
-
 namespace ILCompiler.DependencyAnalysis
 {
     /// <summary>
@@ -198,13 +196,7 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
         protected override TypeSystemContext Context => _owningMethod.Context;
         public override TypeSystemEntity OwningEntity => _owningMethod;
         public MethodDesc OwningMethod => _owningMethod;
-        public override bool HasConditionalStaticDependencies => true;
-        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory)
-        {
-            CombinedDependencyList list = null;
-            factory.MetadataManager.GetConditionalDependenciesDueToMethodGenericDictionary(ref list, factory, _owningMethod);
-            return list ?? (IEnumerable<CombinedDependencyListEntry>)System.Array.Empty<CombinedDependencyListEntry>();
-        }
+        public override bool HasConditionalStaticDependencies => false;
 
         protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFactory factory)
         {
@@ -247,8 +239,6 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
             // Make sure the dictionary can also be populated
             dependencies.Add(factory.ShadowConcreteMethod(_owningMethod), "Dictionary contents");
 
-            factory.MetadataManager.GetDependenciesForGenericDictionary(ref dependencies, factory, _owningMethod);
-
             return dependencies;
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs

@@ -1708,6 +1708,7 @@ public sealed class NativeLayoutFieldLdTokenGenericDictionarySlotNode : NativeLa
 
         public NativeLayoutFieldLdTokenGenericDictionarySlotNode(FieldDesc field)
         {
+            Debug.Assert(field.OwningType.IsRuntimeDeterminedSubtype);
             _field = field;
         }
 
@@ -1717,12 +1718,21 @@ public NativeLayoutFieldLdTokenGenericDictionarySlotNode(FieldDesc field)
 
         public sealed override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
         {
-            yield return new DependencyListEntry(factory.NativeLayout.FieldLdTokenVertex(_field), "Field Signature");
+            var result = new DependencyList
+            {
+                { factory.NativeLayout.FieldLdTokenVertex(_field), "Field Signature" }
+            };
 
             foreach (var dependency in factory.NativeLayout.TemplateConstructableTypes(_field.OwningType))
             {
-                yield return new DependencyListEntry(dependency, "template construction dependency");
+                result.Add(dependency, "template construction dependency");
             }
+
+            var canonOwningType = (InstantiatedType)_field.OwningType.ConvertToCanonForm(CanonicalFormKind.Specific);
+            FieldDesc canonField = factory.TypeSystemContext.GetFieldForInstantiatedType(_field.GetTypicalFieldDefinition(), canonOwningType);
+            factory.MetadataManager.GetDependenciesDueToLdToken(ref result, factory, canonField);
+
+            return result;
         }
 
         protected sealed override Vertex WriteSignatureVertex(NativeWriter writer, NodeFactory factory)
@@ -1747,18 +1757,25 @@ public NativeLayoutMethodLdTokenGenericDictionarySlotNode(MethodDesc method)
 
         public sealed override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory)
         {
-            yield return new DependencyListEntry(factory.NativeLayout.MethodLdTokenVertex(_method), "Method Signature");
+            var result = new DependencyList
+            {
+                { factory.NativeLayout.MethodLdTokenVertex(_method), "Method Signature" }
+            };
 
             foreach (var dependency in factory.NativeLayout.TemplateConstructableTypes(_method.OwningType))
             {
-                yield return new DependencyListEntry(dependency, "template construction dependency for method OwningType");
+                result.Add(dependency, "template construction dependency for method OwningType");
             }
 
             foreach (var type in _method.Instantiation)
             {
                 foreach (var dependency in factory.NativeLayout.TemplateConstructableTypes(type))
-                    yield return new DependencyListEntry(dependency, "template construction dependency for method Instantiation types");
+                    result.Add(dependency, "template construction dependency for method Instantiation types");
             }
+
+            factory.MetadataManager.GetDependenciesDueToLdToken(ref result, factory, _method.GetCanonMethodTarget(CanonicalFormKind.Specific));
+
+            return result;
         }
 
         protected sealed override Vertex WriteSignatureVertex(NativeWriter writer, NodeFactory factory)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.NativeLayout.cs

@@ -176,6 +176,8 @@ public IEnumerable<IDependencyNode> TemplateConstructableTypes(TypeDesc type)
                     {
                         yield return _factory.NativeLayout.TemplateTypeLayout(arrayCanonicalType);
                     }
+
+                    yield return _factory.MaximallyConstructableType(arrayCanonicalType);
                 }
 
                 while (type.IsParameterizedType)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

@@ -957,6 +957,9 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type)
         private NodeCache<MethodDesc, ReflectedMethodNode> _reflectedMethods;
         public ReflectedMethodNode ReflectedMethod(MethodDesc method)
         {
+            // We track reflectability at canonical method body level
+            method = method.GetCanonMethodTarget(CanonicalFormKind.Specific);
+
             return _reflectedMethods.GetOrAdd(method);
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedMethodNode.cs

@@ -24,8 +24,7 @@ public class ReflectedMethodNode : DependencyNodeCore<NodeFactory>
 
         public ReflectedMethodNode(MethodDesc method)
         {
-            Debug.Assert(!method.IsCanonicalMethod(CanonicalFormKind.Any) ||
-                method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method);
+            Debug.Assert(method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method);
             _method = method;
         }
 
@@ -53,12 +52,6 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
                 dependencies.Add(factory.ReflectedMethod(typicalMethod), "Definition of the reflectable method");
             }
 
-            MethodDesc canonMethod = _method.GetCanonMethodTarget(CanonicalFormKind.Specific);
-            if (canonMethod != _method)
-            {
-                dependencies.Add(factory.ReflectedMethod(canonMethod), "Canonical version of the reflectable method");
-            }
-
             // Make sure we generate the method body and other artifacts.
             if (MetadataManager.IsMethodSupportedInReflectionInvoke(_method))
             {
@@ -82,11 +75,7 @@ public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFacto
 
                 if (!_method.IsAbstract)
                 {
-                    dependencies.Add(factory.MethodEntrypoint(canonMethod), "Body of a reflectable method");
-
-                    if (_method.HasInstantiation
-                        && _method != canonMethod)
-                        dependencies.Add(factory.MethodGenericDictionary(_method), "Dictionary of a reflectable method");
+                    dependencies.Add(factory.MethodEntrypoint(_method), "Body of a reflectable method");
                 }
             }