From ce7ae9e13936e948ca3182c70bd7e7e36f7fa3f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 7 May 2024 10:52:16 +0200 Subject: [PATCH 1/7] wip --- .../Compiler/DelegateCreationInfo.cs | 6 +- .../AddressTakenMethodNode.cs | 85 +++++ .../DelegateTargetVirtualMethodNode.cs | 6 +- .../Compiler/DependencyAnalysis/EETypeNode.cs | 27 +- .../FatFunctionPointerNode.cs | 17 +- .../DependencyAnalysis/GenericLookupResult.cs | 15 +- .../DependencyAnalysis/NodeFactory.cs | 59 ++- .../ReadyToRunGenericHelperNode.cs | 7 + .../ReadyToRunHelperNode.cs | 3 + .../DependencyAnalysis/ReflectedMethodNode.cs | 5 - .../ReflectionInvokeMapNode.cs | 7 +- .../DependencyAnalysis/SealedVTableNode.cs | 16 +- .../Compiler/MetadataManager.cs | 14 + .../Compiler/ObjectDataInterner.cs | 154 ++++++++ .../Compiler/ObjectWriter/ObjectWriter.cs | 14 +- .../Compiler/TypePreinit.cs | 2 +- .../Compiler/UsageBasedMetadataManager.cs | 4 +- .../ILCompiler.Compiler.csproj | 2 + .../tools/aot/ILCompiler/repro/Program.cs | 336 +++++++++++++++++- 19 files changed, 743 insertions(+), 36 deletions(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs create mode 100644 src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs index 981d7a183b14b..d5ed36d379c30 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DelegateCreationInfo.cs @@ -141,10 +141,10 @@ public ISymbolNode GetTargetNode(NodeFactory factory) switch (_targetKind) { case TargetKind.CanonicalEntrypoint: - return factory.CanonicalEntrypoint(TargetMethod, TargetMethodIsUnboxingThunk); + return factory.AddressTakenMethodEntrypoint(TargetMethod, TargetMethodIsUnboxingThunk); case TargetKind.ExactCallableAddress: - return factory.ExactCallableAddress(TargetMethod, TargetMethodIsUnboxingThunk); + return factory.ExactCallableAddressTakenAddress(TargetMethod, TargetMethodIsUnboxingThunk); case TargetKind.InterfaceDispatch: return factory.InterfaceDispatchCell(TargetMethod); @@ -347,7 +347,7 @@ internal int CompareTo(DelegateCreationInfo other, TypeSystemComparer comparer) if (compare != 0) return compare; - compare = comparer.Compare(TargetMethod, other.TargetMethod); + compare = comparer.Compare(_targetMethod, other._targetMethod); if (compare != 0) return compare; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs new file mode 100644 index 0000000000000..de0657491bc53 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs @@ -0,0 +1,85 @@ +// 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 System.Diagnostics; + +using ILCompiler.DependencyAnalysisFramework; + +using Internal.Text; +using Internal.TypeSystem; + +namespace ILCompiler.DependencyAnalysis +{ + /// + /// Represents a method with address taken. Under normal circumstances, this node is not emitted + /// into the object file and instead references to it are replaced to refer to the underlying method body. + /// This is achieved through and . + /// However, if the underlying method body got folded together with another method due to identical method body folding + /// optimization, this node is not skipped and instead emits a jump stub. The purpose of the jump stub is to provide a + /// unique code address for the address taken method. + /// + internal sealed class AddressTakenMethodNode : JumpStubNode, IMethodNode, ISymbolNodeWithLinkage + { + private readonly IMethodNode _methodNode; + + public IMethodNode RealBody => _methodNode; + + public AddressTakenMethodNode(IMethodNode methodNode) + : base(methodNode) + { + _methodNode = methodNode; + } + + public MethodDesc Method => _methodNode.Method; + + protected override string GetName(NodeFactory factory) + { + return "Address taken method: " + _methodNode.GetMangledName(factory.NameMangler); + } + + public override bool ShouldSkipEmittingObjectNode(NodeFactory factory) + { + return factory.ObjectInterner.GetDeduplicatedSymbol(RealBody) == RealBody; + } + + public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => null; + public override IEnumerable SearchDynamicDependencies(List> markedNodes, int firstNode, NodeFactory context) => null; + public override bool InterestingForDynamicDependencyAnalysis => false; + public override bool HasDynamicDependencies => false; + public override bool HasConditionalStaticDependencies => false; + + public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) + { + // We use the same mangled name as the underlying real method body. + // This is okay since this node will go out of the way if the real body is marked + // and part of the graph. + _methodNode.AppendMangledName(nameMangler, sb); + } + + public override int CompareToImpl(ISortableNode other, CompilerComparer comparer) + { + return _methodNode.CompareToImpl(((AddressTakenMethodNode)other)._methodNode, comparer); + } + + public ISymbolNode NodeForLinkage(NodeFactory factory) + { + // If someone refers to this node but the target method still has a unique body, + // refer to the target method. + return factory.ObjectInterner.GetDeduplicatedSymbol(RealBody) == RealBody ? RealBody : this; + } + + public override bool RepresentsIndirectionCell + { + get + { + Debug.Assert(!_methodNode.RepresentsIndirectionCell); + return false; + } + } + + public override int ClassCode => 0xfab0355; + + public override bool IsShareable => ((ObjectNode)_methodNode).IsShareable; + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs index ac11ee086b3d0..7eb01c0b64a5b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs @@ -16,16 +16,18 @@ namespace ILCompiler.DependencyAnalysis public class DelegateTargetVirtualMethodNode : DependencyNodeCore { private readonly MethodDesc _method; + private readonly bool _reflected; - public DelegateTargetVirtualMethodNode(MethodDesc method) + public DelegateTargetVirtualMethodNode(MethodDesc method, bool reflected) { Debug.Assert(method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method); _method = method; + _reflected = reflected; } protected override string GetName(NodeFactory factory) { - return "Delegate target method: " + _method.ToString(); + return (_reflected ? "Reflected delegate target method:" : "Delegate target method: ") + _method.ToString(); } public override IEnumerable GetStaticDependencies(NodeFactory factory) => 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 7038e6f1dc208..31d5d6864802c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -426,6 +426,10 @@ public sealed override IEnumerable GetConditionalSt 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")); } if (impl.OwningType == defType) @@ -497,12 +501,19 @@ public sealed override IEnumerable GetConditionalSt MethodDesc defaultIntfMethod = implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); // If the interface method is used virtually, the implementation body is used + // BUGBUG: static virtual delegates? result.Add(new CombinedDependencyListEntry(factory.MethodEntrypoint(defaultIntfMethod), factory.VirtualMethodUse(interfaceMethod), "Interface method")); } else { // If the interface method is used virtually, the slot is used virtually result.Add(new CombinedDependencyListEntry(factory.VirtualMethodUse(implMethod), factory.VirtualMethodUse(interfaceMethod), "Interface method")); + + // If the interface method is virtual delegate target, the slot is virtual delegate target + result.Add(new CombinedDependencyListEntry( + factory.DelegateTargetVirtualMethod(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), + factory.DelegateTargetVirtualMethod(interfaceMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), + "Interface slot is delegate target")); } // If any of the implemented interfaces have variance, calls against compatible interface methods @@ -550,6 +561,11 @@ public sealed override IEnumerable GetConditionalSt } result.Add(new CombinedDependencyListEntry(factory.MethodEntrypoint(defaultIntfMethod), factory.VirtualMethodUse(interfaceMethod), "Interface method")); + result.Add(new CombinedDependencyListEntry( + factory.AddressTakenMethodEntrypoint(defaultIntfMethod), + factory.DelegateTargetVirtualMethod(interfaceMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), + "Slot is delegate target")); + factory.MetadataManager.NoteOverridingMethod(interfaceMethod, implMethod); factory.MetadataManager.GetDependenciesForOverridingMethod(ref result, factory, interfaceMethod, implMethod); @@ -1103,9 +1119,14 @@ private void OutputVirtualSlots(NodeFactory factory, ref ObjectDataBuilder objDa && 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); + IMethodNode implSymbol; + if (canUseTentativeEntrypoint) + implSymbol = factory.TentativeMethodEntrypoint(canonImplMethod, implMethod.OwningType.IsValueType); + else if (factory.DelegateTargetVirtualMethod(declMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)).Marked) + implSymbol = factory.AddressTakenMethodEntrypoint(canonImplMethod, implMethod.OwningType.IsValueType); + else + implSymbol = factory.MethodEntrypoint(canonImplMethod, implMethod.OwningType.IsValueType); + objData.EmitPointerReloc(implSymbol); } else diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FatFunctionPointerNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FatFunctionPointerNode.cs index 8fa6109155c6a..7428f4a8aa5d3 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FatFunctionPointerNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FatFunctionPointerNode.cs @@ -15,22 +15,24 @@ namespace ILCompiler.DependencyAnalysis /// public class FatFunctionPointerNode : DehydratableObjectNode, IMethodNode, ISymbolDefinitionNode { - private bool _isUnboxingStub; + private readonly bool _isUnboxingStub; + private readonly bool _isAddressTaken; public bool IsUnboxingStub => _isUnboxingStub; - public FatFunctionPointerNode(MethodDesc methodRepresented, bool isUnboxingStub) + public FatFunctionPointerNode(MethodDesc methodRepresented, bool isUnboxingStub, bool addressTaken) { // We should not create these for methods that don't have a canonical method body Debug.Assert(methodRepresented.GetCanonMethodTarget(CanonicalFormKind.Specific) != methodRepresented); Method = methodRepresented; _isUnboxingStub = isUnboxingStub; + _isAddressTaken = addressTaken; } public void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) { - string prefix = _isUnboxingStub ? "__fatunboxpointer_" : "__fatpointer_"; + string prefix = $"__fat{(_isUnboxingStub ? "unbox" : "")}{(_isAddressTaken ? "addresstaken" : "")}pointer_"; sb.Append(prefix).Append(nameMangler.GetMangledMethodName(Method)); } @@ -67,7 +69,10 @@ protected override ObjectData GetDehydratableData(NodeFactory factory, bool relo MethodDesc canonMethod = Method.GetCanonMethodTarget(CanonicalFormKind.Specific); // Pointer to the canonical body of the method - builder.EmitPointerReloc(factory.MethodEntrypoint(canonMethod, _isUnboxingStub)); + ISymbolNode target = _isAddressTaken + ? factory.AddressTakenMethodEntrypoint(canonMethod, _isUnboxingStub) + : factory.MethodEntrypoint(canonMethod, _isUnboxingStub); + builder.EmitPointerReloc(target); // Find out what's the context to use ISortableSymbolNode contextParameter; @@ -97,6 +102,10 @@ public override int CompareToImpl(ISortableNode other, CompilerComparer comparer if (compare != 0) return compare; + compare = _isAddressTaken.CompareTo(((FatFunctionPointerNode)other)._isAddressTaken); + if (compare != 0) + return compare; + return comparer.Compare(Method, ((FatFunctionPointerNode)other).Method); } } 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 5b37ad7e0acc2..37a38e8448f65 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs @@ -447,7 +447,8 @@ public MethodEntryGenericLookupResult(MethodDesc method, bool isUnboxingThunk) public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultContext dictionary) { MethodDesc instantiatedMethod = _method.GetNonRuntimeDeterminedMethodFromRuntimeDeterminedMethodViaSubstitution(dictionary.TypeInstantiation, dictionary.MethodInstantiation); - return factory.FatFunctionPointer(instantiatedMethod, _isUnboxingThunk); + // TODO-SIZE: this is address taken only in the delegate target case + return factory.FatAddressTakenFunctionPointer(instantiatedMethod, _isUnboxingThunk); } public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb) @@ -473,10 +474,11 @@ public override NativeLayoutVertexNode TemplateDictionaryNode(NodeFactory factor // bool getUnboxingStubNode = _isUnboxingThunk && !canonMethod.IsCanonicalMethod(CanonicalFormKind.Universal); + // TODO-SIZE: this is address taken only in the delegate target case return factory.NativeLayout.MethodEntrypointDictionarySlot( _method, _isUnboxingThunk, - factory.MethodEntrypoint(canonMethod, getUnboxingStubNode)); + factory.AddressTakenMethodEntrypoint(canonMethod, getUnboxingStubNode)); } protected override int CompareToImpl(GenericLookupResult other, TypeSystemComparer comparer) @@ -884,20 +886,21 @@ public override ISymbolNode GetTarget(NodeFactory factory, GenericLookupResultCo factory.MetadataManager.NoteOverridingMethod(_constrainedMethod, implMethod); + // TODO-SIZE: this is address taken only in the delegate target case if (implMethod.Signature.IsStatic) { if (implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific).IsSharedByGenericInstantiations) - return factory.ExactCallableAddress(implMethod); + return factory.ExactCallableAddressTakenAddress(implMethod); else - return factory.MethodEntrypoint(implMethod); + return factory.AddressTakenMethodEntrypoint(implMethod); } else if (implMethod.HasInstantiation) { - return factory.ExactCallableAddress(implMethod); + return factory.ExactCallableAddressTakenAddress(implMethod); } else { - return factory.CanonicalEntrypoint(implMethod); + return factory.AddressTakenMethodEntrypoint(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)); } } 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 4f7b48f33030e..6fa51d6c68e59 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -25,6 +25,7 @@ public abstract partial class NodeFactory private VTableSliceProvider _vtableSliceProvider; private DictionaryLayoutProvider _dictionaryLayoutProvider; private InlinedThreadStatics _inlinedThreadStatics; + private ObjectDataInterner _interner; protected readonly ImportedNodeProvider _importedNodeProvider; private bool _markingComplete; @@ -115,6 +116,14 @@ public InteropStubManager InteropStubManager get; } + internal ObjectDataInterner ObjectInterner + { + get + { + return _interner ??= new ObjectDataInterner(this); + } + } + /// /// Return true if the type is not permitted by the rules of the runtime to have an MethodTable. /// The implementation here is not intended to be complete, but represents many conditions @@ -283,7 +292,12 @@ private void CreateNodeCaches() _fatFunctionPointers = new NodeCache(method => { - return new FatFunctionPointerNode(method.Method, method.IsUnboxingStub); + return new FatFunctionPointerNode(method.Method, method.IsUnboxingStub, addressTaken: false); + }); + + _fatAddressTakenFunctionPointers = new NodeCache(method => + { + return new FatFunctionPointerNode(method.Method, method.IsUnboxingStub, addressTaken: true); }); _gvmDependenciesNode = new NodeCache(method => @@ -306,9 +320,19 @@ private void CreateNodeCaches() return new TypeGVMEntriesNode(type); }); + _addressTakenMethods = new NodeCache(method => + { + return new AddressTakenMethodNode(MethodEntrypoint(method, unboxingStub: false)); + }); + + _reflectedDelegateTargetMethods = new NodeCache(method => + { + return new DelegateTargetVirtualMethodNode(method, reflected: true); + }); + _delegateTargetMethods = new NodeCache(method => { - return new DelegateTargetVirtualMethodNode(method); + return new DelegateTargetVirtualMethodNode(method, reflected: false); }); _reflectedDelegates = new NodeCache(type => @@ -971,6 +995,13 @@ public IMethodNode FatFunctionPointer(MethodDesc method, bool isUnboxingStub = f return _fatFunctionPointers.GetOrAdd(new MethodKey(method, isUnboxingStub)); } + private NodeCache _fatAddressTakenFunctionPointers; + + public IMethodNode FatAddressTakenFunctionPointer(MethodDesc method, bool isUnboxingStub = false) + { + return _fatAddressTakenFunctionPointers.GetOrAdd(new MethodKey(method, isUnboxingStub)); + } + public IMethodNode ExactCallableAddress(MethodDesc method, bool isUnboxingStub = false) { MethodDesc canonMethod = method.GetCanonMethodTarget(CanonicalFormKind.Specific); @@ -980,6 +1011,15 @@ public IMethodNode ExactCallableAddress(MethodDesc method, bool isUnboxingStub = return MethodEntrypoint(method, isUnboxingStub); } + public IMethodNode ExactCallableAddressTakenAddress(MethodDesc method, bool isUnboxingStub = false) + { + MethodDesc canonMethod = method.GetCanonMethodTarget(CanonicalFormKind.Specific); + if (method != canonMethod) + return FatAddressTakenFunctionPointer(method, isUnboxingStub); + else + return AddressTakenMethodEntrypoint(method, isUnboxingStub); + } + public IMethodNode CanonicalEntrypoint(MethodDesc method, bool isUnboxingStub = false) { MethodDesc canonMethod = method.GetCanonMethodTarget(CanonicalFormKind.Specific); @@ -1013,6 +1053,21 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type) return _gvmTableEntries.GetOrAdd(type); } + private NodeCache _addressTakenMethods; + public IMethodNode AddressTakenMethodEntrypoint(MethodDesc method, bool unboxingStub = false) + { + if (unboxingStub) + return MethodEntrypoint(method, unboxingStub); + + return _addressTakenMethods.GetOrAdd(method); + } + + private NodeCache _reflectedDelegateTargetMethods; + public DelegateTargetVirtualMethodNode ReflectedDelegateTargetVirtualMethod(MethodDesc method) + { + return _reflectedDelegateTargetMethods.GetOrAdd(method); + } + private NodeCache _delegateTargetMethods; public DelegateTargetVirtualMethodNode DelegateTargetVirtualMethod(MethodDesc method) { 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 284adf484ca6f..4d34bd1a02327 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunGenericHelperNode.cs @@ -222,6 +222,13 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact dependencies.Add(new DependencyListEntry(dependency, "GenericLookupResultDependency")); } + if (_id == ReadyToRunHelperId.DelegateCtor) + { + var delegateCreationInfo = (DelegateCreationInfo)_target; + MethodDesc targetMethod = delegateCreationInfo.PossiblyUnresolvedTargetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencies, factory, delegateCreationInfo.DelegateType, targetMethod); + } + return dependencies; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs index ab9ccafa2db95..071c11a9820d7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs @@ -156,6 +156,9 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact #endif } + factory.MetadataManager.GetDependenciesDueToDelegateCreation(ref dependencyList, factory, info.DelegateType, + info.PossiblyUnresolvedTargetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)); + return dependencyList; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedMethodNode.cs index 6fae955a5e5c9..a04579eb49953 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedMethodNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectedMethodNode.cs @@ -72,11 +72,6 @@ public override IEnumerable GetStaticDependencies(NodeFacto dependencies.Add(factory.VirtualMethodUse(slotDefiningMethod), "Virtually callable reflectable method"); } } - - if (!_method.IsAbstract) - { - dependencies.Add(factory.MethodEntrypoint(_method), "Body of a reflectable method"); - } } return dependencies; 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 e9d5b45d3ee80..a94fccb1cb231 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ReflectionInvokeMapNode.cs @@ -63,6 +63,11 @@ public static void AddDependenciesDueToReflectability(ref DependencyList depende if (method.OwningType.IsValueType && !method.Signature.IsStatic) dependencies.Add(factory.MethodEntrypoint(method, unboxingStub: true), "Reflection unboxing stub"); + if (!method.IsAbstract) + { + dependencies.Add(factory.AddressTakenMethodEntrypoint(method), "Body of a reflectable method"); + } + // If the method is defined in a different module than this one, a metadata token isn't known for performing the reference // Use a name/sig reference instead. if (!factory.MetadataManager.WillUseMetadataTokenToReferenceMethod(method)) @@ -201,7 +206,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) { vertex = writer.GetTuple(vertex, writer.GetUnsignedConstant(_externalReferences.GetIndex( - factory.MethodEntrypoint(method, + factory.AddressTakenMethodEntrypoint(method, unboxingStub: method.OwningType.IsValueType && !method.Signature.IsStatic)))); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs index 69f02c2406cfa..e77aee0989674 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs @@ -132,7 +132,13 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) if (implMethod.CanMethodBeInSealedVTable(factory)) { - IMethodNode node = factory.MethodEntrypoint(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !implMethod.Signature.IsStatic && declType.IsValueType); + IMethodNode node; + + if (factory.DelegateTargetVirtualMethod(virtualSlots[i].GetCanonMethodTarget(CanonicalFormKind.Specific)).Marked) + node = factory.AddressTakenMethodEntrypoint(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !implMethod.Signature.IsStatic && declType.IsValueType); + else + node = factory.MethodEntrypoint(implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !implMethod.Signature.IsStatic && declType.IsValueType); + _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(implMethod, node)); } } @@ -186,6 +192,7 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) if (targetMethod.CanMethodBeInSealedVTable(factory) || implMethod.Signature.IsStatic) { + // BUGBUG IMethodNode node = factory.MethodEntrypoint(targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !targetMethod.Signature.IsStatic && declType.IsValueType); _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(targetMethod, node)); } @@ -215,7 +222,12 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) _nonRelocationDependencies.Add(factory.InterfaceUse(declTypeRuntimeInterfaces[i].GetTypeDefinition()), "Interface with shared default methods folows this"); } } - IMethodNode node = factory.MethodEntrypoint(canonImplMethod, unboxingStub: implMethod.OwningType.IsValueType && !implMethod.Signature.IsStatic); + + IMethodNode node; + if (factory.DelegateTargetVirtualMethod(declMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)).Marked) + node = factory.AddressTakenMethodEntrypoint(canonImplMethod, unboxingStub: implMethod.OwningType.IsValueType && !implMethod.Signature.IsStatic); + else + node = factory.MethodEntrypoint(canonImplMethod, unboxingStub: implMethod.OwningType.IsValueType && !implMethod.Signature.IsStatic); _sealedVTableEntries.Add(SealedVTableEntry.FromDefaultInterfaceMethod(implMethod, providingInterfaceDefinitionType, node)); } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index bd70b1660db3f..a159341d12c00 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -562,6 +562,15 @@ public virtual void GetDependenciesDueToLdToken(ref DependencyList dependencies, // RuntimeFieldHandle data structure. } + public void GetDependenciesDueToDelegateCreation(ref DependencyList dependencies, NodeFactory factory, TypeDesc delegateType, MethodDesc target) + { + if (target.IsVirtual) + { + dependencies ??= new DependencyList(); + dependencies.Add(factory.DelegateTargetVirtualMethod(target), "Delegate to a virtual method created"); + } + } + /// /// This method is an extension point that can provide additional metadata-based dependencies to delegate targets. /// @@ -699,6 +708,11 @@ protected void ComputeMetadata( (GetMetadataCategory(method) & MetadataCategory.RuntimeMapping) != 0) continue; + // If the method will be folded, no need to emit stack trace info for this one + ISymbolNode internedBody = factory.ObjectInterner.GetDeduplicatedSymbol(methodBody); + if (internedBody != methodBody) + continue; + MethodStackTraceVisibilityFlags stackVisibility = _stackTraceEmissionPolicy.GetMethodVisibility(method); bool isHidden = (stackVisibility & MethodStackTraceVisibilityFlags.IsHidden) != 0; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs new file mode 100644 index 0000000000000..e8261bfbf5e44 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs @@ -0,0 +1,154 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; + +using ILCompiler.DependencyAnalysis; + +using Debug = System.Diagnostics.Debug; + +namespace ILCompiler +{ + internal sealed class ObjectDataInterner + { + private readonly NodeFactory _factory; + + private readonly Dictionary _symbolRemapping; + + public ObjectDataInterner(NodeFactory factory) + { + Debug.Assert(factory.MarkingComplete); + + _factory = factory; + + var symbolRemapping = new Dictionary(); + var methodHash = new HashSet(); + + foreach (IMethodBodyNode body in factory.MetadataManager.GetCompiledMethodBodies()) + { + var key = new MethodInternKey(body, this); + if (methodHash.TryGetValue(key, out MethodInternKey found)) + { + symbolRemapping.Add(body, found.Method); + } + else + { + methodHash.Add(key); + } + } + + _symbolRemapping = symbolRemapping; + } + + public ISymbolNode GetDeduplicatedSymbol(ISymbolNode original) + { + ISymbolNode target = original; + if (target is ISymbolNodeWithLinkage symbolWithLinkage) + target = symbolWithLinkage.NodeForLinkage(_factory); + + return _symbolRemapping.TryGetValue(target, out ISymbolNode result) ? result : original; + } + + private sealed class MethodInternKey : IEquatable + { + private readonly ObjectDataInterner _parent; + private readonly IMethodBodyNode _node; + private readonly int _hashCode; + + public IMethodBodyNode Method => _node; + + public MethodInternKey(IMethodBodyNode node, ObjectDataInterner parent) + { + ObjectNode.ObjectData data = ((ObjectNode)node).GetData(parent._factory, relocsOnly: false); + + var hashCode = default(HashCode); + hashCode.AddBytes(data.Data); + + var nodeWithCodeInfo = (INodeWithCodeInfo)node; + + hashCode.AddBytes(nodeWithCodeInfo.GCInfo); + + foreach (FrameInfo fi in nodeWithCodeInfo.FrameInfos) + hashCode.Add(fi.GetHashCode()); + + ObjectNode.ObjectData ehData = nodeWithCodeInfo.EHInfo?.GetData(parent._factory, relocsOnly: false); + + if (ehData is not null) + hashCode.AddBytes(ehData.Data); + + _parent = parent; + _hashCode = hashCode.ToHashCode(); + _node = node; + } + + public override bool Equals(object obj) => obj is MethodInternKey other && Equals(other); + + public override int GetHashCode() => _hashCode; + + private static bool AreSame(ReadOnlySpan o1, ReadOnlySpan o2) => o1.SequenceEqual(o2); + + private static bool AreSame(ObjectNode.ObjectData o1, ObjectNode.ObjectData o2) + { + if (AreSame(o1.Data, o2.Data) && o1.Relocs.Length == o2.Relocs.Length) + { + for (int i = 0; i < o1.Relocs.Length; i++) + { + ref Relocation r1 = ref o1.Relocs[i]; + ref Relocation r2 = ref o2.Relocs[i]; + if (r1.RelocType != r2.RelocType + || r1.Offset != r2.Offset + // TODO: should be comparing target after folding to catch more sameness + || r1.Target != r2.Target) + { + return false; + } + } + + return true; + } + + return false; + } + + public bool Equals(MethodInternKey other) + { + if (other._hashCode != _hashCode) + return false; + + ObjectNode.ObjectData o1data = ((ObjectNode)_node).GetData(_parent._factory, relocsOnly: false); + ObjectNode.ObjectData o2data = ((ObjectNode)other._node).GetData(_parent._factory, relocsOnly: false); + + if (!AreSame(o1data, o2data)) + return false; + + var o1codeinfo = (INodeWithCodeInfo)_node; + var o2codeinfo = (INodeWithCodeInfo)other._node; + if (!AreSame(o1codeinfo.GCInfo, o2codeinfo.GCInfo)) + return false; + + FrameInfo[] o1frames = o1codeinfo.FrameInfos; + FrameInfo[] o2frames = o2codeinfo.FrameInfos; + if (o1frames.Length != o2frames.Length) + return false; + + for (int i = 0; i < o1frames.Length; i++) + { + if (!o1frames[i].Equals(o2frames[i])) + return false; + } + + MethodExceptionHandlingInfoNode o1eh = o1codeinfo.EHInfo; + MethodExceptionHandlingInfoNode o2eh = o2codeinfo.EHInfo; + + if (o1eh == o2eh) + return true; + + if (o1eh == null || o2eh == null) + return false; + + return AreSame(o1eh.GetData(_parent._factory, relocsOnly: false), o2eh.GetData(_parent._factory, relocsOnly: false)); + } + } + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs index 086cd56bd3b07..d824f63685caa 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs @@ -395,12 +395,16 @@ private void EmitObject(string objectFilePath, IReadOnlyCollection.CombinedDependencyListEntry( targetVirtualMethod, @@ -615,7 +615,7 @@ public override void GetDependenciesForOverridingMethod(ref CombinedDependencyLi dependencies ??= new CombinedDependencyList(); dependencies.Add(new DependencyNodeCore.CombinedDependencyListEntry( factory.ReflectedMethod(impl.GetCanonMethodTarget(CanonicalFormKind.Specific)), - factory.DelegateTargetVirtualMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), + factory.ReflectedDelegateTargetVirtualMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Virtual method declaration is reflectable")); } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj index a895dbe1726fa..345d8c9ab67f2 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj @@ -347,6 +347,7 @@ + @@ -454,6 +455,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs index 9e71394c3732a..28b3f2aa72c87 100644 --- a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs @@ -2,11 +2,345 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Runtime.CompilerServices; class Program { + class SimpleDelegateTargets + { + public static object Return1DelStatic() => new object(); + public static object Return2DelStatic() => new object(); + + public object Return1DelInstance() => new object(); + public object Return2DelInstance() => new object(); + } + + class BaseDelegateTargets + { + public virtual object Return1Del() => new object(); + public virtual object Return2Del() => new object(); + } + + class DerivedDelegateTargets : BaseDelegateTargets + { + public override object Return1Del() => new object(); + public override object Return2Del() => new object(); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static BaseDelegateTargets GetInstance() => new DerivedDelegateTargets(); + } + + class PreinitializedDelegateTargets + { + public static readonly PreinitializedDelegateTargets s_o = new PreinitializedDelegateTargets(); + public static readonly Func s_f1 = s_o.Return1Del; + public static readonly Func s_f2 = s_o.Return2Del; + + public object Return1Del() => new object(); + public object Return2Del() => new object(); + } + + interface IInterfaceTarget + { + object Return1Del(); + object Return2Del(); + } + + class InterfaceImplementedTargets : IInterfaceTarget + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IInterfaceTarget GetInstance() => new InterfaceImplementedTargets(); + public object Return1Del() => new object(); + public object Return2Del() => new object(); + } + + interface IDefaultInterfaceTarget + { + object Return1Del() => new object(); + object Return2Del() => new object(); + } + + class DefaultInterfaceImplementedTargets : IDefaultInterfaceTarget + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IDefaultInterfaceTarget GetInstance() => new DefaultInterfaceImplementedTargets(); + } + + interface IGenericDefaultInterfaceTarget + { + object Return1Del() => new object(); + object Return2Del() => new object(); + } + + class GenericDefaultInterfaceImplementedTargets : IGenericDefaultInterfaceTarget + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IGenericDefaultInterfaceTarget GetInstance() => new GenericDefaultInterfaceImplementedTargets(); + } + + interface IInterfaceToBeDefaultImplemented + { + object Return1Del(); + object Return2Del(); + } + + interface IDefaultImplementingInterface : IInterfaceToBeDefaultImplemented + { + object IInterfaceToBeDefaultImplemented.Return1Del() => new object(); + object IInterfaceToBeDefaultImplemented.Return2Del() => new object(); + } + + class DefaultInterfaceImplementedTargetsFromOther : IDefaultImplementingInterface + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IInterfaceToBeDefaultImplemented GetInstance() => new DefaultInterfaceImplementedTargetsFromOther(); + } + + abstract class AbstractBaseClass + { + public virtual object Return1Del() => new object(); + public virtual object Return2Del() => new object(); + } + + class ClassDerivedFromAbstract : AbstractBaseClass + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static AbstractBaseClass GetInstance() => new ClassDerivedFromAbstract(); + } + + interface IStaticInterfaceTargets + { + static abstract object Return1Del(); + static abstract object Return2Del(); + + public static void Test() where T : IStaticInterfaceTargets + { + { + Func f1 = T.Return1Del; + Func f2 = T.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = T.Return1Del; + Func f2 = T.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + } + } + + class StaticInterfaceImplementedTargetsClass : IStaticInterfaceTargets + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + struct StaticInterfaceImplementedTargetsStruct : IStaticInterfaceTargets + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + class ReflectedOnType + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + static object Return1() => new object(); + static object Return2() => new object(); + + class ConstructedInGenericContextToGenerics + { + static object Return1Del() => new object(); + static object Return2Del() => new object(); + + public static void Test() + { + { + Func f1 = ConstructedInGenericContextToGenerics.Return1Del; + Func f2 = ConstructedInGenericContextToGenerics.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = ConstructedInGenericContextToGenerics.Return1Del; + Func f2 = ConstructedInGenericContextToGenerics.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + } + } + static void Main() { - Console.WriteLine("Hello world"); + Return1(); + Return2(); + + // Static method + { + Func f1 = SimpleDelegateTargets.Return1DelStatic; + Func f2 = SimpleDelegateTargets.Return2DelStatic; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = SimpleDelegateTargets.Return1DelStatic; + Func f2 = SimpleDelegateTargets.Return1DelStatic; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Instance method + { + var o = new SimpleDelegateTargets(); + Func f1 = o.Return1DelInstance; + Func f2 = o.Return2DelInstance; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = new SimpleDelegateTargets(); + Func f1 = o.Return1DelInstance; + Func f2 = o.Return1DelInstance; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Virtual method + { + var o = DerivedDelegateTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = DerivedDelegateTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Method from a frozen delegate + { + if (PreinitializedDelegateTargets.s_f1.Equals(PreinitializedDelegateTargets.s_f2)) + throw new Exception(); + } + + // Interface method + { + var o = InterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = InterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Default interface method + { + var o = DefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = DefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Default generic interface method + { + var o = GenericDefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = GenericDefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Default interface method + { + var o = DefaultInterfaceImplementedTargetsFromOther.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = DefaultInterfaceImplementedTargetsFromOther.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Interaction with virtuals on abstract classes optimization + { + var o = ClassDerivedFromAbstract.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = ClassDerivedFromAbstract.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + IStaticInterfaceTargets.Test(); + IStaticInterfaceTargets.Test(); + + { + var f1 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return1Del)).CreateDelegate>(); + var f2 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return2Del)).CreateDelegate>(); + if (f1.Equals(f2)) + throw new Exception(); + } + + ConstructedInGenericContextToGenerics.Test(); + ConstructedInGenericContextToGenerics.Test(); } } From c0da1086d63f11e8c758c986a893b98bc0054507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 9 May 2024 11:23:21 +0200 Subject: [PATCH 2/7] wip --- .../Compiler/ObjectDataInterner.cs | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs index e8261bfbf5e44..bfe87928ffe47 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs @@ -23,11 +23,11 @@ public ObjectDataInterner(NodeFactory factory) _factory = factory; var symbolRemapping = new Dictionary(); - var methodHash = new HashSet(); + var methodHash = new HashSet(new MethodInternComparer(factory)); foreach (IMethodBodyNode body in factory.MetadataManager.GetCompiledMethodBodies()) { - var key = new MethodInternKey(body, this); + var key = new MethodInternKey(body, factory); if (methodHash.TryGetValue(key, out MethodInternKey found)) { symbolRemapping.Add(body, found.Method); @@ -50,17 +50,14 @@ public ISymbolNode GetDeduplicatedSymbol(ISymbolNode original) return _symbolRemapping.TryGetValue(target, out ISymbolNode result) ? result : original; } - private sealed class MethodInternKey : IEquatable + private sealed class MethodInternKey { - private readonly ObjectDataInterner _parent; - private readonly IMethodBodyNode _node; - private readonly int _hashCode; + public IMethodBodyNode Method { get; } + public int HashCode { get; } - public IMethodBodyNode Method => _node; - - public MethodInternKey(IMethodBodyNode node, ObjectDataInterner parent) + public MethodInternKey(IMethodBodyNode node, NodeFactory factory) { - ObjectNode.ObjectData data = ((ObjectNode)node).GetData(parent._factory, relocsOnly: false); + ObjectNode.ObjectData data = ((ObjectNode)node).GetData(factory, relocsOnly: false); var hashCode = default(HashCode); hashCode.AddBytes(data.Data); @@ -72,19 +69,24 @@ public MethodInternKey(IMethodBodyNode node, ObjectDataInterner parent) foreach (FrameInfo fi in nodeWithCodeInfo.FrameInfos) hashCode.Add(fi.GetHashCode()); - ObjectNode.ObjectData ehData = nodeWithCodeInfo.EHInfo?.GetData(parent._factory, relocsOnly: false); + ObjectNode.ObjectData ehData = nodeWithCodeInfo.EHInfo?.GetData(factory, relocsOnly: false); if (ehData is not null) hashCode.AddBytes(ehData.Data); - _parent = parent; - _hashCode = hashCode.ToHashCode(); - _node = node; + HashCode = hashCode.ToHashCode(); + Method = node; } + } + + private sealed class MethodInternComparer : IEqualityComparer + { + private readonly NodeFactory _factory; - public override bool Equals(object obj) => obj is MethodInternKey other && Equals(other); + public MethodInternComparer(NodeFactory factory) + => (_factory) = (factory); - public override int GetHashCode() => _hashCode; + public int GetHashCode(MethodInternKey key) => key.HashCode; private static bool AreSame(ReadOnlySpan o1, ReadOnlySpan o2) => o1.SequenceEqual(o2); @@ -111,19 +113,19 @@ private static bool AreSame(ObjectNode.ObjectData o1, ObjectNode.ObjectData o2) return false; } - public bool Equals(MethodInternKey other) + public bool Equals(MethodInternKey a, MethodInternKey b) { - if (other._hashCode != _hashCode) + if (a.HashCode != b.HashCode) return false; - ObjectNode.ObjectData o1data = ((ObjectNode)_node).GetData(_parent._factory, relocsOnly: false); - ObjectNode.ObjectData o2data = ((ObjectNode)other._node).GetData(_parent._factory, relocsOnly: false); + ObjectNode.ObjectData o1data = ((ObjectNode)a.Method).GetData(_factory, relocsOnly: false); + ObjectNode.ObjectData o2data = ((ObjectNode)b.Method).GetData(_factory, relocsOnly: false); if (!AreSame(o1data, o2data)) return false; - var o1codeinfo = (INodeWithCodeInfo)_node; - var o2codeinfo = (INodeWithCodeInfo)other._node; + var o1codeinfo = (INodeWithCodeInfo)a.Method; + var o2codeinfo = (INodeWithCodeInfo)b.Method; if (!AreSame(o1codeinfo.GCInfo, o2codeinfo.GCInfo)) return false; @@ -147,7 +149,7 @@ public bool Equals(MethodInternKey other) if (o1eh == null || o2eh == null) return false; - return AreSame(o1eh.GetData(_parent._factory, relocsOnly: false), o2eh.GetData(_parent._factory, relocsOnly: false)); + return AreSame(o1eh.GetData(_factory, relocsOnly: false), o2eh.GetData(_factory, relocsOnly: false)); } } } From e896ca6e3aa7e34a09ad4bed49fc00f4acc858bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Fri, 10 May 2024 11:25:10 +0200 Subject: [PATCH 3/7] wip --- .../ExactMethodInstantiationsNode.cs | 6 +- .../NativeLayoutVertexNode.cs | 3 +- .../Compiler/ObjectDataInterner.cs | 4 + .../tools/aot/ILCompiler/repro/Program.cs | 336 +-------------- .../StackTraceMetadata/BodyFoldingTest.cs | 397 ++++++++++++++++++ .../StackTraceMetadata/StackTraceMetadata.cs | 3 +- .../StackTraceMetadata.csproj | 1 + .../StackTraceMetadata_Stripped.csproj | 1 + 8 files changed, 412 insertions(+), 339 deletions(-) create mode 100644 src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs index 46d38a7b9857b..03dff7801d8ad 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs @@ -58,7 +58,8 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false) // Get the method pointer vertex bool getUnboxingStub = method.OwningType.IsValueType && !method.Signature.IsStatic; - IMethodNode methodEntryPointNode = factory.MethodEntrypoint(method, getUnboxingStub); + // TODO-SIZE: we need address taken entrypoint only if this was a target of a delegate + IMethodNode methodEntryPointNode = factory.AddressTakenMethodEntrypoint(method, getUnboxingStub); Vertex methodPointer = nativeWriter.GetUnsignedConstant(_externalReferences.GetIndex(methodEntryPointNode)); // Get native layout vertices for the declaring type @@ -112,7 +113,8 @@ public static void GetExactMethodInstantiationDependenciesForMethod(ref Dependen // Method entry point dependency bool getUnboxingStub = method.OwningType.IsValueType && !method.Signature.IsStatic; - IMethodNode methodEntryPointNode = factory.MethodEntrypoint(method, getUnboxingStub); + // TODO-SIZE: we need address taken entrypoint only if this was a target of a delegate + IMethodNode methodEntryPointNode = factory.AddressTakenMethodEntrypoint(method, getUnboxingStub); dependencies.Add(new DependencyListEntry(methodEntryPointNode, "Exact method instantiation entry")); // Get native layout dependencies for the declaring type 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 60c6158de41a2..16ef36907af6a 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs @@ -752,7 +752,8 @@ protected override IMethodNode GetMethodEntrypointNode(NodeFactory factory, out { Debug.Assert(NeedsEntrypoint(_method)); unboxingStub = _method.OwningType.IsValueType && !_method.Signature.IsStatic; - IMethodNode methodEntryPointNode = factory.MethodEntrypoint(_method, unboxingStub); + // TODO-SIZE: this is only address taken if it's a target of a delegate + IMethodNode methodEntryPointNode = factory.AddressTakenMethodEntrypoint(_method, unboxingStub); // Note: We don't set the IsUnboxingStub flag on template methods (all template lookups performed at runtime are performed with this flag not set, // since it can't always be conveniently computed for a concrete method before looking up its template) unboxingStub = false; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs index bfe87928ffe47..84f1d59845ffb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs @@ -27,6 +27,10 @@ public ObjectDataInterner(NodeFactory factory) foreach (IMethodBodyNode body in factory.MetadataManager.GetCompiledMethodBodies()) { + // We don't track special unboxing thunks as virtual method use related so ignore them + if (body is ISpecialUnboxThunkNode unboxThunk && unboxThunk.IsSpecialUnboxingThunk) + continue; + var key = new MethodInternKey(body, factory); if (methodHash.TryGetValue(key, out MethodInternKey found)) { diff --git a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs index 28b3f2aa72c87..9e71394c3732a 100644 --- a/src/coreclr/tools/aot/ILCompiler/repro/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/repro/Program.cs @@ -2,345 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Runtime.CompilerServices; class Program { - class SimpleDelegateTargets - { - public static object Return1DelStatic() => new object(); - public static object Return2DelStatic() => new object(); - - public object Return1DelInstance() => new object(); - public object Return2DelInstance() => new object(); - } - - class BaseDelegateTargets - { - public virtual object Return1Del() => new object(); - public virtual object Return2Del() => new object(); - } - - class DerivedDelegateTargets : BaseDelegateTargets - { - public override object Return1Del() => new object(); - public override object Return2Del() => new object(); - - [MethodImpl(MethodImplOptions.NoInlining)] - public static BaseDelegateTargets GetInstance() => new DerivedDelegateTargets(); - } - - class PreinitializedDelegateTargets - { - public static readonly PreinitializedDelegateTargets s_o = new PreinitializedDelegateTargets(); - public static readonly Func s_f1 = s_o.Return1Del; - public static readonly Func s_f2 = s_o.Return2Del; - - public object Return1Del() => new object(); - public object Return2Del() => new object(); - } - - interface IInterfaceTarget - { - object Return1Del(); - object Return2Del(); - } - - class InterfaceImplementedTargets : IInterfaceTarget - { - [MethodImpl(MethodImplOptions.NoInlining)] - public static IInterfaceTarget GetInstance() => new InterfaceImplementedTargets(); - public object Return1Del() => new object(); - public object Return2Del() => new object(); - } - - interface IDefaultInterfaceTarget - { - object Return1Del() => new object(); - object Return2Del() => new object(); - } - - class DefaultInterfaceImplementedTargets : IDefaultInterfaceTarget - { - [MethodImpl(MethodImplOptions.NoInlining)] - public static IDefaultInterfaceTarget GetInstance() => new DefaultInterfaceImplementedTargets(); - } - - interface IGenericDefaultInterfaceTarget - { - object Return1Del() => new object(); - object Return2Del() => new object(); - } - - class GenericDefaultInterfaceImplementedTargets : IGenericDefaultInterfaceTarget - { - [MethodImpl(MethodImplOptions.NoInlining)] - public static IGenericDefaultInterfaceTarget GetInstance() => new GenericDefaultInterfaceImplementedTargets(); - } - - interface IInterfaceToBeDefaultImplemented - { - object Return1Del(); - object Return2Del(); - } - - interface IDefaultImplementingInterface : IInterfaceToBeDefaultImplemented - { - object IInterfaceToBeDefaultImplemented.Return1Del() => new object(); - object IInterfaceToBeDefaultImplemented.Return2Del() => new object(); - } - - class DefaultInterfaceImplementedTargetsFromOther : IDefaultImplementingInterface - { - [MethodImpl(MethodImplOptions.NoInlining)] - public static IInterfaceToBeDefaultImplemented GetInstance() => new DefaultInterfaceImplementedTargetsFromOther(); - } - - abstract class AbstractBaseClass - { - public virtual object Return1Del() => new object(); - public virtual object Return2Del() => new object(); - } - - class ClassDerivedFromAbstract : AbstractBaseClass - { - [MethodImpl(MethodImplOptions.NoInlining)] - public static AbstractBaseClass GetInstance() => new ClassDerivedFromAbstract(); - } - - interface IStaticInterfaceTargets - { - static abstract object Return1Del(); - static abstract object Return2Del(); - - public static void Test() where T : IStaticInterfaceTargets - { - { - Func f1 = T.Return1Del; - Func f2 = T.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - Func f1 = T.Return1Del; - Func f2 = T.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - } - } - - class StaticInterfaceImplementedTargetsClass : IStaticInterfaceTargets - { - public static object Return1Del() => new object(); - public static object Return2Del() => new object(); - } - - struct StaticInterfaceImplementedTargetsStruct : IStaticInterfaceTargets - { - public static object Return1Del() => new object(); - public static object Return2Del() => new object(); - } - - class ReflectedOnType - { - public static object Return1Del() => new object(); - public static object Return2Del() => new object(); - } - - static object Return1() => new object(); - static object Return2() => new object(); - - class ConstructedInGenericContextToGenerics - { - static object Return1Del() => new object(); - static object Return2Del() => new object(); - - public static void Test() - { - { - Func f1 = ConstructedInGenericContextToGenerics.Return1Del; - Func f2 = ConstructedInGenericContextToGenerics.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - Func f1 = ConstructedInGenericContextToGenerics.Return1Del; - Func f2 = ConstructedInGenericContextToGenerics.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - } - } - static void Main() { - Return1(); - Return2(); - - // Static method - { - Func f1 = SimpleDelegateTargets.Return1DelStatic; - Func f2 = SimpleDelegateTargets.Return2DelStatic; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - Func f1 = SimpleDelegateTargets.Return1DelStatic; - Func f2 = SimpleDelegateTargets.Return1DelStatic; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Instance method - { - var o = new SimpleDelegateTargets(); - Func f1 = o.Return1DelInstance; - Func f2 = o.Return2DelInstance; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = new SimpleDelegateTargets(); - Func f1 = o.Return1DelInstance; - Func f2 = o.Return1DelInstance; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Virtual method - { - var o = DerivedDelegateTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = DerivedDelegateTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Method from a frozen delegate - { - if (PreinitializedDelegateTargets.s_f1.Equals(PreinitializedDelegateTargets.s_f2)) - throw new Exception(); - } - - // Interface method - { - var o = InterfaceImplementedTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = InterfaceImplementedTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Default interface method - { - var o = DefaultInterfaceImplementedTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = DefaultInterfaceImplementedTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Default generic interface method - { - var o = GenericDefaultInterfaceImplementedTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = GenericDefaultInterfaceImplementedTargets.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Default interface method - { - var o = DefaultInterfaceImplementedTargetsFromOther.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = DefaultInterfaceImplementedTargetsFromOther.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - - // Interaction with virtuals on abstract classes optimization - { - var o = ClassDerivedFromAbstract.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return2Del; - if (f1.Equals(f2)) - throw new Exception(); - } - - // Sanity check equality of delegates pointing to exact same thing - { - var o = ClassDerivedFromAbstract.GetInstance(); - Func f1 = o.Return1Del; - Func f2 = o.Return1Del; - if (!f1.Equals(f2)) - throw new Exception(); - } - - IStaticInterfaceTargets.Test(); - IStaticInterfaceTargets.Test(); - - { - var f1 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return1Del)).CreateDelegate>(); - var f2 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return2Del)).CreateDelegate>(); - if (f1.Equals(f2)) - throw new Exception(); - } - - ConstructedInGenericContextToGenerics.Test(); - ConstructedInGenericContextToGenerics.Test(); + Console.WriteLine("Hello world"); } } diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs new file mode 100644 index 0000000000000..4018c14aed1a1 --- /dev/null +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs @@ -0,0 +1,397 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; + +class BodyFoldingTest +{ + class SimpleDelegateTargets + { + public static object Return1DelStatic() => new object(); + public static object Return2DelStatic() => new object(); + + public object Return1DelInstance() => new object(); + public object Return2DelInstance() => new object(); + } + + class BaseDelegateTargets + { + public virtual object Return1Del() => new object(); + public virtual object Return2Del() => new object(); + } + + class DerivedDelegateTargets : BaseDelegateTargets + { + public override object Return1Del() => new object(); + public override object Return2Del() => new object(); + + [MethodImpl(MethodImplOptions.NoInlining)] + public static BaseDelegateTargets GetInstance() => new DerivedDelegateTargets(); + } + + class PreinitializedDelegateTargets + { + public static readonly PreinitializedDelegateTargets s_o = new PreinitializedDelegateTargets(); + public static readonly Func s_f1 = s_o.Return1Del; + public static readonly Func s_f2 = s_o.Return2Del; + + public object Return1Del() => new object(); + public object Return2Del() => new object(); + } + + interface IInterfaceTarget + { + object Return1Del(); + object Return2Del(); + } + + class InterfaceImplementedTargets : IInterfaceTarget + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IInterfaceTarget GetInstance() => new InterfaceImplementedTargets(); + public object Return1Del() => new object(); + public object Return2Del() => new object(); + } + + interface IDefaultInterfaceTarget + { + object Return1Del() => new object(); + object Return2Del() => new object(); + } + + class DefaultInterfaceImplementedTargets : IDefaultInterfaceTarget + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IDefaultInterfaceTarget GetInstance() => new DefaultInterfaceImplementedTargets(); + } + + interface IGenericDefaultInterfaceTarget + { + object Return1Del() => new object(); + object Return2Del() => new object(); + } + + class GenericDefaultInterfaceImplementedTargets : IGenericDefaultInterfaceTarget + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IGenericDefaultInterfaceTarget GetInstance() => new GenericDefaultInterfaceImplementedTargets(); + } + + interface IInterfaceToBeDefaultImplemented + { + object Return1Del(); + object Return2Del(); + } + + interface IDefaultImplementingInterface : IInterfaceToBeDefaultImplemented + { + object IInterfaceToBeDefaultImplemented.Return1Del() => new object(); + object IInterfaceToBeDefaultImplemented.Return2Del() => new object(); + } + + class DefaultInterfaceImplementedTargetsFromOther : IDefaultImplementingInterface + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IInterfaceToBeDefaultImplemented GetInstance() => new DefaultInterfaceImplementedTargetsFromOther(); + } + + abstract class AbstractBaseClass + { + public virtual object Return1Del() => new object(); + public virtual object Return2Del() => new object(); + } + + class ClassDerivedFromAbstract : AbstractBaseClass + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static AbstractBaseClass GetInstance() => new ClassDerivedFromAbstract(); + } + + interface IStaticInterfaceTargets + { + static abstract object Return1Del(); + static abstract object Return2Del(); + + public static void Test() where T : IStaticInterfaceTargets + { + { + Func f1 = T.Return1Del; + Func f2 = T.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = T.Return1Del; + Func f2 = T.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + } + } + + class StaticInterfaceImplementedTargetsClass : IStaticInterfaceTargets + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + struct StaticInterfaceImplementedTargetsStruct : IStaticInterfaceTargets + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + class ReflectedOnType + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + static object Return1() => new object(); + static object Return2() => new object(); + + class ConstructedInGenericContextToGenerics + { + static object Return1Del() => new object(); + static object Return2Del() => new object(); + + public static void Test() + { + { + Func f1 = ConstructedInGenericContextToGenerics.Return1Del; + Func f2 = ConstructedInGenericContextToGenerics.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = ConstructedInGenericContextToGenerics.Return1Del; + Func f2 = ConstructedInGenericContextToGenerics.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + } + } + + interface IInterfaceWithGvms + { + object Return1Del(); + object Return2Del(); + } + + class GvmImplementingClass : IInterfaceWithGvms + { + [MethodImpl(MethodImplOptions.NoInlining)] + public static IInterfaceWithGvms GetInstance() => new GvmImplementingClass(); + + public virtual object Return1Del() => new object(); + public virtual object Return2Del() => new object(); + } + + public static void Run() + { + Return1(); + Return2(); + + // Static method + { + Func f1 = SimpleDelegateTargets.Return1DelStatic; + Func f2 = SimpleDelegateTargets.Return2DelStatic; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = SimpleDelegateTargets.Return1DelStatic; + Func f2 = SimpleDelegateTargets.Return1DelStatic; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Instance method + { + var o = new SimpleDelegateTargets(); + Func f1 = o.Return1DelInstance; + Func f2 = o.Return2DelInstance; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = new SimpleDelegateTargets(); + Func f1 = o.Return1DelInstance; + Func f2 = o.Return1DelInstance; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Virtual method + { + var o = DerivedDelegateTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = DerivedDelegateTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Method from a frozen delegate + { + if (PreinitializedDelegateTargets.s_f1.Equals(PreinitializedDelegateTargets.s_f2)) + throw new Exception(); + } + + // Interface method + { + var o = InterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = InterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Default interface method + { + var o = DefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = DefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Default generic interface method + { + var o = GenericDefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = GenericDefaultInterfaceImplementedTargets.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Default interface method + { + var o = DefaultInterfaceImplementedTargetsFromOther.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = DefaultInterfaceImplementedTargetsFromOther.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Interaction with virtuals on abstract classes optimization + { + var o = ClassDerivedFromAbstract.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = ClassDerivedFromAbstract.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + IStaticInterfaceTargets.Test(); + IStaticInterfaceTargets.Test(); + + { + var f1 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return1Del)).CreateDelegate>(); + var f2 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return2Del)).CreateDelegate>(); + if (f1.Equals(f2)) + throw new Exception(); + } + + ConstructedInGenericContextToGenerics.Test(); + ConstructedInGenericContextToGenerics.Test(); + + // Interface generic virtual methods (shared) + { + var o = GvmImplementingClass.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = GvmImplementingClass.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + + // Interface generic virtual methods (unshared) + { + var o = GvmImplementingClass.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + var o = GvmImplementingClass.GetInstance(); + Func f1 = o.Return1Del; + Func f2 = o.Return1Del; + if (!f1.Equals(f2)) + throw new Exception(); + } + } +} diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs index 19bff10ee7f48..339aedd78b48f 100644 --- a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.cs @@ -6,9 +6,10 @@ class Program { - [MethodImpl(MethodImplOptions.NoInlining)] static int Main() { + BodyFoldingTest.Run(); + string stackTrace = Environment.StackTrace; Console.WriteLine(stackTrace); diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.csproj b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.csproj index ad170f1dda90e..7e4d202e8fff6 100644 --- a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.csproj +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata.csproj @@ -10,5 +10,6 @@ + diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata_Stripped.csproj b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata_Stripped.csproj index f825042a895be..c5aa06097b81e 100644 --- a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata_Stripped.csproj +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/StackTraceMetadata_Stripped.csproj @@ -12,5 +12,6 @@ + From f589dfa595b27fd68f9c90433d3bae217103ccf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 14 May 2024 09:27:30 +0200 Subject: [PATCH 4/7] More wip --- .../Compiler/DependencyAnalysis/EETypeNode.cs | 6 +- .../DependencyAnalysis/SealedVTableNode.cs | 8 +- .../StackTraceMetadata/BodyFoldingTest.cs | 78 +++++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) 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 31d5d6864802c..435c01bc3bde2 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs @@ -501,8 +501,12 @@ public sealed override IEnumerable GetConditionalSt MethodDesc defaultIntfMethod = implMethod.GetCanonMethodTarget(CanonicalFormKind.Specific); // If the interface method is used virtually, the implementation body is used - // BUGBUG: static virtual delegates? result.Add(new CombinedDependencyListEntry(factory.MethodEntrypoint(defaultIntfMethod), factory.VirtualMethodUse(interfaceMethod), "Interface method")); + + // If the interface method is virtual delegate target, the implementation is address taken + result.Add(new CombinedDependencyListEntry( + factory.AddressTakenMethodEntrypoint(defaultIntfMethod), + factory.DelegateTargetVirtualMethod(interfaceMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Interface slot is delegate target")); } else { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs index e77aee0989674..dad11a03c6548 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs @@ -192,8 +192,12 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) if (targetMethod.CanMethodBeInSealedVTable(factory) || implMethod.Signature.IsStatic) { - // BUGBUG - IMethodNode node = factory.MethodEntrypoint(targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !targetMethod.Signature.IsStatic && declType.IsValueType); + IMethodNode node; + if (factory.DelegateTargetVirtualMethod(declMethod.GetCanonMethodTarget(CanonicalFormKind.Specific)).Marked) + node = factory.AddressTakenMethodEntrypoint(targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !targetMethod.Signature.IsStatic && declType.IsValueType); + else + node = factory.MethodEntrypoint(targetMethod.GetCanonMethodTarget(CanonicalFormKind.Specific), unboxingStub: !targetMethod.Signature.IsStatic && declType.IsValueType); + _sealedVTableEntries.Add(SealedVTableEntry.FromVirtualMethod(targetMethod, node)); } } diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs index 4018c14aed1a1..948de1144d0f5 100644 --- a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs @@ -113,6 +113,9 @@ interface IStaticInterfaceTargets static abstract object Return1Del(); static abstract object Return2Del(); + static virtual object Return1DelDefaultImpl() => new object(); + static virtual object Return2DelDefaultImpl() => new object(); + public static void Test() where T : IStaticInterfaceTargets { { @@ -129,6 +132,21 @@ public static void Test() where T : IStaticInterfaceTargets if (!f1.Equals(f2)) throw new Exception(); } + + { + Func f1 = T.Return1DelDefaultImpl; + Func f2 = T.Return2DelDefaultImpl; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Sanity check equality of delegates pointing to exact same thing + { + Func f1 = T.Return1DelDefaultImpl; + Func f2 = T.Return1DelDefaultImpl; + if (!f1.Equals(f2)) + throw new Exception(); + } } } @@ -138,6 +156,13 @@ class StaticInterfaceImplementedTargetsClass : IStaticInterfaceTargets public static object Return2Del() => new object(); } + class StaticInterfaceImplementedTargetsClass2 : IStaticInterfaceTargets + { + public static object Return1Del() => new object(); + public static object Return2Del() => new object(); + } + + struct StaticInterfaceImplementedTargetsStruct : IStaticInterfaceTargets { public static object Return1Del() => new object(); @@ -192,6 +217,18 @@ class GvmImplementingClass : IInterfaceWithGvms public virtual object Return2Del() => new object(); } + struct UnboxingThunkStruct + { + public object Return1Del() => new object(); + public object Return2Del() => new object(); + } + + struct ReflectedUnboxingThunkStruct + { + public object Return1Del() => new object(); + public object Return2Del() => new object(); + } + public static void Run() { Return1(); @@ -347,6 +384,11 @@ public static void Run() IStaticInterfaceTargets.Test(); IStaticInterfaceTargets.Test(); + typeof(IStaticInterfaceTargets).GetMethod(nameof(IStaticInterfaceTargets.Test)) + .MakeGenericMethod(GetStaticInterfaceImplementedTargetsClass2()) + .Invoke(null, []); + + static Type GetStaticInterfaceImplementedTargetsClass2() => typeof(StaticInterfaceImplementedTargetsClass2); { var f1 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return1Del)).CreateDelegate>(); @@ -393,5 +435,41 @@ public static void Run() if (!f1.Equals(f2)) throw new Exception(); } + + // Instance method on struct (unshared) + { + var o = new UnboxingThunkStruct(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Instance method on struct (shared) + { + var o = new UnboxingThunkStruct(); + Func f1 = o.Return1Del; + Func f2 = o.Return2Del; + if (f1.Equals(f2)) + throw new Exception(); + } + + // Reflection-created open delegates to valuetypes (unshared) + { + var f1 = typeof(ReflectedUnboxingThunkStruct).GetMethod(nameof(ReflectedUnboxingThunkStruct.Return1Del)).CreateDelegate>>(); + var f2 = typeof(ReflectedUnboxingThunkStruct).GetMethod(nameof(ReflectedUnboxingThunkStruct.Return2Del)).CreateDelegate>>(); + if (f1.Equals(f2)) + throw new Exception(); + } + + // Reflection-created open delegates to valuetypes (shared) + { + var f1 = typeof(ReflectedUnboxingThunkStruct).GetMethod(nameof(ReflectedUnboxingThunkStruct.Return1Del)).CreateDelegate>>(); + var f2 = typeof(ReflectedUnboxingThunkStruct).GetMethod(nameof(ReflectedUnboxingThunkStruct.Return2Del)).CreateDelegate>>(); + if (f1.Equals(f2)) + throw new Exception(); + } } + + delegate object RefParamDelegate(ref T inst); } From 99cd33ea9951cd1ba5143155af449ed045cf66d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 14 May 2024 11:55:43 +0200 Subject: [PATCH 5/7] Try this --- .../Microsoft.NETCore.Native.targets | 2 +- .../DependencyAnalysis/ObjectNodeSection.cs | 2 -- .../tools/Common/JitInterface/CorInfoImpl.cs | 9 ++------- .../AddressTakenMethodNode.cs | 4 ++-- .../DependencyAnalysis/ILScanNodeFactory.cs | 2 +- .../DependencyAnalysis/NodeFactory.cs | 15 ++++++++------- .../Compiler/MetadataManager.cs | 2 +- .../Compiler/ObjectDataInterner.cs | 19 ++++++++++++------- .../Compiler/ObjectWriter/ObjectWriter.cs | 8 +++----- .../DependencyAnalysis/MethodCodeNode.cs | 7 ++----- .../DependencyAnalysis/RyuJitNodeFactory.cs | 4 ++-- .../Compiler/RyuJitCompilation.cs | 7 +++---- .../Compiler/RyuJitCompilationBuilder.cs | 7 +++---- 13 files changed, 40 insertions(+), 48 deletions(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 2c9a4e02aa496..f9665cb8db0a9 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -255,7 +255,7 @@ The .NET Foundation licenses this file to you under the MIT license. - + diff --git a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNodeSection.cs b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNodeSection.cs index 370a4d3b1435a..cecffda8bfe53 100644 --- a/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNodeSection.cs +++ b/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNodeSection.cs @@ -52,9 +52,7 @@ public bool IsStandardSection public static readonly ObjectNodeSection BssSection = new ObjectNodeSection("bss", SectionType.Uninitialized); public static readonly ObjectNodeSection HydrationTargetSection = new ObjectNodeSection("hydrated", SectionType.Uninitialized); public static readonly ObjectNodeSection ManagedCodeWindowsContentSection = new ObjectNodeSection(".managedcode$I", SectionType.Executable); - public static readonly ObjectNodeSection FoldableManagedCodeWindowsContentSection = new ObjectNodeSection(".managedcode$I", SectionType.Executable); public static readonly ObjectNodeSection ManagedCodeUnixContentSection = new ObjectNodeSection("__managedcode", SectionType.Executable); - public static readonly ObjectNodeSection FoldableManagedCodeUnixContentSection = new ObjectNodeSection("__managedcode", SectionType.Executable); // Section name on Windows has to be alphabetically less than the ending WindowsUnboxingStubsRegionNode node, and larger than // the begining WindowsUnboxingStubsRegionNode node, in order to have proper delimiters to the begining/ending of the diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 7719b39aa7e85..5066ab42e462d 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -479,13 +479,8 @@ private void PublishCode() } } -#pragma warning disable SA1001, SA1113, SA1115 // Comma should be on the same line as previous parameter - _methodCodeNode.SetCode(objectData -#if !SUPPORT_JIT && !READYTORUN - , isFoldable: (_compilation._compilationOptions & RyuJitCompilationOptions.MethodBodyFolding) != 0 -#endif - ); -#pragma warning restore SA1001, SA1113, SA1115 // Comma should be on the same line as previous parameter + _methodCodeNode.SetCode(objectData); + #if READYTORUN if (_methodColdCodeNode != null) { diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs index de0657491bc53..2b5f079a308f7 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/AddressTakenMethodNode.cs @@ -40,7 +40,7 @@ protected override string GetName(NodeFactory factory) public override bool ShouldSkipEmittingObjectNode(NodeFactory factory) { - return factory.ObjectInterner.GetDeduplicatedSymbol(RealBody) == RealBody; + return factory.ObjectInterner.GetDeduplicatedSymbol(factory, RealBody) == RealBody; } public override IEnumerable GetConditionalStaticDependencies(NodeFactory factory) => null; @@ -66,7 +66,7 @@ public ISymbolNode NodeForLinkage(NodeFactory factory) { // If someone refers to this node but the target method still has a unique body, // refer to the target method. - return factory.ObjectInterner.GetDeduplicatedSymbol(RealBody) == RealBody ? RealBody : this; + return factory.ObjectInterner.GetDeduplicatedSymbol(factory, RealBody) == RealBody ? RealBody : this; } public override bool RepresentsIndirectionCell diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ILScanNodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ILScanNodeFactory.cs index ab5dbd0e85089..2d13f181a6224 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ILScanNodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ILScanNodeFactory.cs @@ -13,7 +13,7 @@ namespace ILCompiler.DependencyAnalysis public sealed class ILScanNodeFactory : NodeFactory { public ILScanNodeFactory(CompilerTypeSystemContext context, CompilationModuleGroup compilationModuleGroup, MetadataManager metadataManager, InteropStubManager interopStubManager, NameMangler nameMangler, PreinitializationManager preinitManager) - : base(context, compilationModuleGroup, metadataManager, interopStubManager, nameMangler, new LazyGenericsDisabledPolicy(), new LazyVTableSliceProvider(), new LazyDictionaryLayoutProvider(), new InlinedThreadStatics(), new ExternSymbolsImportedNodeProvider(), preinitManager, new DevirtualizationManager()) + : base(context, compilationModuleGroup, metadataManager, interopStubManager, nameMangler, new LazyGenericsDisabledPolicy(), new LazyVTableSliceProvider(), new LazyDictionaryLayoutProvider(), new InlinedThreadStatics(), new ExternSymbolsImportedNodeProvider(), preinitManager, new DevirtualizationManager(), ObjectDataInterner.Null) { } 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 6fa51d6c68e59..586ec5eb0be77 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -25,7 +25,6 @@ public abstract partial class NodeFactory private VTableSliceProvider _vtableSliceProvider; private DictionaryLayoutProvider _dictionaryLayoutProvider; private InlinedThreadStatics _inlinedThreadStatics; - private ObjectDataInterner _interner; protected readonly ImportedNodeProvider _importedNodeProvider; private bool _markingComplete; @@ -41,7 +40,8 @@ public NodeFactory( InlinedThreadStatics inlinedThreadStatics, ImportedNodeProvider importedNodeProvider, PreinitializationManager preinitializationManager, - DevirtualizationManager devirtualizationManager) + DevirtualizationManager devirtualizationManager, + ObjectDataInterner dataInterner) { _target = context.Target; _context = context; @@ -57,6 +57,7 @@ public NodeFactory( _importedNodeProvider = importedNodeProvider; PreinitializationManager = preinitializationManager; DevirtualizationManager = devirtualizationManager; + ObjectInterner = dataInterner; } public void SetMarkingComplete() @@ -118,10 +119,7 @@ public InteropStubManager InteropStubManager internal ObjectDataInterner ObjectInterner { - get - { - return _interner ??= new ObjectDataInterner(this); - } + get; } /// @@ -999,6 +997,9 @@ public IMethodNode FatFunctionPointer(MethodDesc method, bool isUnboxingStub = f public IMethodNode FatAddressTakenFunctionPointer(MethodDesc method, bool isUnboxingStub = false) { + if (ObjectInterner.IsNull) + return FatFunctionPointer(method, isUnboxingStub); + return _fatAddressTakenFunctionPointers.GetOrAdd(new MethodKey(method, isUnboxingStub)); } @@ -1056,7 +1057,7 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type) private NodeCache _addressTakenMethods; public IMethodNode AddressTakenMethodEntrypoint(MethodDesc method, bool unboxingStub = false) { - if (unboxingStub) + if (unboxingStub || ObjectInterner.IsNull) return MethodEntrypoint(method, unboxingStub); return _addressTakenMethods.GetOrAdd(method); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs index a159341d12c00..96cfc811ca6be 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MetadataManager.cs @@ -709,7 +709,7 @@ protected void ComputeMetadata( continue; // If the method will be folded, no need to emit stack trace info for this one - ISymbolNode internedBody = factory.ObjectInterner.GetDeduplicatedSymbol(methodBody); + ISymbolNode internedBody = factory.ObjectInterner.GetDeduplicatedSymbol(factory, methodBody); if (internedBody != methodBody) continue; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs index 84f1d59845ffb..743fdaeda6ed0 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs @@ -10,17 +10,20 @@ namespace ILCompiler { - internal sealed class ObjectDataInterner + public sealed class ObjectDataInterner { - private readonly NodeFactory _factory; + private Dictionary _symbolRemapping; - private readonly Dictionary _symbolRemapping; + public static ObjectDataInterner Null { get; } = new ObjectDataInterner() { _symbolRemapping = new() }; - public ObjectDataInterner(NodeFactory factory) + public bool IsNull => _symbolRemapping != null && _symbolRemapping.Count == 0; + + private void EnsureMap(NodeFactory factory) { Debug.Assert(factory.MarkingComplete); - _factory = factory; + if (_symbolRemapping != null) + return; var symbolRemapping = new Dictionary(); var methodHash = new HashSet(new MethodInternComparer(factory)); @@ -45,11 +48,13 @@ public ObjectDataInterner(NodeFactory factory) _symbolRemapping = symbolRemapping; } - public ISymbolNode GetDeduplicatedSymbol(ISymbolNode original) + public ISymbolNode GetDeduplicatedSymbol(NodeFactory factory, ISymbolNode original) { + EnsureMap(factory); + ISymbolNode target = original; if (target is ISymbolNodeWithLinkage symbolWithLinkage) - target = symbolWithLinkage.NodeForLinkage(_factory); + target = symbolWithLinkage.NodeForLinkage(factory); return _symbolRemapping.TryGetValue(target, out ISymbolNode result) ? result : original; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs index d824f63685caa..ebccf0bba30a3 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectWriter/ObjectWriter.cs @@ -118,9 +118,7 @@ private protected bool ShouldShareSymbol(ObjectNode node, ObjectNodeSection sect return false; // Foldable sections are always COMDATs - if (section == ObjectNodeSection.FoldableManagedCodeUnixContentSection || - section == ObjectNodeSection.FoldableManagedCodeWindowsContentSection || - section == ObjectNodeSection.FoldableReadOnlyDataSection) + if (section == ObjectNodeSection.FoldableReadOnlyDataSection) return true; if (_isSingleFileCompilation) @@ -396,7 +394,7 @@ private void EmitObject(string objectFilePath, IReadOnlyCollection _method; @@ -52,8 +50,7 @@ public void SetCode(ObjectData data, bool isFoldable) public override ObjectNodeSection GetSection(NodeFactory factory) { return factory.Target.IsWindows ? - (_isFoldable ? ObjectNodeSection.FoldableManagedCodeWindowsContentSection : ObjectNodeSection.ManagedCodeWindowsContentSection) : - (_isFoldable ? ObjectNodeSection.FoldableManagedCodeUnixContentSection : ObjectNodeSection.ManagedCodeUnixContentSection); + ObjectNodeSection.ManagedCodeWindowsContentSection : ObjectNodeSection.ManagedCodeUnixContentSection; } public override bool StaticDependenciesAreComputed => _methodCode != null; diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs index f304b920fddc7..fa328d268a9a5 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs @@ -11,8 +11,8 @@ public sealed class RyuJitNodeFactory : NodeFactory { public RyuJitNodeFactory(CompilerTypeSystemContext context, CompilationModuleGroup compilationModuleGroup, MetadataManager metadataManager, InteropStubManager interopStubManager, NameMangler nameMangler, VTableSliceProvider vtableSliceProvider, DictionaryLayoutProvider dictionaryLayoutProvider, InlinedThreadStatics inlinedThreadStatics, PreinitializationManager preinitializationManager, - DevirtualizationManager devirtualizationManager) - : base(context, compilationModuleGroup, metadataManager, interopStubManager, nameMangler, new LazyGenericsDisabledPolicy(), vtableSliceProvider, dictionaryLayoutProvider, inlinedThreadStatics, new ExternSymbolsImportedNodeProvider(), preinitializationManager, devirtualizationManager) + DevirtualizationManager devirtualizationManager, ObjectDataInterner dataInterner) + : base(context, compilationModuleGroup, metadataManager, interopStubManager, nameMangler, new LazyGenericsDisabledPolicy(), vtableSliceProvider, dictionaryLayoutProvider, inlinedThreadStatics, new ExternSymbolsImportedNodeProvider(), preinitializationManager, devirtualizationManager, dataInterner) { } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs index 80aef8f4c2118..d25a14f576531 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs @@ -223,9 +223,8 @@ private void CompileSingleMethod(CorInfoImpl corInfo, MethodCodeNode methodCodeN [Flags] public enum RyuJitCompilationOptions { - MethodBodyFolding = 0x1, - ControlFlowGuardAnnotations = 0x2, - UseDwarf5 = 0x4, - UseResilience = 0x8, + ControlFlowGuardAnnotations = 0x1, + UseDwarf5 = 0x2, + UseResilience = 0x4, } } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs index 37c2eba50195e..6456205cb795c 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs @@ -108,9 +108,6 @@ public override ICompilation ToCompilation() jitFlagBuilder.Add(CorJitFlag.CORJIT_FLAG_DEBUG_INFO); RyuJitCompilationOptions options = 0; - if (_methodBodyFolding) - options |= RyuJitCompilationOptions.MethodBodyFolding; - if ((_mitigationOptions & SecurityMitigationOptions.ControlFlowGuardAnnotations) != 0) { jitFlagBuilder.Add(CorJitFlag.CORJIT_FLAG_ENABLE_CFG); @@ -123,7 +120,9 @@ public override ICompilation ToCompilation() if (_resilient) options |= RyuJitCompilationOptions.UseResilience; - var factory = new RyuJitNodeFactory(_context, _compilationGroup, _metadataManager, _interopStubManager, _nameMangler, _vtableSliceProvider, _dictionaryLayoutProvider, _inlinedThreadStatics, GetPreinitializationManager(), _devirtualizationManager); + ObjectDataInterner interner = _methodBodyFolding ? new ObjectDataInterner() : ObjectDataInterner.Null; + + var factory = new RyuJitNodeFactory(_context, _compilationGroup, _metadataManager, _interopStubManager, _nameMangler, _vtableSliceProvider, _dictionaryLayoutProvider, _inlinedThreadStatics, GetPreinitializationManager(), _devirtualizationManager, interner); JitConfigProvider.Initialize(_context.Target, jitFlagBuilder.ToArray(), _ryujitOptions, _jitPath); DependencyAnalyzerBase graph = CreateDependencyGraph(factory, new ObjectNode.ObjectNodeComparer(CompilerComparer.Instance)); From c11a42e8728ce8d3db2017540cac30e54ecded47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 14 May 2024 12:52:42 +0200 Subject: [PATCH 6/7] Hmm --- .../Compiler/DependencyAnalysis/SealedVTableNode.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs index dad11a03c6548..dfa6c0967282e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/SealedVTableNode.cs @@ -169,12 +169,13 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) if (!declMethod.Signature.IsStatic && !needsEntriesForInstanceInterfaceMethodImpls) continue; + MethodDesc interfaceDefDeclMethod = declMethod; if (!interfaceType.IsTypeDefinition) - declMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), (InstantiatedType)definitionInterfaceType); + interfaceDefDeclMethod = factory.TypeSystemContext.GetMethodForInstantiatedType(declMethod.GetTypicalMethodDefinition(), (InstantiatedType)definitionInterfaceType); var implMethod = declMethod.Signature.IsStatic ? - declTypeDefinition.ResolveInterfaceMethodToStaticVirtualMethodOnType(declMethod) : - declTypeDefinition.ResolveInterfaceMethodToVirtualMethodOnType(declMethod); + declTypeDefinition.ResolveInterfaceMethodToStaticVirtualMethodOnType(interfaceDefDeclMethod) : + declTypeDefinition.ResolveInterfaceMethodToVirtualMethodOnType(interfaceDefDeclMethod); // Interface methods first implemented by a base type in the hierarchy will return null for the implMethod (runtime interface // dispatch will walk the inheritance chain). @@ -206,7 +207,7 @@ public bool BuildSealedVTableSlots(NodeFactory factory, bool relocsOnly) { // If the interface method is provided by a default implementation, add the default implementation // to the sealed vtable. - var resolution = declTypeDefinition.ResolveInterfaceMethodToDefaultImplementationOnType(declMethod, out implMethod); + var resolution = declTypeDefinition.ResolveInterfaceMethodToDefaultImplementationOnType(interfaceDefDeclMethod, out implMethod); if (resolution == DefaultInterfaceMethodResolution.DefaultImplementation) { DefType providingInterfaceDefinitionType = (DefType)implMethod.OwningType; From 4129d8efd1d3af93b0879cdb7c87065b6d10efc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Thu, 13 Jun 2024 08:42:32 +0200 Subject: [PATCH 7/7] FB --- .../SmokeTests/StackTraceMetadata/BodyFoldingTest.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs index 948de1144d0f5..607e2761a5363 100644 --- a/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs +++ b/src/tests/nativeaot/SmokeTests/StackTraceMetadata/BodyFoldingTest.cs @@ -397,6 +397,14 @@ public static void Run() throw new Exception(); } + // Sanity check equality of delegates pointing to exact same thing + { + var f1 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return1Del)).CreateDelegate>(); + var f2 = typeof(ReflectedOnType).GetMethod(nameof(ReflectedOnType.Return1Del)).CreateDelegate>(); + if (!f1.Equals(f2)) + throw new Exception(); + } + ConstructedInGenericContextToGenerics.Test(); ConstructedInGenericContextToGenerics.Test();