diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationModuleGroup.Aot.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationModuleGroup.Aot.cs index a56072d34842e..896967d28ada3 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationModuleGroup.Aot.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationModuleGroup.Aot.cs @@ -55,5 +55,11 @@ public partial class CompilationModuleGroup : IInliningPolicy /// If true, instance methods will only be generated once their owning type is created. /// public abstract bool AllowInstanceMethodOptimization(MethodDesc method); + + /// + /// If true, virtual methods on abstract types will only be generated once a non-abstract derived + /// type that doesn't override the virtual method is created. + /// + public abstract bool AllowVirtualMethodOnAbstractTypeOptimization(MethodDesc method); } } 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 01bf228f51dfc..80091e77ecdab 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -280,11 +280,25 @@ public sealed override bool HasConditionalStaticDependencies // // The conditional dependencies conditionally add the implementation of the virtual method // if the virtual method is used. - foreach (var method in _type.GetClosestDefType().GetAllVirtualMethods()) + // + // We walk the inheritance chain because abstract bases would only add a "tentative" + // method body of the implementation that can be trimmed away if no other type uses it. + DefType currentType = _type.GetClosestDefType(); + while (currentType != null) { - // Generic virtual methods are tracked by an orthogonal mechanism. - if (!method.HasInstantiation) - return true; + if (currentType == _type || (currentType is MetadataType mdType && mdType.IsAbstract)) + { + foreach (var method in currentType.GetAllVirtualMethods()) + { + // Abstract methods don't have a body associated with it so there's no conditional + // dependency to add. + // Generic virtual methods are tracked by an orthogonal mechanism. + if (!method.IsAbstract && !method.HasInstantiation) + return true; + } + } + + currentType = currentType.BaseType; } // If the type implements at least one interface, calls against that interface could result in this type's @@ -342,6 +356,8 @@ public sealed override IEnumerable GetConditionalSt if (needsDependenciesForVirtualMethodImpls) { + bool isNonInterfaceAbstractType = !defType.IsInterface && ((MetadataType)defType).IsAbstract; + foreach (MethodDesc decl in defType.EnumAllVirtualSlots()) { // Generic virtual methods are tracked by an orthogonal mechanism. @@ -349,10 +365,34 @@ public sealed override IEnumerable GetConditionalSt continue; MethodDesc impl = defType.FindVirtualFunctionTargetMethodOnObjectType(decl); - if (impl.OwningType == defType && !impl.IsAbstract) + 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) { MethodDesc canonImpl = impl.GetCanonMethodTarget(CanonicalFormKind.Specific); - IMethodNode implNode = factory.MethodEntrypoint(canonImpl, impl.OwningType.IsValueType); + + // 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 overriden in all their children. + // + // We don't do this if the method can be placed in the sealed vtable since + // those can never be overriden by children anyway. + bool canUseTentativeMethod = isNonInterfaceAbstractType + && !decl.CanMethodBeInSealedVTable() + && 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")); } @@ -932,7 +972,20 @@ private void OutputVirtualSlots(NodeFactory factory, ref ObjectDataBuilder objDa if (!implMethod.IsAbstract) { MethodDesc canonImplMethod = implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); - objData.EmitPointerReloc(factory.MethodEntrypoint(canonImplMethod, implMethod.OwningType.IsValueType)); + + // If the type we're generating now is abstract, and the implementation comes from an abstract type, + // only use a tentative method entrypoint that can have its body replaced by a throwing stub + // if no "hard" reference to that entrypoint exists in the program. + // This helps us to eliminate method bodies for virtual methods on abstract types that are fully overriden + // in the children of that abstract type. + bool canUseTentativeEntrypoint = implType is MetadataType mdImplType && mdImplType.IsAbstract && !mdImplType.IsInterface + && implMethod.OwningType is MetadataType mdImplMethodType && mdImplMethodType.IsAbstract + && factory.CompilationModuleGroup.AllowVirtualMethodOnAbstractTypeOptimization(canonImplMethod); + + IMethodNode implSymbol = canUseTentativeEntrypoint ? + factory.TentativeMethodEntrypoint(canonImplMethod, implMethod.OwningType.IsValueType) : + factory.MethodEntrypoint(canonImplMethod, implMethod.OwningType.IsValueType); + objData.EmitPointerReloc(implSymbol); } else { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs index a63c6142a3042..efa72b9bcd929 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -255,6 +255,13 @@ private void CreateNodeCaches() _methodEntrypoints = new MethodEntrypointHashtable(this); + _tentativeMethodEntrypoints = new NodeCache((MethodDesc method) => + { + IMethodNode entrypoint = MethodEntrypoint(method, unboxingStub: false); + return new TentativeMethodNode(entrypoint is TentativeMethodNode tentative ? + tentative.RealBody : (IMethodBodyNode)entrypoint); + }); + _tentativeMethods = new NodeCache(method => { return new TentativeInstanceMethodNode((IMethodBodyNode)MethodEntrypoint(method)); @@ -849,6 +856,15 @@ public IMethodNode MethodEntrypoint(MethodDesc method, bool unboxingStub = false return _methodEntrypoints.GetOrCreateValue(method); } + protected NodeCache _tentativeMethodEntrypoints; + + public IMethodNode TentativeMethodEntrypoint(MethodDesc method, bool unboxingStub = false) + { + // Didn't implement unboxing stubs for now. Would need to pass down the flag. + Debug.Assert(!unboxingStub); + return _tentativeMethodEntrypoints.GetOrAdd(method); + } + private NodeCache _tentativeMethods; public IMethodNode MethodEntrypointOrTentativeMethod(MethodDesc method, bool unboxingStub = false) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index c15d56635cdaa..0b954b0849412 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -442,7 +442,9 @@ public ScannedDevirtualizationManager(ImmutableArray