Split generic virtual method slot use and impl tracking #82222
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Diagnostics; | ||
| using System.Collections.Generic; | ||
|
|
||
| using ILCompiler.DependencyAnalysisFramework; | ||
|
|
||
| using Internal.TypeSystem; | ||
|
|
||
| namespace ILCompiler.DependencyAnalysis | ||
| { | ||
| /// <summary> | ||
| /// Represents a use of a generic virtual method body implementation. | ||
| /// </summary> | ||
| public class GenericVirtualMethodImplNode : DependencyNodeCore<NodeFactory> | ||
| { | ||
| private const int UniversalCanonGVMDepthHeuristic_CanonDepth = 2; | ||
| private readonly MethodDesc _method; | ||
|
|
||
| public GenericVirtualMethodImplNode(MethodDesc method) | ||
| { | ||
| Debug.Assert(method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method); | ||
| Debug.Assert(method.HasInstantiation); | ||
|
|
||
| // This is either a generic virtual method or a MethodImpl for a static interface method. | ||
| // We can't test for static MethodImpl so at least sanity check it's static and noninterface. | ||
| Debug.Assert(method.IsVirtual || (method.Signature.IsStatic && !method.OwningType.IsInterface)); | ||
|
There was a problem hiding this comment. What about default interface implementations? In that case, isn't the method owned by an interface? There was a problem hiding this comment. This will change with #80602 because we'll run into the assert. We don't currently track interfaces because they're assumed to be associated with a class (default implementations just show up on the owning class). This works for instance methods. It doesn't work for statics. It's a bit of a overhaul which is why I didn't want to do it in one go. |
||
|
|
||
| _method = method; | ||
| } | ||
|
|
||
| public override bool HasConditionalStaticDependencies => false; | ||
| public override bool InterestingForDynamicDependencyAnalysis => false; | ||
| public override bool StaticDependenciesAreComputed => true; | ||
| protected override string GetName(NodeFactory factory) => "__GVMImplNode_" + factory.NameMangler.GetMangledMethodName(_method); | ||
|
|
||
| public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory) | ||
| { | ||
| DependencyList dependencies = null; | ||
|
|
||
| factory.MetadataManager.GetDependenciesDueToVirtualMethodReflectability(ref dependencies, factory, _method); | ||
|
|
||
| bool validInstantiation = | ||
| _method.IsSharedByGenericInstantiations || ( // Non-exact methods are always valid instantiations (always pass constraints check) | ||
| _method.Instantiation.CheckValidInstantiationArguments() && | ||
| _method.OwningType.Instantiation.CheckValidInstantiationArguments() && | ||
| _method.CheckConstraints()); | ||
|
|
||
| if (validInstantiation) | ||
|
There was a problem hiding this comment. By "valid" here, do we mean that the code is not broken? That the compiler hasn't written incorrect IL? There was a problem hiding this comment. This is a cargo cult I brought over from the previous spot. I don't know when it kicks in. It goes back all the way to dotnet/corert#2521. |
||
| { | ||
| if (factory.TypeSystemContext.SupportsUniversalCanon && _method.IsGenericDepthGreaterThan(UniversalCanonGVMDepthHeuristic_CanonDepth)) | ||
|
There was a problem hiding this comment. I don't think I understand the "generic depth" thing here. There was a problem hiding this comment. Copy paste from the previous spot - if universal shared generics are supported, we cut off analysis early if it gets too deep and leave universal shared code to handle it. SupportsUniversalCanon is always false in NativeAOT. |
||
| { | ||
| // fall back to using the universal generic variant of the generic method | ||
| return dependencies; | ||
| } | ||
|
|
||
| bool getUnboxingStub = _method.OwningType.IsValueType && !_method.Signature.IsStatic; | ||
| dependencies ??= new DependencyList(); | ||
| dependencies.Add(factory.MethodEntrypoint(_method, getUnboxingStub), "GVM Dependency - Canon method"); | ||
|
|
||
| if (_method.IsSharedByGenericInstantiations) | ||
| { | ||
| dependencies.Add(factory.NativeLayout.TemplateMethodEntry(_method), "GVM Dependency - Template entry"); | ||
| dependencies.Add(factory.NativeLayout.TemplateMethodLayout(_method), "GVM Dependency - Template"); | ||
| } | ||
| } | ||
|
|
||
| return dependencies; | ||
| } | ||
|
|
||
| public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory context) => null; | ||
|
|
||
| public override bool HasDynamicDependencies => false; | ||
|
There was a problem hiding this comment. It seems like this is important, but it's not clear to me why. There was a problem hiding this comment. All nodes have HasDynamicDependencies false, except for GVMDependencies node. Returning true means the dependency analysis will call into This node not reporting it is the optimization. There was a problem hiding this comment. And why is this a valid optimization? Or, why is it necessary for GVMDependencies to always return true? There was a problem hiding this comment.
But we don't need to do this for generic virtual method implementations that don't define a new slot (i.e. they're just overrides) - this is what the optimization tries to split - a slot use + an override ("implementation", since we also introduce this node for non abstract new slots) of the slot. |
||
|
|
||
| public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not quite clear to me what a "slot" means in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slot in the sense of ECMA-335 - either a newslot method, or virtual that doesn't have another virtual method with the same name/sig in the inheritance hierarchy.