diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/INodeWithRuntimeDeterminedDependencies.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/INodeWithRuntimeDeterminedDependencies.cs index 645835a365d4cd..d6e5f5445c37a2 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/INodeWithRuntimeDeterminedDependencies.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/INodeWithRuntimeDeterminedDependencies.cs @@ -17,6 +17,6 @@ public interface INodeWithRuntimeDeterminedDependencies /// /// Instantiates runtime determined dependencies of this node using the supplied generic context. /// - IEnumerable InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation); + IEnumerable InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation, bool isConcreteInstantiation); } } diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowConcreteMethodNode.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowConcreteMethodNode.cs index 4480f9d3941035..1ce5973c387d38 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowConcreteMethodNode.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowConcreteMethodNode.cs @@ -13,113 +13,20 @@ namespace ILCompiler.DependencyAnalysis { /// - /// Represents a concrete method on a generic type (or a generic method) that doesn't - /// have code emitted in the executable because it's physically backed by a canonical - /// method body. The purpose of this node is to track the dependencies of the concrete - /// method body, as if it was generated. The node acts as a symbol for the canonical - /// method for convenience. + /// Represents a concrete method (fully instantiated) for the purpose of + /// tracking dependencies of inlinees. /// - public class ShadowConcreteMethodNode : DependencyNodeCore, IMethodNode, ISymbolNodeWithLinkage + public class ShadowConcreteMethodNode : ShadowMethodNode, IMethodNode, ISymbolNodeWithLinkage { - /// - /// Gets the canonical method body that defines the dependencies of this node. - /// - public IMethodNode CanonicalMethodNode { get; } - - /// - /// Gets the concrete method represented by this node. - /// - public MethodDesc Method { get; } - - // Implementation of ISymbolNode that makes this node act as a symbol for the canonical body - public void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) - { - CanonicalMethodNode.AppendMangledName(nameMangler, sb); - } - public int Offset => CanonicalMethodNode.Offset; - public bool RepresentsIndirectionCell => CanonicalMethodNode.RepresentsIndirectionCell; - - public override bool StaticDependenciesAreComputed - => CanonicalMethodNode.StaticDependenciesAreComputed; - public ShadowConcreteMethodNode(MethodDesc method, IMethodNode canonicalMethod) + : base(method, canonicalMethod) { Debug.Assert(!method.IsSharedByGenericInstantiations); - Debug.Assert(!method.IsRuntimeDeterminedExactMethod); - Debug.Assert(canonicalMethod.Method.IsSharedByGenericInstantiations); - Debug.Assert(canonicalMethod.Method == method.GetCanonMethodTarget(CanonicalFormKind.Specific)); - Method = method; - CanonicalMethodNode = canonicalMethod; - } - - public ISymbolNode NodeForLinkage(NodeFactory factory) - { - return CanonicalMethodNode; - } - - public override IEnumerable GetStaticDependencies(NodeFactory factory) - { - DependencyList dependencies = new DependencyList(); - - // Make sure the canonical body gets generated - dependencies.Add(new DependencyListEntry(CanonicalMethodNode, "Canonical body")); - - // Instantiate the runtime determined dependencies of the canonical method body - // with the concrete instantiation of the method to get concrete dependencies. - Instantiation typeInst = Method.OwningType.Instantiation; - Instantiation methodInst = Method.Instantiation; - IEnumerable staticDependencies = CanonicalMethodNode.GetStaticDependencies(factory); - - if (staticDependencies != null) - { - foreach (DependencyListEntry canonDep in staticDependencies) - { - var runtimeDep = canonDep.Node as INodeWithRuntimeDeterminedDependencies; - if (runtimeDep != null) - { - dependencies.AddRange(runtimeDep.InstantiateDependencies(factory, typeInst, methodInst)); - } - } - } - - return dependencies; - } - - public sealed override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) - { - // Instantiate the runtime determined dependencies of the canonical method body - // with the concrete instantiation of the method to get concrete dependencies. - Instantiation typeInst = Method.OwningType.Instantiation; - Instantiation methodInst = Method.Instantiation; - IEnumerable staticDependencies = CanonicalMethodNode.GetConditionalStaticDependencies(factory); - - if (staticDependencies != null) - { - foreach (CombinedDependencyListEntry canonDep in staticDependencies) - { - Debug.Assert(canonDep.OtherReasonNode is not INodeWithRuntimeDeterminedDependencies); - - var node = canonDep.Node; - if (node is INodeWithRuntimeDeterminedDependencies runtimeDeterminedNode) - { - foreach (var nodeInner in runtimeDeterminedNode.InstantiateDependencies(factory, typeInst, methodInst)) - yield return new CombinedDependencyListEntry(nodeInner.Node, canonDep.OtherReasonNode, nodeInner.Reason); - } - } - } } - protected override string GetName(NodeFactory factory) => $"{Method} backed by {CanonicalMethodNode.GetMangledName(factory.NameMangler)}"; - - public sealed override bool HasConditionalStaticDependencies => CanonicalMethodNode.HasConditionalStaticDependencies; - public sealed override bool HasDynamicDependencies => false; - public sealed override bool InterestingForDynamicDependencyAnalysis => false; - - public sealed override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory factory) => null; - - int ISortableNode.ClassCode => -1440570971; + protected override int ClassCode => -1440570971; - int ISortableNode.CompareToImpl(ISortableNode other, CompilerComparer comparer) + protected override int CompareToImpl(ISortableNode other, CompilerComparer comparer) { var compare = comparer.Compare(Method, ((ShadowConcreteMethodNode)other).Method); if (compare != 0) diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowMethodNode.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowMethodNode.cs new file mode 100644 index 00000000000000..ba0b8280fb0b30 --- /dev/null +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowMethodNode.cs @@ -0,0 +1,128 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +using ILCompiler.DependencyAnalysisFramework; + +using Internal.Text; +using Internal.TypeSystem; + +using Debug = System.Diagnostics.Debug; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Represents a method on a generic type (or a generic method) that doesn't + /// have code emitted in the executable because it's physically backed by a canonical + /// method body. The purpose of this node is to track the dependencies of the generic + /// method body, as if it was generated. The node acts as a symbol for the canonical + /// method for convenience. + /// + public abstract class ShadowMethodNode : DependencyNodeCore, IMethodNode, ISymbolNodeWithLinkage + { + /// + /// Gets the canonical method body that defines the dependencies of this node. + /// + public IMethodNode CanonicalMethodNode { get; } + + /// + /// Gets the generic method represented by this node. + /// + public MethodDesc Method { get; } + + // Implementation of ISymbolNode that makes this node act as a symbol for the canonical body + public void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) + { + CanonicalMethodNode.AppendMangledName(nameMangler, sb); + } + public int Offset => CanonicalMethodNode.Offset; + public bool RepresentsIndirectionCell => CanonicalMethodNode.RepresentsIndirectionCell; + + public override bool StaticDependenciesAreComputed + => CanonicalMethodNode.StaticDependenciesAreComputed; + + public ShadowMethodNode(MethodDesc method, IMethodNode canonicalMethod) + { + Debug.Assert(!method.IsRuntimeDeterminedExactMethod); + Debug.Assert(canonicalMethod.Method == method.GetCanonMethodTarget(CanonicalFormKind.Specific)); + Debug.Assert(canonicalMethod.Method.IsSharedByGenericInstantiations); + Method = method; + CanonicalMethodNode = canonicalMethod; + } + + public ISymbolNode NodeForLinkage(NodeFactory factory) + { + return CanonicalMethodNode; + } + + public override IEnumerable GetStaticDependencies(NodeFactory factory) + { + DependencyList dependencies = new DependencyList(); + + // Make sure the canonical body gets generated + dependencies.Add(new DependencyListEntry(CanonicalMethodNode, "Canonical body")); + + // Instantiate the runtime determined dependencies of the canonical method body + // with the concrete instantiation of the method to get concrete dependencies. + Instantiation typeInst = Method.OwningType.Instantiation; + Instantiation methodInst = Method.Instantiation; + IEnumerable staticDependencies = CanonicalMethodNode.GetStaticDependencies(factory); + + if (staticDependencies != null) + { + foreach (DependencyListEntry canonDep in staticDependencies) + { + var runtimeDep = canonDep.Node as INodeWithRuntimeDeterminedDependencies; + if (runtimeDep != null) + { + dependencies.AddRange(runtimeDep.InstantiateDependencies(factory, typeInst, methodInst, isConcreteInstantiation: !Method.IsSharedByGenericInstantiations)); + } + } + } + + return dependencies; + } + + public sealed override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) + { + // Instantiate the runtime determined dependencies of the canonical method body + // with the concrete instantiation of the method to get concrete dependencies. + Instantiation typeInst = Method.OwningType.Instantiation; + Instantiation methodInst = Method.Instantiation; + IEnumerable staticDependencies = CanonicalMethodNode.GetConditionalStaticDependencies(factory); + + if (staticDependencies != null) + { + foreach (CombinedDependencyListEntry canonDep in staticDependencies) + { + Debug.Assert(canonDep.OtherReasonNode is not INodeWithRuntimeDeterminedDependencies); + + var node = canonDep.Node; + if (node is INodeWithRuntimeDeterminedDependencies runtimeDeterminedNode) + { + foreach (var nodeInner in runtimeDeterminedNode.InstantiateDependencies(factory, typeInst, methodInst, isConcreteInstantiation: !Method.IsSharedByGenericInstantiations)) + yield return new CombinedDependencyListEntry(nodeInner.Node, canonDep.OtherReasonNode, nodeInner.Reason); + } + } + } + } + + + protected override string GetName(NodeFactory factory) => $"{Method} backed by {CanonicalMethodNode.GetMangledName(factory.NameMangler)}"; + + public sealed override bool HasConditionalStaticDependencies => CanonicalMethodNode.HasConditionalStaticDependencies; + public sealed override bool HasDynamicDependencies => false; + public sealed override bool InterestingForDynamicDependencyAnalysis => false; + + public sealed override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory factory) => null; + + int ISortableNode.ClassCode => ClassCode; + + protected abstract int ClassCode { get; } + + int ISortableNode.CompareToImpl(ISortableNode other, CompilerComparer comparer) => CompareToImpl(other, comparer); + + protected abstract int CompareToImpl(ISortableNode other, CompilerComparer comparer); + } +} diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowNonConcreteMethodNode.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowNonConcreteMethodNode.cs new file mode 100644 index 00000000000000..2163f0093eff7b --- /dev/null +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ShadowNonConcreteMethodNode.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +using ILCompiler.DependencyAnalysisFramework; + +using Internal.Text; +using Internal.TypeSystem; + +using Debug = System.Diagnostics.Debug; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Represents a non-concrete method (not fully instantiated) for the purpose of + /// tracking dependencies. + /// + public class ShadowNonConcreteMethodNode : ShadowMethodNode, IMethodNode, ISymbolNodeWithLinkage + { + public ShadowNonConcreteMethodNode(MethodDesc method, IMethodNode canonicalMethod) + : base(method, canonicalMethod) + { + Debug.Assert(method.IsSharedByGenericInstantiations); + } + + protected override int ClassCode => 2120942405; + + protected override int CompareToImpl(ISortableNode other, CompilerComparer comparer) + { + var compare = comparer.Compare(Method, ((ShadowNonConcreteMethodNode)other).Method); + if (compare != 0) + return compare; + + return comparer.Compare(CanonicalMethodNode, ((ShadowNonConcreteMethodNode)other).CanonicalMethodNode); + } + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs index 95895cacff11d8..610a1a84a038c9 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs @@ -749,7 +749,7 @@ private sealed class MakeGenericMethodSite : INodeWithRuntimeDeterminedDependenc public MakeGenericMethodSite(MethodDesc method) => _method = method; - public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) + public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation, bool isConcreteInstantiation) { var list = new DependencyList(); MethodDesc instantiatedMethod = _method.InstantiateSignature(typeInstantiation, methodInstantiation); @@ -765,7 +765,7 @@ private sealed class MakeGenericTypeSite : INodeWithRuntimeDeterminedDependencie public MakeGenericTypeSite(TypeDesc type) => _type = type; - public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) + public IEnumerable.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation, bool isConcreteInstantiation) { var list = new DependencyList(); TypeDesc instantiatedType = _type.InstantiateSignature(typeInstantiation, methodInstantiation); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DataflowAnalyzedMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DataflowAnalyzedMethodNode.cs index 6a6e2e0b9fb697..026b9a54028948 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DataflowAnalyzedMethodNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DataflowAnalyzedMethodNode.cs @@ -61,7 +61,7 @@ public override IEnumerable SearchDynamicDependenci // Instantiate all runtime dependencies for the found generic specialization foreach (var n in _runtimeDependencies) { - foreach (var d in n.InstantiateDependencies(factory, method.OwningType.Instantiation, method.Instantiation)) + foreach (var d in n.InstantiateDependencies(factory, method.OwningType.Instantiation, method.Instantiation, isConcreteInstantiation: !method.IsSharedByGenericInstantiations)) { yield return new CombinedDependencyListEntry(d.Node, null, d.Reason); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs index b15a29d99b7b2e..1a8e15f067b200 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs @@ -72,7 +72,7 @@ public GenericLookupResultContext(TypeSystemEntity canonicalOwner, Instantiation public abstract class GenericLookupResult { protected abstract int ClassCode { get; } - public abstract ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary); + public abstract ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation); public abstract void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb); public abstract override string ToString(); protected abstract int CompareToImpl(GenericLookupResult other, TypeSystemComparer comparer); @@ -93,12 +93,33 @@ public sealed override int GetHashCode() return ClassCode * 31 + GetHashCodeImpl(); } + private static bool InstantiationIsConcrete(Instantiation instantiation) + { + if (instantiation.IsNull || instantiation.Length == 0) + { + return true; + } + + for (int i = 0; i < instantiation.Length; i++) + { + TypeDesc argument = instantiation[i]; + if (argument.IsRuntimeDeterminedSubtype || argument.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return false; + } + } + + return true; + } + public virtual void EmitDictionaryEntry(ref ObjectDataBuilder builder, NodeFactory factory, GenericLookupResultContext dictionary, GenericDictionaryNode dictionaryNode) { ISymbolNode target; try { - target = GetTarget(factory, dictionary); + Debug.Assert(InstantiationIsConcrete(dictionary.TypeInstantiation)); + Debug.Assert(InstantiationIsConcrete(dictionary.MethodInstantiation)); + target = GetTarget(factory, dictionary, isConcreteInstantiation: true); } catch (TypeSystemException) { @@ -176,11 +197,15 @@ public TypeHandleGenericLookupResult(TypeDesc type) _type = type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { // We are getting a maximally constructable type symbol because this might be something passed to newobj. TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedType); return factory.MaximallyConstructableType(instantiatedType); @@ -231,10 +256,14 @@ public NecessaryTypeHandleGenericLookupResult(TypeDesc type) _type = type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedType); return factory.NecessaryTypeSymbol(instantiatedType); @@ -285,10 +314,14 @@ public MetadataTypeHandleGenericLookupResult(TypeDesc type) _type = type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedType); return factory.MetadataTypeSymbol(instantiatedType); @@ -339,7 +372,7 @@ public UnwrapNullableTypeHandleGenericLookupResult(TypeDesc type) _type = type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); @@ -347,6 +380,12 @@ public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultCo if (instantiatedType.IsNullable) instantiatedType = instantiatedType.Instantiation[0]; + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); + // We are getting a constructed type symbol because this might be something passed to newobj. return factory.ConstructedTypeSymbol(instantiatedType); } @@ -396,10 +435,25 @@ public MethodHandleGenericLookupResult(MethodDesc method) _method = method; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { MethodDesc instantiatedMethod = _method.GetNonRuntimeDeterminedMethodFromRuntimeDeterminedMethodViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - return factory.RuntimeMethodHandle(instantiatedMethod); + if (isConcreteInstantiation || !instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)) + { + return factory.RuntimeMethodHandle(instantiatedMethod); + } + else + { + Debug.Assert(instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)); + // The substituted method is still not concrete, but it may have concrete dependencies + // so we track it as a shadow node to ensure its dependencies are discovered. + if (!instantiatedMethod.IsAbstract) + { + factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedMethod); + return factory.ShadowNonConcreteMethod(instantiatedMethod); + } + return null; + } } public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) @@ -446,9 +500,15 @@ public FieldHandleGenericLookupResult(FieldDesc field) _field = field; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { FieldDesc instantiatedField = _field.GetNonRuntimeDeterminedFieldFromRuntimeDeterminedFieldViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && instantiatedField.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedField.OwningType.IsCanonicalSubtype(CanonicalFormKind.Any)); + return factory.RuntimeFieldHandle(instantiatedField); } @@ -496,13 +556,23 @@ public MethodDictionaryGenericLookupResult(MethodDesc method) _method = method; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { MethodDesc instantiatedMethod = _method.GetNonRuntimeDeterminedMethodFromRuntimeDeterminedMethodViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedMethod); - - return factory.MethodGenericDictionary(instantiatedMethod); + if (isConcreteInstantiation || !instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)) + { + return factory.MethodGenericDictionary(instantiatedMethod); + } + else + { + Debug.Assert(instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)); + if (!instantiatedMethod.IsAbstract) + { + return factory.ShadowNonConcreteMethod(instantiatedMethod); + } + return null; + } } public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) @@ -552,11 +622,24 @@ public MethodEntryGenericLookupResult(MethodDesc method, bool isUnboxingThunk) _isUnboxingThunk = isUnboxingThunk; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { MethodDesc instantiatedMethod = _method.GetNonRuntimeDeterminedMethodFromRuntimeDeterminedMethodViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - // TODO-SIZE: this is address taken only in the delegate target case - return factory.FatAddressTakenFunctionPointer(instantiatedMethod, _isUnboxingThunk); + if (isConcreteInstantiation || !instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)) + { + // TODO-SIZE: this is address taken only in the delegate target case + return factory.FatAddressTakenFunctionPointer(instantiatedMethod, _isUnboxingThunk); + } + else + { + Debug.Assert(instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)); + if (!instantiatedMethod.IsAbstract) + { + factory.TypeSystemContext.DetectGenericCycles(dictionary.Context, instantiatedMethod); + return factory.ShadowNonConcreteMethod(instantiatedMethod); + } + return null; + } } public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) @@ -622,17 +705,29 @@ public VirtualDispatchCellGenericLookupResult(MethodDesc method) _method = method; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext context) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext context, bool isConcreteInstantiation) { MethodDesc instantiatedMethod = _method.GetNonRuntimeDeterminedMethodFromRuntimeDeterminedMethodViaSubstitution(context.TypeInstantiation, context.MethodInstantiation); + if (isConcreteInstantiation || !instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)) + { + TypeSystemEntity contextOwner = context.Context; + GenericDictionaryNode dictionary = + contextOwner is TypeDesc ? + (GenericDictionaryNode)factory.TypeGenericDictionary((TypeDesc)contextOwner) : + (GenericDictionaryNode)factory.MethodGenericDictionary((MethodDesc)contextOwner); - TypeSystemEntity contextOwner = context.Context; - GenericDictionaryNode dictionary = - contextOwner is TypeDesc ? - (GenericDictionaryNode)factory.TypeGenericDictionary((TypeDesc)contextOwner) : - (GenericDictionaryNode)factory.MethodGenericDictionary((MethodDesc)contextOwner); - - return factory.InterfaceDispatchCell(instantiatedMethod, dictionary); + return factory.InterfaceDispatchCell(instantiatedMethod, dictionary); + } + else + { + Debug.Assert(instantiatedMethod.IsCanonicalMethod(CanonicalFormKind.Any)); + if (!instantiatedMethod.IsAbstract) + { + factory.TypeSystemContext.DetectGenericCycles(context.Context, instantiatedMethod); + return factory.ShadowNonConcreteMethod(instantiatedMethod); + } + return null; + } } public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) @@ -680,9 +775,15 @@ public TypeNonGCStaticBaseGenericLookupResult(TypeDesc type) _type = (MetadataType)type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { var instantiatedType = (MetadataType)_type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); + return factory.TypeNonGCStaticsSymbol(instantiatedType); } @@ -731,9 +832,14 @@ public TypeThreadStaticBaseIndexGenericLookupResult(TypeDesc type) _type = (MetadataType)type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { var instantiatedType = (MetadataType)_type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); return factory.TypeThreadStaticIndex(instantiatedType); } @@ -782,9 +888,14 @@ public TypeGCStaticBaseGenericLookupResult(TypeDesc type) _type = (MetadataType)type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { var instantiatedType = (MetadataType)_type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); return factory.TypeGCStaticsSymbol(instantiatedType); } @@ -833,9 +944,14 @@ public ObjectAllocatorGenericLookupResult(TypeDesc type) _type = type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); return factory.ExternFunctionSymbol(new Utf8String(JitHelper.GetNewObjectHelperForType(instantiatedType))); } @@ -880,9 +996,14 @@ public DefaultConstructorLookupResult(TypeDesc type) _type = type; } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { TypeDesc instantiatedType = _type.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)) + { + return null; + } + Debug.Assert(!instantiatedType.IsCanonicalSubtype(CanonicalFormKind.Any)); MethodDesc defaultCtor = Compilation.GetConstructorForCreateInstanceIntrinsic(instantiatedType); return instantiatedType.IsValueType ? factory.ExactCallableAddress(defaultCtor) : factory.CanonicalEntrypoint(defaultCtor); } @@ -950,10 +1071,19 @@ public override IEnumerable> NonRelocDependencie } } - public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) + public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary, bool isConcreteInstantiation) { MethodDesc instantiatedConstrainedMethod = _constrainedMethod.GetNonRuntimeDeterminedMethodFromRuntimeDeterminedMethodViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); TypeDesc instantiatedConstraintType = _constraintType.GetNonRuntimeDeterminedTypeFromRuntimeDeterminedSubtypeViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); + if (!isConcreteInstantiation && + (instantiatedConstrainedMethod.IsCanonicalMethod(CanonicalFormKind.Any) || + instantiatedConstraintType.IsCanonicalSubtype(CanonicalFormKind.Any))) + { + return null; + } + Debug.Assert(!instantiatedConstrainedMethod.IsCanonicalMethod(CanonicalFormKind.Any)); + Debug.Assert(!instantiatedConstraintType.IsCanonicalSubtype(CanonicalFormKind.Any)); + MethodDesc implMethod; MethodDesc instantiatedConstrainedMethodDefinition = instantiatedConstrainedMethod.GetMethodDefinition(); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericVirtualMethodImplNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericVirtualMethodImplNode.cs index 6912c9358a18dc..98615a90ce980b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericVirtualMethodImplNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericVirtualMethodImplNode.cs @@ -56,6 +56,7 @@ public override IEnumerable GetStaticDependencies(NodeFacto { dependencies.Add(factory.NativeLayout.TemplateMethodEntry(_method), "GVM Dependency - Template entry"); dependencies.Add(factory.NativeLayout.TemplateMethodLayout(_method), "GVM Dependency - Template"); + dependencies.Add(factory.ShadowNonConcreteMethod(_method), "GVM Dependency - shadow generic method"); } 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 30ad30e4f228fc..820cf40f5813be 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -410,6 +410,8 @@ private void CreateNodeCaches() _shadowConcreteMethods = new ShadowConcreteMethodHashtable(this); + _shadowNonConcreteMethods = new ShadowNonConcreteMethodHashtable(this); + _virtMethods = new VirtualMethodUseHashtable(this); _variantMethods = new NodeCache((MethodDesc method) => @@ -1156,9 +1158,8 @@ public IMethodNode ExactCallableAddressTakenAddress(MethodDesc method, bool isUn public IMethodNode CanonicalEntrypoint(MethodDesc method) { MethodDesc canonMethod = method.GetCanonMethodTarget(CanonicalFormKind.Specific); - if (method != canonMethod) { + if (method != canonMethod) return ShadowConcreteMethod(method); - } else return MethodEntrypoint(method); } @@ -1279,11 +1280,29 @@ protected override ShadowConcreteMethodNode CreateValueFromKey(MethodDesc key) = } private ShadowConcreteMethodHashtable _shadowConcreteMethods; - public IMethodNode ShadowConcreteMethod(MethodDesc method) + public ShadowConcreteMethodNode ShadowConcreteMethod(MethodDesc method) { return _shadowConcreteMethods.GetOrCreateValue(method); } + private sealed class ShadowNonConcreteMethodHashtable : LockFreeReaderHashtable + { + private readonly NodeFactory _factory; + public ShadowNonConcreteMethodHashtable(NodeFactory factory) => _factory = factory; + protected override bool CompareKeyToValue(MethodDesc key, ShadowNonConcreteMethodNode value) => key == value.Method; + protected override bool CompareValueToValue(ShadowNonConcreteMethodNode value1, ShadowNonConcreteMethodNode value2) => value1.Method == value2.Method; + protected override ShadowNonConcreteMethodNode CreateValueFromKey(MethodDesc key) => + new ShadowNonConcreteMethodNode(key, _factory.MethodEntrypoint(key.GetCanonMethodTarget(CanonicalFormKind.Specific))); + protected override int GetKeyHashCode(MethodDesc key) => key.GetHashCode(); + protected override int GetValueHashCode(ShadowNonConcreteMethodNode value) => value.Method.GetHashCode(); + } + + private ShadowNonConcreteMethodHashtable _shadowNonConcreteMethods; + public ShadowNonConcreteMethodNode ShadowNonConcreteMethod(MethodDesc method) + { + return _shadowNonConcreteMethods.GetOrCreateValue(method); + } + private static readonly string[][] s_helperEntrypointNames = new string[][] { new string[] { "System.Runtime.CompilerServices", "ClassConstructorRunner", "CheckStaticClassConstructionReturnGCStaticBase" }, new string[] { "System.Runtime.CompilerServices", "ClassConstructorRunner", "CheckStaticClassConstructionReturnNonGCStaticBase" }, diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs index 3ca290ed29b359..536c347ba3fdf4 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs @@ -126,10 +126,9 @@ private bool TriggersLazyStaticConstructor(NodeFactory factory) return factory.PreinitializationManager.HasLazyStaticConstructor(type.ConvertToCanonForm(CanonicalFormKind.Specific)); } - public IEnumerable InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation) + public IEnumerable InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation, bool isConcreteInstantiation) { DependencyList result = new DependencyList(); - var lookupContext = new GenericLookupResultContext(_dictionaryOwner, typeInstantiation, methodInstantiation); switch (_id) @@ -141,10 +140,11 @@ public IEnumerable InstantiateDependencies(NodeFactory fact // because that's where the class constructor context is. if (TriggersLazyStaticConstructor(factory)) { - result.Add( - new DependencyListEntry( - factory.GenericLookup.TypeNonGCStaticBase((TypeDesc)_target).GetTarget(factory, lookupContext), - "Dictionary dependency")); + var lookupTarget = factory.GenericLookup.TypeNonGCStaticBase((TypeDesc)_target).GetTarget(factory, lookupContext, isConcreteInstantiation); + if (lookupTarget != null) + { + result.Add(new DependencyListEntry(lookupTarget, "Dictionary dependency")); + } } } break; @@ -171,10 +171,12 @@ public IEnumerable InstantiateDependencies(NodeFactory fact try { - // All generic lookups depend on the thing they point to - result.Add(new DependencyListEntry( - _lookupSignature.GetTarget(factory, lookupContext), - "Dictionary dependency")); + var lookupTarget = _lookupSignature.GetTarget(factory, lookupContext, isConcreteInstantiation); + if (lookupTarget != null) + { + // All generic lookups depend on the thing they point to + result.Add(new DependencyListEntry(lookupTarget, "Dictionary dependency")); + } } catch (TypeSystemException) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs index f74267f9c26be4..310da2dd794634 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs @@ -65,6 +65,10 @@ public static void AddDependenciesDueToReflectability(ref DependencyList depende if (!method.IsAbstract) { + if (method.IsSharedByGenericInstantiations) + { + dependencies.Add(factory.ShadowNonConcreteMethod(method), "Shadow generic reflectable method"); + } dependencies.Add(factory.AddressTakenMethodEntrypoint(method), "Body of a reflectable method"); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index 07eca99c87b1fd..3c2c08c0005c33 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -64,7 +64,7 @@ protected override void ComputeDependencyNodeDependencies(List + + diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj index 86e59171f36c13..be32f6e22dc88c 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj @@ -91,6 +91,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs index 039b50d699aede..5c635bf69e3b4c 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs @@ -132,7 +132,7 @@ protected override void ComputeDependencyNodeDependencies(List + { + public static int MyStaticField; + } + + abstract class Base + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public virtual void VirtualMethod() + { + Container.MyStaticField = 3; + } + } + + sealed class Derived : Base + { + public override void VirtualMethod() + { + base.VirtualMethod(); + } + } + + interface IFoo { } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Base Get() => new Derived(); + + public static void Run() + { + // Testing for compilation failures due to missing GVM dependencies. + // See: https://github.com/dotnet/runtime/issues/120847 + Container.MyStaticField = 3; + Get().VirtualMethod(); + } + } + + // Additional testcases for GVM inlining dependencies with different inheritance patterns. + // See: https://github.com/dotnet/runtime/issues/120847 + class TestGvmInliningWithAbstract + { + class Container + { + public static int MyStaticField; + } + + abstract class Base + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public abstract void VirtualMethod(); + } + + class Mid : Base + { + public override void VirtualMethod() + { + Container.MyStaticField = 3; + } + } + + sealed class Derived : Mid + { + public override void VirtualMethod() + { + base.VirtualMethod(); + } + } + + interface IFoo { } + + [MethodImpl(MethodImplOptions.NoInlining)] + static Base Get() => new Derived(); + + public static void Run() + { + Container.MyStaticField = 5; + Get().VirtualMethod(); + } + } + + // Testcases for generic inlining with reflection. + // See: https://github.com/dotnet/runtime/issues/120847 + class TestGenericInliningWithReflection + { + class Container { } + + class Base + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public string BaseMethod() => new Container().ToString(); + } + + class Derived : Base + { + public string DerivedMethod() where T : class => BaseMethod(); + } + + static Type s_stringType = typeof(string); + + public static void Run() + { + object r = typeof(Derived).GetMethod(nameof(Derived.DerivedMethod)).MakeGenericMethod(s_stringType).Invoke(new Derived(), []); + if (r is null || !r.ToString().Contains("Container")) + throw new Exception(); + } + } + + class TestGenericInliningMethodGenericsOnly + { + class Container { } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + static string CalleeMethod() => new Container().ToString(); + + public static string CallerMethod() => CalleeMethod(); + + static Type s_stringType = typeof(string); + + public static void Run() + { + var r = typeof(TestGenericInliningMethodGenericsOnly).GetMethod(nameof(CallerMethod)).MakeGenericMethod(s_stringType).Invoke(null, []); + if (r is null || !r.ToString().Contains("Container")) + throw new Exception(); + } + } + + class TestGenericInliningTypeGenericsOnly + { + class Container { } + + class Base + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public string BaseMethod() + { + return new Container().ToString(); + } + } + + class Derived : Base + { + public string DerivedMethod() => base.BaseMethod(); + } + + static Type s_stringType = typeof(string); + + public static void Run() + { + var type = typeof(Derived<>).MakeGenericType(s_stringType); + var instance = Activator.CreateInstance(type); + var r = type.GetMethod("DerivedMethod").Invoke(instance, null); + if (r is null || !r.ToString().Contains("Container")) + throw new Exception(); + } + } + class Test99198Regression { delegate void Set(ref T t, IFoo ifoo);