diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs index 50eafd7d0b50bc..8222bc9cec0844 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs @@ -972,25 +972,26 @@ public override IEnumerable ComputeAllVirtualSlots(TypeDesc type) // Enumerate all possible virtual slots of a type public static IEnumerable EnumAllVirtualSlots(MetadataType type) + { + return type.IsInterface ? type.GetAllVirtualMethods() : EnumAllVirtualSlotsOnClass(type); + } + private static IEnumerable EnumAllVirtualSlotsOnClass(MetadataType type) { MethodDescHashtable alreadyEnumerated = new MethodDescHashtable(); - if (!type.IsInterface) + do { - do + foreach (MethodDesc m in type.GetAllVirtualMethods()) { - foreach (MethodDesc m in type.GetAllVirtualMethods()) + MethodDesc possibleVirtual = FindSlotDefiningMethodForVirtualMethod(m); + if (!alreadyEnumerated.Contains(possibleVirtual)) { - MethodDesc possibleVirtual = FindSlotDefiningMethodForVirtualMethod(m); - if (!alreadyEnumerated.Contains(possibleVirtual)) - { - alreadyEnumerated.AddOrGetExisting(possibleVirtual); - yield return possibleVirtual; - } + alreadyEnumerated.AddOrGetExisting(possibleVirtual); + yield return possibleVirtual; } + } - type = type.BaseType; - } while (type != null); - } + type = type.BaseType; + } while (type != null); } /// diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs index 0f6fc1d3e3aeee..e8a1145ec7f698 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -141,7 +141,7 @@ private static VirtualMethodAnalysisFlags AnalyzeVirtualMethods(TypeDesc type) // foreach (DefType interfaceImpl in defType.RuntimeInterfaces) { - foreach (MethodDesc method in interfaceImpl.GetAllVirtualMethods()) + foreach (MethodDesc method in interfaceImpl.EnumAllVirtualSlots()) { if (!method.HasInstantiation) continue; @@ -369,56 +369,59 @@ public override IEnumerable GetConditionalStaticDep // If we're producing a full vtable, none of the dependencies are conditional. if (!factory.VTable(defType).HasKnownVirtualMethodUse) { - bool isNonInterfaceAbstractType = !defType.IsInterface && ((MetadataType)defType).IsAbstract; - - foreach (MethodDesc decl in defType.EnumAllVirtualSlots()) + if (!defType.IsInterface) { - // Generic virtual methods are tracked by an orthogonal mechanism. - if (decl.HasInstantiation) - continue; + bool isAbstractType = ((MetadataType)defType).IsAbstract; - MethodDesc impl = defType.FindVirtualFunctionTargetMethodOnObjectType(decl); - bool implOwnerIsAbstract = ((MetadataType)impl.OwningType).IsAbstract; - - // We add a conditional dependency in two situations: - // 1. The implementation is on this type. This is pretty obvious. - // 2. The implementation comes from an abstract base type. We do this - // because abstract types only request a TentativeMethodEntrypoint of the implementation. - // The actual method body of this entrypoint might still be trimmed away. - // We don't need to do this for implementations from non-abstract bases since - // non-abstract types will create a hard conditional reference to their virtual - // method implementations. - // - // We also skip abstract methods since they don't have a body to refer to. - if ((impl.OwningType == defType || implOwnerIsAbstract) && !impl.IsAbstract) + foreach (MethodDesc decl in defType.EnumAllVirtualSlots()) { - MethodDesc canonImpl = impl.GetCanonMethodTarget(CanonicalFormKind.Specific); + // Generic virtual methods are tracked by an orthogonal mechanism. + if (decl.HasInstantiation) + continue; - // If this is an abstract type, only request a tentative entrypoint (whose body - // might just be stubbed out). This lets us avoid generating method bodies for - // virtual method on abstract types that are overridden in all their children. + MethodDesc impl = defType.FindVirtualFunctionTargetMethodOnObjectType(decl); + bool implOwnerIsAbstract = ((MetadataType)impl.OwningType).IsAbstract; + + // We add a conditional dependency in two situations: + // 1. The implementation is on this type. This is pretty obvious. + // 2. The implementation comes from an abstract base type. We do this + // because abstract types only request a TentativeMethodEntrypoint of the implementation. + // The actual method body of this entrypoint might still be trimmed away. + // We don't need to do this for implementations from non-abstract bases since + // non-abstract types will create a hard conditional reference to their virtual + // method implementations. // - // We don't do this if the method can be placed in the sealed vtable since - // those can never be overridden by children anyway. - bool canUseTentativeMethod = isNonInterfaceAbstractType - && !decl.CanMethodBeInSealedVTable(factory) - && factory.CompilationModuleGroup.AllowVirtualMethodOnAbstractTypeOptimization(canonImpl); - IMethodNode implNode = canUseTentativeMethod ? - factory.TentativeMethodEntrypoint(canonImpl, impl.OwningType.IsValueType) : - factory.MethodEntrypoint(canonImpl, impl.OwningType.IsValueType); - result.Add(new CombinedDependencyListEntry(implNode, factory.VirtualMethodUse(decl), "Virtual method")); + // We also skip abstract methods since they don't have a body to refer to. + if ((impl.OwningType == defType || implOwnerIsAbstract) && !impl.IsAbstract) + { + MethodDesc canonImpl = impl.GetCanonMethodTarget(CanonicalFormKind.Specific); + + // If this is an abstract type, only request a tentative entrypoint (whose body + // might just be stubbed out). This lets us avoid generating method bodies for + // virtual method on abstract types that are overridden in all their children. + // + // We don't do this if the method can be placed in the sealed vtable since + // those can never be overridden by children anyway. + bool canUseTentativeMethod = isAbstractType + && !decl.CanMethodBeInSealedVTable(factory) + && factory.CompilationModuleGroup.AllowVirtualMethodOnAbstractTypeOptimization(canonImpl); + IMethodNode implNode = canUseTentativeMethod ? + factory.TentativeMethodEntrypoint(canonImpl, impl.OwningType.IsValueType) : + factory.MethodEntrypoint(canonImpl, impl.OwningType.IsValueType); + result.Add(new CombinedDependencyListEntry(implNode, factory.VirtualMethodUse(decl), "Virtual method")); + + result.Add(new CombinedDependencyListEntry( + factory.AddressTakenMethodEntrypoint(canonImpl, impl.OwningType.IsValueType), + factory.DelegateTargetVirtualMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Slot is a delegate target")); + } - result.Add(new CombinedDependencyListEntry( - factory.AddressTakenMethodEntrypoint(canonImpl, impl.OwningType.IsValueType), - factory.DelegateTargetVirtualMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Slot is a delegate target")); - } + if (impl.OwningType == defType) + { + factory.MetadataManager.NoteOverridingMethod(decl, impl); + } - if (impl.OwningType == defType) - { - factory.MetadataManager.NoteOverridingMethod(decl, impl); + factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, decl, impl); } - - factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, decl, impl); } Debug.Assert( @@ -448,7 +451,7 @@ public override IEnumerable GetConditionalStaticDep bool isVariantInterfaceImpl = VariantInterfaceMethodUseNode.IsVariantInterfaceImplementation(factory, _type, interfaceType); - foreach (MethodDesc interfaceMethod in interfaceType.GetAllVirtualMethods()) + foreach (MethodDesc interfaceMethod in interfaceType.EnumAllVirtualSlots()) { // Generic virtual methods are tracked by an orthogonal mechanism. if (interfaceMethod.HasInstantiation) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs index c57f8ad072c233..ba72bd9802fe08 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -1055,30 +1055,6 @@ public override Vertex WriteVertex(NodeFactory factory) return SetSavedVertex(layoutInfo); } - - private static IEnumerable EnumVirtualSlotsDeclaredOnType(TypeDesc declType) - { - // VirtualMethodUse of Foo.Method will bring in VirtualMethodUse - // of Foo<__Canon>.Method. This in turn should bring in Foo.Method. - DefType defType = declType.GetClosestDefType(); - - Debug.Assert(!declType.IsInterface); - - IEnumerable allSlots = defType.EnumAllVirtualSlots(); - - foreach (var method in allSlots) - { - // Generic virtual methods are tracked by an orthogonal mechanism. - if (method.HasInstantiation) - continue; - - // Current type doesn't define this slot. Another VTableSlice will take care of this. - if (method.OwningType != defType) - continue; - - yield return method; - } - } } public abstract class NativeLayoutGenericDictionarySlotNode : NativeLayoutVertexNode diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeGVMEntriesNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeGVMEntriesNode.cs index e3a0b4d38ab17e..e175f4cdcd9674 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeGVMEntriesNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/TypeGVMEntriesNode.cs @@ -46,6 +46,7 @@ public InterfaceGVMEntryInfo(MethodDesc callingMethod, MethodDesc implementation public TypeGVMEntriesNode(TypeDesc associatedType) { Debug.Assert(associatedType.IsTypeDefinition); + Debug.Assert(!associatedType.IsInterface); _associatedType = associatedType; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/VTableSliceNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/VTableSliceNode.cs index e412116fc3970f..4d0e0785e15117 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/VTableSliceNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/VTableSliceNode.cs @@ -33,8 +33,7 @@ protected static MethodDesc[] ComputeSlots(TypeDesc type) bool isObjectType = type.IsObject; DefType defType = type.GetClosestDefType(); - IEnumerable allSlots = type.IsInterface ? - type.GetAllVirtualMethods() : defType.EnumAllVirtualSlots(); + IEnumerable allSlots = defType.EnumAllVirtualSlots(); foreach (var method in allSlots) { @@ -224,8 +223,7 @@ public override IEnumerable GetConditionalStaticDep // of Foo<__Canon>.Method. This in turn should bring in Foo.Method. DefType defType = _type.GetClosestDefType(); - IEnumerable allSlots = _type.IsInterface ? - _type.GetAllVirtualMethods() : defType.EnumAllVirtualSlots(); + IEnumerable allSlots = defType.EnumAllVirtualSlots(); foreach (var method in allSlots) { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs index 4d633e6dee2118..649c1e48f6c893 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeValidationChecker.cs @@ -494,40 +494,43 @@ Task ValidateTypeWorkerHelper(TypeDesc typeToCheckForSkipValidation) } } - foreach (var virtualMethod in type.EnumAllVirtualSlots()) + if (!type.IsInterface) { - var implementationMethod = virtualMethodAlgorithm.FindVirtualFunctionTargetMethodOnObjectType(virtualMethod, type); - - if (implementationMethod != null) + foreach (var virtualMethod in type.EnumAllVirtualSlots()) { - // Validate that for every override involving generic methods that the generic method constraints are matching - if (!CompareMethodConstraints(virtualMethod, implementationMethod)) - { - AddTypeValidationError(type, $"Virtual method '{virtualMethod}' overridden by method '{implementationMethod}' which does not have matching generic constraints"); - return false; - } + var implementationMethod = virtualMethodAlgorithm.FindVirtualFunctionTargetMethodOnObjectType(virtualMethod, type); - // Validate that if the decl method for the virtual is not on the immediate base type, that the intermediate type did not establish a - // covariant return type which requires the implementation method to specify a more specific base type - if ((virtualMethod.OwningType != type.BaseType) && (virtualMethod.OwningType != type) && (baseTypeVirtualMethodAlgorithm != null)) + if (implementationMethod != null) { - var implementationOnBaseType = baseTypeVirtualMethodAlgorithm.FindVirtualFunctionTargetMethodOnObjectType(virtualMethod, type.BaseType); - if (!implementationMethod.Signature.ApplySubstitution(type.Instantiation).EquivalentWithCovariantReturnType(implementationOnBaseType.Signature.ApplySubstitution(type.Instantiation))) + // Validate that for every override involving generic methods that the generic method constraints are matching + if (!CompareMethodConstraints(virtualMethod, implementationMethod)) { - AddTypeValidationError(type, $"Virtual method '{virtualMethod}' overridden by method '{implementationMethod}' does not satisfy the covariant return type introduced with '{implementationOnBaseType}'"); + AddTypeValidationError(type, $"Virtual method '{virtualMethod}' overridden by method '{implementationMethod}' which does not have matching generic constraints"); return false; } + + // Validate that if the decl method for the virtual is not on the immediate base type, that the intermediate type did not establish a + // covariant return type which requires the implementation method to specify a more specific base type + if ((virtualMethod.OwningType != type.BaseType) && (virtualMethod.OwningType != type) && (baseTypeVirtualMethodAlgorithm != null)) + { + var implementationOnBaseType = baseTypeVirtualMethodAlgorithm.FindVirtualFunctionTargetMethodOnObjectType(virtualMethod, type.BaseType); + if (!implementationMethod.Signature.ApplySubstitution(type.Instantiation).EquivalentWithCovariantReturnType(implementationOnBaseType.Signature.ApplySubstitution(type.Instantiation))) + { + AddTypeValidationError(type, $"Virtual method '{virtualMethod}' overridden by method '{implementationMethod}' does not satisfy the covariant return type introduced with '{implementationOnBaseType}'"); + return false; + } + } } - } - // Validate that all virtual static methods are actually implemented if the type is not abstract - // Validate that all virtual instance methods are actually implemented if the type is not abstract - if (!type.IsAbstract) - { - if (implementationMethod == null || implementationMethod.IsAbstract) + // Validate that all virtual static methods are actually implemented if the type is not abstract + // Validate that all virtual instance methods are actually implemented if the type is not abstract + if (!type.IsAbstract) { - AddTypeValidationError(type, $"Interface method '{virtualMethod}' does not have implementation"); - return false; + if (implementationMethod == null || implementationMethod.IsAbstract) + { + AddTypeValidationError(type, $"Interface method '{virtualMethod}' does not have implementation"); + return false; + } } } }