Delete `arrayArgumentsFound` logic from type loader (#85509)
authorMichal Strehovský <MichalStrehovsky@users.noreply.github.com>
Mon, 1 May 2023 04:39:37 +0000 (13:39 +0900)
committerGitHub <noreply@github.com>
Mon, 1 May 2023 04:39:37 +0000 (21:39 -0700)
I think this was handling a situation where we're trying to find an existing `MethodTable` for `Foo<SomeType[]>` and such `MethodTable` does statically exist but we didn't have a mapping table entry for `SomeType[]` and we couldn't get a `MethodTable` for it, even though it statically exists in the image. Since the compiler optimization to skip emitting this mapping got commented out, it wasn't actually possible to end up in a situation where we wouldn't know how to find a `MethodTable` for an existing array at runtime. We therefore don't need the slow path that tries to find the type without having `MethodTable`s for all generic arguments.

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericTypesLookup.cs
src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/TypeSystem/TypeDesc.Runtime.cs
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ArrayMapNode.cs

index 5c918fc..2db0a24 100644 (file)
@@ -91,76 +91,34 @@ namespace Internal.Runtime.TypeLoader
             }
         }
 
-        internal abstract class GenericTypeLookupData
-        {
-            internal abstract int LookupHashCode();
-            internal abstract bool MatchParsedEntry(RuntimeTypeHandle tentativeType);
-            internal abstract bool MatchGenericTypeEntry(GenericTypeEntry entry);
-        }
-        internal class DefTypeBasedGenericTypeLookup : GenericTypeLookupData
-        {
-            protected DefType _typeToLookup;
-
-            internal DefTypeBasedGenericTypeLookup(DefType typeToLookup) { _typeToLookup = typeToLookup; }
-
-            internal override int LookupHashCode() { return _typeToLookup.GetHashCode(); }
-
-            internal override bool MatchParsedEntry(RuntimeTypeHandle tentativeType)
-            {
-                //
-                // Entries read from the hashtable are loaded as DefTypes, and compared to the input.
-                // This lookup is slower than the lookups using RuntimeTypeHandles, but can handle cases where we don't have
-                // RuntimeTypeHandle values for all of the components of the input DefType, but still need to look it up in case the type
-                // statically exists and has an existing RuntimeTypeHandle value.
-                //
-                TypeSystemContext context = _typeToLookup.Context;
-
-                RuntimeTypeHandle[] parsedArgsHandles;
-                RuntimeTypeHandle parsedTypeDefinitionHandle = RuntimeAugments.GetGenericInstantiation(tentativeType, out parsedArgsHandles);
-
-                DefType parsedTypeDefinition = (DefType)context.ResolveRuntimeTypeHandle(parsedTypeDefinitionHandle);
-                Instantiation parsedArgs = context.ResolveRuntimeTypeHandles(parsedArgsHandles);
-                DefType parsedGenericType = context.ResolveGenericInstantiation(parsedTypeDefinition, parsedArgs);
-
-                return parsedGenericType == _typeToLookup;
-            }
-
-            internal override bool MatchGenericTypeEntry(GenericTypeEntry entry)
-            {
-                TypeSystemContext context = _typeToLookup.Context;
-
-                DefType parsedTypeDefinition = (DefType)context.ResolveRuntimeTypeHandle(entry._genericTypeDefinitionHandle);
-                Instantiation parsedArgs = context.ResolveRuntimeTypeHandles(entry._genericTypeArgumentHandles);
-                DefType parsedGenericType = context.ResolveGenericInstantiation(parsedTypeDefinition, parsedArgs);
-
-                return parsedGenericType == _typeToLookup;
-            }
-        }
-        internal class HandleBasedGenericTypeLookup : DefTypeBasedGenericTypeLookup
+        internal struct GenericTypeLookupData
         {
+            private DefType _typeToLookup;
             private RuntimeTypeHandle _genericTypeDefinitionHandle;
             private RuntimeTypeHandle[] _genericTypeArgumentHandles;
 
-            internal HandleBasedGenericTypeLookup(DefType typeToLookup) : base(typeToLookup)
+            internal GenericTypeLookupData(DefType typeToLookup)
             {
                 Debug.Assert(typeToLookup != null);
+                _typeToLookup = typeToLookup;
                 _genericTypeDefinitionHandle = _typeToLookup.GetTypeDefinition().RuntimeTypeHandle;
                 // _genericTypeArgumentHandles not initialized here to avoid allocation of new array (and it's not used if we initialize _typeToLookup).
             }
 
-            internal HandleBasedGenericTypeLookup(RuntimeTypeHandle genericTypeDefinitionHandle, RuntimeTypeHandle[] genericTypeArgumentHandles) : base(null)
+            internal GenericTypeLookupData(RuntimeTypeHandle genericTypeDefinitionHandle, RuntimeTypeHandle[] genericTypeArgumentHandles)
             {
                 Debug.Assert(genericTypeArgumentHandles != null);
+                _typeToLookup = null;
                 _genericTypeDefinitionHandle = genericTypeDefinitionHandle;
                 _genericTypeArgumentHandles = genericTypeArgumentHandles;
             }
 
-            internal override int LookupHashCode()
+            internal int LookupHashCode()
             {
                 return _typeToLookup != null ? _typeToLookup.GetHashCode() : TypeHashingAlgorithms.ComputeGenericInstanceHashCode(_genericTypeDefinitionHandle.GetHashCode(), _genericTypeArgumentHandles);
             }
 
-            internal override bool MatchParsedEntry(RuntimeTypeHandle tentativeType)
+            internal bool MatchParsedEntry(RuntimeTypeHandle tentativeType)
             {
                 RuntimeTypeHandle parsedTypeDefinitionHandle = RuntimeAugments.GetGenericDefinition(tentativeType);
                 if (!parsedTypeDefinitionHandle.Equals(_genericTypeDefinitionHandle))
@@ -179,7 +137,7 @@ namespace Internal.Runtime.TypeLoader
                 return true;
             }
 
-            internal override bool MatchGenericTypeEntry(GenericTypeEntry entry)
+            internal bool MatchGenericTypeEntry(GenericTypeEntry entry)
             {
                 if (!entry._genericTypeDefinitionHandle.Equals(_genericTypeDefinitionHandle))
                     return false;
@@ -254,7 +212,7 @@ namespace Internal.Runtime.TypeLoader
 
         public bool TryLookupConstructedGenericTypeForComponents(RuntimeTypeHandle genericTypeDefinitionHandle, RuntimeTypeHandle[] genericTypeArgumentHandles, out RuntimeTypeHandle runtimeTypeHandle)
         {
-            return TryLookupConstructedGenericTypeForComponents(new HandleBasedGenericTypeLookup(genericTypeDefinitionHandle, genericTypeArgumentHandles), out runtimeTypeHandle);
+            return TryLookupConstructedGenericTypeForComponents(new GenericTypeLookupData(genericTypeDefinitionHandle, genericTypeArgumentHandles), out runtimeTypeHandle);
         }
 
         public bool TryLookupConstructedLazyDictionaryForContext(IntPtr context, IntPtr signature, out IntPtr dictionary)
index 63108ba..07e06d0 100644 (file)
@@ -77,23 +77,16 @@ namespace Internal.TypeSystem
                         // Generic type. First make sure we have type handles for the arguments, then check
                         // the instantiation.
                         bool argumentsRegistered = true;
-                        bool arrayArgumentsFound = false;
                         for (int i = 0; i < instantiation.Length; i++)
                         {
                             if (!instantiation[i].RetrieveRuntimeTypeHandleIfPossible())
                             {
                                 argumentsRegistered = false;
-                                arrayArgumentsFound = arrayArgumentsFound || (instantiation[i] is ArrayType);
                             }
                         }
 
                         RuntimeTypeHandle rtth;
-
-                        // If at least one of the arguments is not known to the runtime, we take a slower
-                        // path to compare the current type we need a handle for to the list of generic
-                        // types statically available, by loading them as DefTypes and doing a DefType comparaison
-                        if ((argumentsRegistered && TypeLoaderEnvironment.Instance.TryLookupConstructedGenericTypeForComponents(new TypeLoaderEnvironment.HandleBasedGenericTypeLookup(typeAsDefType), out rtth)) ||
-                            (arrayArgumentsFound && TypeLoaderEnvironment.Instance.TryLookupConstructedGenericTypeForComponents(new TypeLoaderEnvironment.DefTypeBasedGenericTypeLookup(typeAsDefType), out rtth)))
+                        if (argumentsRegistered && TypeLoaderEnvironment.Instance.TryLookupConstructedGenericTypeForComponents(new TypeLoaderEnvironment.GenericTypeLookupData(typeAsDefType), out rtth))
                         {
                             typeAsDefType.SetRuntimeTypeHandleUnsafe(rtth);
                             return true;
index 2fff199..dc57546 100644 (file)
@@ -57,16 +57,6 @@ namespace ILCompiler.DependencyAnalysis
 
                 var arrayType = (ArrayType)type;
 
-                // This optimization is not compatible with canInlineTypeCheck on JIT/EE interface returning
-                // CORINFO_INLINE_TYPECHECK_PASS unconditionally.
-                //
-                // If we're generating a template for this type, we can skip generating the hashtable entry
-                // since the type loader can just create this type at runtime if something needs it. It's
-                // okay to have multiple EETypes for the same array type.
-                // var canonArrayType = arrayType.ConvertToCanonForm(CanonicalFormKind.Specific);
-                // if (arrayType != canonArrayType && factory.NativeLayout.TemplateTypeLayout(canonArrayType).Marked)
-                //     continue;
-
                 // Look at the constructed type symbol. If a constructed type wasn't emitted, then the array map entry isn't valid for use
                 IEETypeNode arrayTypeSymbol = factory.ConstructedTypeSymbol(arrayType);