Allow NecessaryTypeSymbol in generic dictionaries (#117315)

MichalStrehovsky · web-flow · commit 61fc02b140c8 · 2025-07-11T04:50:06.000-07:00
Before this change, whenever we needed to refer to a type from a generic dictionary, we used the fully loaded "allocated" form of it. But we can do the same optimization we do for non-generic code where we distinguish between MethodTables that we need for casting-like operations and MethodTables that are actually for allocation.
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs
@@ -266,6 +266,11 @@ public bool NeedsRuntimeLookup(ReadyToRunHelperId lookupKind, object targetOfLoo
 
         public ReadyToRunHelperId GetLdTokenHelperForType(TypeDesc type)
         {
+            // This will not correctly answer questions for canonical forms because the answer would
+            // be about whether a type loader template for the type exists.
+            // We need to make a small exception for canonical definitions because of RuntimeAugments.GetCanonType
+            Debug.Assert(!type.IsCanonicalSubtype(CanonicalFormKind.Any) || type.IsCanonicalDefinitionType(CanonicalFormKind.Any));
+
             bool canPotentiallyConstruct = ConstructedEETypeNode.CreationAllowed(type)
                 && NodeFactory.DevirtualizationManager.CanReferenceConstructedMethodTable(type.NormalizeInstantiation());
             if (canPotentiallyConstruct)
@@ -393,12 +398,6 @@ public GenericDictionaryLookup ComputeGenericLookup(MethodDesc contextMethod, Re
                 }
             }
 
-            // We don't have separate entries for necessary type handles to avoid possible duplication
-            if (lookupKind == ReadyToRunHelperId.NecessaryTypeHandle)
-            {
-                lookupKind = ReadyToRunHelperId.TypeHandle;
-            }
-
             DictionaryLayoutNode dictionaryLayout;
             if (contextSource == GenericContextSource.MethodParameter)
                 dictionaryLayout = _nodeFactory.GenericDictionaryLayout(contextMethod);
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DictionaryLayoutNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DictionaryLayoutNode.cs
@@ -212,6 +212,20 @@ public override bool TryGetSlotForEntry(GenericLookupResult entry, out int slot)
         {
             slot = Array.IndexOf(_layout, entry);
 
+            // If we're looking for a necessary type handle and it doesn't exist, check for a constructed type handle.
+            if (slot < 0 && entry is NecessaryTypeHandleGenericLookupResult necessaryLookup)
+            {
+                for (int i = 0; i < _layout.Length; i++)
+                {
+                    if (_layout[i] is TypeHandleGenericLookupResult other
+                        && other.Type == necessaryLookup.Type)
+                    {
+                        slot = i;
+                        return true;
+                    }
+                }
+            }
+
             // If this is a slot we should discard, respond false
             if (slot < 0 && Array.IndexOf(_discardedSlots, entry) >= 0)
                 return false;
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs
@@ -215,6 +215,59 @@ protected override bool EqualsImpl(GenericLookupResult obj)
         }
     }
 
+    /// <summary>
+    /// Generic lookup result that points to an MethodTable.
+    /// </summary>
+    public sealed class NecessaryTypeHandleGenericLookupResult : GenericLookupResult
+    {
+        private TypeDesc _type;
+
+        protected override int ClassCode => -4882991;
+
+        public NecessaryTypeHandleGenericLookupResult(TypeDesc type)
+        {
+            Debug.Assert(type.IsRuntimeDeterminedSubtype, "Concrete type in a generic dictionary?");
+            _type = type;
+        }
+
+        public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary)
+        {
+            TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation);
+
+            factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedType);
+
+            return factory.NecessaryTypeSymbol(instantiatedType);
+        }
+
+        public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
+        {
+            sb.Append("NecessaryTypeHandle_"u8);
+            sb.Append(nameMangler.GetMangledTypeName(_type));
+        }
+
+        public TypeDesc Type => _type;
+        public override string ToString() => $"NecessaryTypeHandle: {_type}";
+
+        public override NativeLayoutVertexNode TemplateDictionaryNode(NodeFactory factory)
+        {
+            return factory.NativeLayout.TypeHandleDictionarySlot(_type);
+        }
+
+        protected override int CompareToImpl(GenericLookupResult other, TypeSystemComparer comparer)
+        {
+            return comparer.Compare(_type, ((NecessaryTypeHandleGenericLookupResult)other)._type);
+        }
+
+        protected override int GetHashCodeImpl()
+        {
+            return _type.GetHashCode();
+        }
+
+        protected override bool EqualsImpl(GenericLookupResult obj)
+        {
+            return ((NecessaryTypeHandleGenericLookupResult)obj)._type == _type;
+        }
+    }
 
     /// <summary>
     /// Generic lookup result that points to an MethodTable where if the type is Nullable&lt;X&gt; the MethodTable is X
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.GenericLookups.cs
@@ -27,6 +27,11 @@ private void CreateNodeCaches()
                     return new TypeHandleGenericLookupResult(type);
                 });
 
+                _necessaryTypeSymbols = new NodeCache<TypeDesc, GenericLookupResult>(type =>
+                {
+                    return new NecessaryTypeHandleGenericLookupResult(type);
+                });
+
                 _unwrapNullableSymbols = new NodeCache<TypeDesc, GenericLookupResult>(type =>
                 {
                     return new UnwrapNullableTypeHandleGenericLookupResult(type);
@@ -95,6 +100,13 @@ public GenericLookupResult Type(TypeDesc type)
                 return _typeSymbols.GetOrAdd(type);
             }
 
+            private NodeCache<TypeDesc, GenericLookupResult> _necessaryTypeSymbols;
+
+            public GenericLookupResult NecessaryType(TypeDesc type)
+            {
+                return _necessaryTypeSymbols.GetOrAdd(type);
+            }
+
             private NodeCache<TypeDesc, GenericLookupResult> _unwrapNullableSymbols;
 
             public GenericLookupResult UnwrapNullableType(TypeDesc type)
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs
@@ -51,13 +51,12 @@ public ReadyToRunGenericHelperNode(NodeFactory factory, ReadyToRunHelperId helpe
 
         public static GenericLookupResult GetLookupSignature(NodeFactory factory, ReadyToRunHelperId id, object target)
         {
-            // Necessary type handle is not something you can put in a dictionary - someone should have normalized to TypeHandle
-            Debug.Assert(id != ReadyToRunHelperId.NecessaryTypeHandle);
-
             switch (id)
             {
                 case ReadyToRunHelperId.TypeHandle:
                     return factory.GenericLookup.Type((TypeDesc)target);
+                case ReadyToRunHelperId.NecessaryTypeHandle:
+                    return factory.GenericLookup.NecessaryType((TypeDesc)target);
                 case ReadyToRunHelperId.TypeHandleForCasting:
                     // Check that we unwrapped the cases that could be unwrapped to prevent duplicate entries
                     Debug.Assert(factory.GenericLookup.Type((TypeDesc)target) != factory.GenericLookup.UnwrapNullableType((TypeDesc)target));
@@ -304,6 +303,7 @@ public override int CompareToImpl(ISortableNode other, CompilerComparer comparer
             switch (_id)
             {
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.GetGCStaticBase:
                 case ReadyToRunHelperId.GetNonGCStaticBase:
                 case ReadyToRunHelperId.GetThreadStaticBase:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM/ARMReadyToRunGenericHelperNode.cs
@@ -188,6 +188,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARMEmitter enco
 
                 // These are all simple: just get the thing from the dictionary and we're done
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.MethodHandle:
                 case ReadyToRunHelperId.FieldHandle:
                 case ReadyToRunHelperId.MethodDictionary:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM64/ARM64ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_ARM64/ARM64ReadyToRunGenericHelperNode.cs
@@ -186,6 +186,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref ARM64Emitter en
 
                 // These are all simple: just get the thing from the dictionary and we're done
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.MethodHandle:
                 case ReadyToRunHelperId.FieldHandle:
                 case ReadyToRunHelperId.MethodDictionary:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_LoongArch64/LoongArch64ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_LoongArch64/LoongArch64ReadyToRunGenericHelperNode.cs
@@ -188,6 +188,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref LoongArch64Emit
 
                 // These are all simple: just get the thing from the dictionary and we're done
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.MethodHandle:
                 case ReadyToRunHelperId.FieldHandle:
                 case ReadyToRunHelperId.MethodDictionary:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_RiscV64/RiscV64ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_RiscV64/RiscV64ReadyToRunGenericHelperNode.cs
@@ -185,6 +185,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref RiscV64Emitter
 
                 // These are all simple: just get the thing from the dictionary and we're done
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.MethodHandle:
                 case ReadyToRunHelperId.FieldHandle:
                 case ReadyToRunHelperId.MethodDictionary:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_X64/X64ReadyToRunGenericHelperNode.cs
@@ -196,6 +196,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref X64Emitter enco
 
                 // These are all simple: just get the thing from the dictionary and we're done
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.MethodHandle:
                 case ReadyToRunHelperId.FieldHandle:
                 case ReadyToRunHelperId.MethodDictionary:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_X86/X86ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/Target_X86/X86ReadyToRunGenericHelperNode.cs
@@ -190,6 +190,7 @@ protected sealed override void EmitCode(NodeFactory factory, ref X86Emitter enco
 
                 // These are all simple: just get the thing from the dictionary and we're done
                 case ReadyToRunHelperId.TypeHandle:
+                case ReadyToRunHelperId.NecessaryTypeHandle:
                 case ReadyToRunHelperId.MethodHandle:
                 case ReadyToRunHelperId.FieldHandle:
                 case ReadyToRunHelperId.MethodDictionary:
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs
@@ -393,6 +393,14 @@ private static GenericLookupResult[] OptimizeSlots(NodeFactory factory, IEnumera
                 ArrayBuilder<GenericLookupResult> slotBuilder = default;
                 ArrayBuilder<GenericLookupResult> discardedBuilder = default;
 
+                // Find all constructed type lookups. We'll deduplicate those with necessary type lookups.
+                var constructedTypeLookups = new HashSet<TypeDesc>();
+                foreach (GenericLookupResult lookupResult in slots)
+                {
+                    if (lookupResult is TypeHandleGenericLookupResult thLookup)
+                        constructedTypeLookups.Add(thLookup.Type);
+                }
+
                 // We go over all slots in the layout, looking for references to method dictionaries
                 // that are going to be empty.
                 // Set those slots aside so that we can avoid generating the references to such dictionaries.
@@ -412,6 +420,11 @@ private static GenericLookupResult[] OptimizeSlots(NodeFactory factory, IEnumera
                             continue;
                         }
                     }
+                    else if (lookupResult is NecessaryTypeHandleGenericLookupResult thLookup)
+                    {
+                        if (constructedTypeLookups.Contains(thLookup.Type))
+                            continue;
+                    }
 
                     slotBuilder.Add(lookupResult);
                 }
diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
@@ -889,7 +889,7 @@ private void ImportUnbox(int token, ILOpcode opCode)
 
             if (type.IsRuntimeDeterminedSubtype)
             {
-                _dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, type), "Unbox");
+                _dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.NecessaryTypeHandle, type), "Unbox");
             }
             else
             {
@@ -966,7 +966,8 @@ private void ImportLdToken(int token)
                 ISymbolNode reference;
                 if (type.IsRuntimeDeterminedSubtype)
                 {
-                    reference = GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, type);
+                    reference = GetGenericLookupHelper(
+                        isTypeEquals ? ReadyToRunHelperId.NecessaryTypeHandle : ReadyToRunHelperId.TypeHandle, type);
                 }
                 else
                 {
@@ -1260,7 +1261,7 @@ private void ImportAddressOfElement(int token)
             if (elementType.IsGCPointer && !_isReadOnly)
             {
                 if (elementType.IsRuntimeDeterminedSubtype)
-                    _dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, elementType), "ldelema");
+                    _dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.NecessaryTypeHandle, elementType), "ldelema");
                 else
                     _dependencies.Add(_factory.NecessaryTypeSymbol(elementType), "ldelema");
 
diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs
@@ -1773,7 +1773,8 @@ private void embedGenericHandle(ref CORINFO_RESOLVED_TOKEN pResolvedToken, bool
                 if (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_NewObj
                         || pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Newarr
                         || pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Box
-                        || pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Constrained)
+                        || pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Constrained
+                        || pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Method)
                 {
                     helperId = ReadyToRunHelperId.TypeHandle;
                 }
@@ -1783,7 +1784,26 @@ private void embedGenericHandle(ref CORINFO_RESOLVED_TOKEN pResolvedToken, bool
                 }
                 else if (pResolvedToken.tokenType == CorInfoTokenKind.CORINFO_TOKENKIND_Ldtoken)
                 {
-                    helperId = _compilation.GetLdTokenHelperForType(td);
+                    if (((TypeDesc)target).IsRuntimeDeterminedSubtype)
+                    {
+                        // If we're in a generic context and the type is canonical, Compilation cannot give
+                        // us a good answer on what handle kind to use because we don't track canonical forms.
+                        // If we already have a fixed generic dictionary layout (a build with a scanner pass),
+                        // ask for NecessaryTypeSymbol - it will get upgraded as needed.
+                        // If we don't have a fixed dictionary layout, ask for a full type symbol because that's
+                        // always correct.
+                        MethodDesc callerContextMethod = HandleToObject(callerHandle);
+                        DictionaryLayoutNode contextLayout = _compilation.NodeFactory.GenericDictionaryLayout(
+                            callerContextMethod.RequiresInstMethodDescArg() ? callerContextMethod : callerContextMethod.OwningType);
+                        if (contextLayout.HasFixedSlots)
+                            helperId = ReadyToRunHelperId.NecessaryTypeHandle;
+                        else
+                            helperId = ReadyToRunHelperId.TypeHandle;
+                    }
+                    else
+                    {
+                        helperId = _compilation.GetLdTokenHelperForType(td);
+                    }
                 }
                 else
                 {
diff --git a/src/libraries/System.Linq.Expressions/tests/default.rd.xml b/src/libraries/System.Linq.Expressions/tests/default.rd.xml
@@ -13,6 +13,8 @@
       <Type Name="System.DateTimeOffset" Dynamic="Required All" />
       <Type Name="System.TimeSpan" Dynamic="Required All" />
       <Type Name="System.DateTime" Dynamic="Required All" />
+      <Type Name="System.DayOfWeek[]" Dynamic="Required All" />
+      <Type Name="System.Nullable`1[[System.DayOfWeek,System.Runtime]][]" Dynamic="Required All" />
 
       <Type Name="System.Text.StringBuilder" Dynamic="Required All" />
       <Type Name="System.Array" Dynamic="Required All" />
diff --git a/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs b/src/tests/nativeaot/SmokeTests/TrimmingBehaviors/DeadCodeElimination.cs
@@ -34,6 +34,7 @@ public static int Run()
         TestGetMethodOptimization.Run();
         TestTypeOfCodegenBranchElimination.Run();
         TestInvisibleGenericsTrimming.Run();
+        TestTypeHandlesInGenericDictionaries.Run();
 
         return 100;
     }
@@ -1130,6 +1131,36 @@ public static void Run()
         }
     }
 
+    class TestTypeHandlesInGenericDictionaries
+    {
+        class InvisibleType1;
+        class InvisibleType2;
+        class VisibleType1;
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static void GenericMethod<T, U, V>(object o, string expectedNameOfV)
+        {
+            if (o is T)
+                Console.WriteLine("Yes");
+
+            if (o.GetType() == typeof(U))
+                Console.WriteLine("Yes");
+
+            if (typeof(V).Name != expectedNameOfV)
+                throw new Exception();
+        }
+
+        public static void Run()
+        {
+            GenericMethod<InvisibleType1, InvisibleType2, VisibleType1>(new object(), nameof(VisibleType1));
+
+            ThrowIfPresent(typeof(TestTypeHandlesInGenericDictionaries), nameof(InvisibleType1));
+#if !DEBUG
+            ThrowIfPresent(typeof(TestTypeHandlesInGenericDictionaries), nameof(InvisibleType2));
+#endif
+        }
+    }
+
     [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2070:UnrecognizedReflectionPattern",
         Justification = "That's the point")]
     private static Type GetTypeSecretly(Type testType, string typeName) => testType.GetNestedType(typeName, BindingFlags.NonPublic | BindingFlags.Public);
