Fold identical method bodies in the compiler

src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets

@@ -255,7 +255,7 @@ The .NET Foundation licenses this file to you under the MIT license.
       <IlcArg Condition="$(IlcGenerateCompleteTypeMetadata) == 'true'" Include="--completetypemetadata" />
       <IlcArg Condition="$(StackTraceSupport) != 'false'" Include="--stacktracedata" />
       <IlcArg Condition="$(IlcScanReflection) != 'false' and $(IlcDisableReflection) != 'true'" Include="--scanreflection" />
-      <IlcArg Condition="$(IlcFoldIdenticalMethodBodies) == 'true'" Include="--methodbodyfolding" />
+      <IlcArg Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'" Include="--methodbodyfolding" />
       <IlcArg Condition="$(Optimize) == 'true' and $(OptimizationPreference) == 'Size'" Include="--Os" />
       <IlcArg Condition="$(Optimize) == 'true' and $(OptimizationPreference) == 'Speed'" Include="--Ot" />
       <IlcArg Condition="'$(_linuxLibcFlavor)' == 'bionic'" Include="--noinlinetls" />

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

src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

@@ -480,13 +480,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)
             {

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;
 

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
+{
+    /// <summary>
+    /// 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 <see cref="ShouldSkipEmittingObjectNode(NodeFactory)"/> and <see cref="NodeForLinkage(NodeFactory)"/>.
+    /// 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.
+    /// </summary>
+    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(factory, RealBody) == RealBody;
+        }
+
+        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
+        public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> 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(factory, 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;
+    }
+}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs

@@ -16,16 +16,18 @@ namespace ILCompiler.DependencyAnalysis
     public class DelegateTargetVirtualMethodNode : DependencyNodeCore<NodeFactory>
     {
         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<DependencyListEntry> GetStaticDependencies(NodeFactory factory) => null;

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs

@@ -426,6 +426,10 @@ public sealed override IEnumerable<CombinedDependencyListEntry> 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)
@@ -498,11 +502,22 @@ public sealed override IEnumerable<CombinedDependencyListEntry> GetConditionalSt
 
                                 // If the interface method is used virtually, the implementation body is used
                                 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
                             {
                                 // 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 +565,11 @@ public sealed override IEnumerable<CombinedDependencyListEntry> 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 +1123,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

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

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/FatFunctionPointerNode.cs

@@ -15,22 +15,24 @@ namespace ILCompiler.DependencyAnalysis
     /// </summary>
     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);
         }
     }