Fold generic method bodies by default (#117411)

Fixes #103951.

Same method with different genericness are not distinguishable in the stack traces. They are still distinguishable in the debugger, but it might be acceptable, especially since we have an opt out (undocumented for now).

Author: MichalStrehovsky

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

@@ -237,6 +237,12 @@ The .NET Foundation licenses this file to you under the MIT license.
       <_IlcNoSingleWarnAssemblies Include="@(_IlcNoSingleWarnAssembliesRaw)" Condition="!$([System.IO.File]::Exists('%(Identity)'))" />
     </ItemGroup>
 
+    <PropertyGroup>
+      <_IlcMethodBodyFoldingValue Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'">all</_IlcMethodBodyFoldingValue>
+      <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false' and $(IlcMultiModule) != 'true'">generic</_IlcMethodBodyFoldingValue>
+      <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' or $(Optimize) != 'true'">none</_IlcMethodBodyFoldingValue>
+    </PropertyGroup>
+
     <ItemGroup>
       <IlcArg Include="@(IlcCompileInput)" />
       <IlcArg Include="-o:$(NativeIntermediateOutputPath)%(ManagedBinary.Filename)$(IlcOutputFileExt)" />
@@ -275,7 +281,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'" Include="--scanreflection" />
-      <IlcArg Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'" Include="--methodbodyfolding" />
+      <IlcArg Include="--methodbodyfolding:$(_IlcMethodBodyFoldingValue)" />
       <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/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.cs

@@ -21,7 +21,7 @@ public partial class CompilationBuilder
         protected MethodImportationErrorProvider _methodImportationErrorProvider = new MethodImportationErrorProvider();
         protected ReadOnlyFieldPolicy _readOnlyFieldPolicy = new ReadOnlyFieldPolicy();
         protected IInliningPolicy _inliningPolicy;
-        protected bool _methodBodyFolding;
+        protected MethodBodyFoldingMode _methodBodyFolding;
         protected InstructionSetSupport _instructionSetSupport;
         protected SecurityMitigationOptions _mitigationOptions;
         protected bool _dehydrate;
@@ -94,9 +94,9 @@ public CompilationBuilder UseDehydration(bool dehydrate)
             return this;
         }
 
-        public CompilationBuilder UseMethodBodyFolding(bool enable)
+        public CompilationBuilder UseMethodBodyFolding(MethodBodyFoldingMode mode)
         {
-            _methodBodyFolding = enable;
+            _methodBodyFolding = mode;
             return this;
         }
 
@@ -154,4 +154,11 @@ public enum SecurityMitigationOptions
     {
         ControlFlowGuardAnnotations = 0x0001,
     }
+
+    public enum MethodBodyFoldingMode
+    {
+        None,
+        Generic,
+        All,
+    }
 }

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

@@ -1062,7 +1062,7 @@ public IMethodNode FatFunctionPointer(MethodDesc method, bool isUnboxingStub = f
 
         public IMethodNode FatAddressTakenFunctionPointer(MethodDesc method, bool isUnboxingStub = false)
         {
-            if (ObjectInterner.IsNull)
+            if (!ObjectInterner.CanFold(method))
                 return FatFunctionPointer(method, isUnboxingStub);
 
             return _fatAddressTakenFunctionPointers.GetOrAdd(new MethodKey(method, isUnboxingStub));
@@ -1128,7 +1128,7 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type)
         private NodeCache<MethodDesc, AddressTakenMethodNode> _addressTakenMethods;
         public IMethodNode AddressTakenMethodEntrypoint(MethodDesc method, bool unboxingStub = false)
         {
-            if (unboxingStub || ObjectInterner.IsNull)
+            if (unboxingStub || !ObjectInterner.CanFold(method))
                 return MethodEntrypoint(method, unboxingStub);
 
             return _addressTakenMethods.GetOrAdd(method);

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs

@@ -6,17 +6,29 @@
 
 using ILCompiler.DependencyAnalysis;
 
+using Internal.TypeSystem;
+
 using Debug = System.Diagnostics.Debug;
 
 namespace ILCompiler
 {
     public sealed class ObjectDataInterner
     {
+        private readonly bool _genericsOnly;
         private Dictionary<ISymbolNode, ISymbolNode> _symbolRemapping;
 
-        public static ObjectDataInterner Null { get; } = new ObjectDataInterner() { _symbolRemapping = new() };
+        public static ObjectDataInterner Null { get; } = new ObjectDataInterner(genericsOnly: false) { _symbolRemapping = new() };
 
-        public bool IsNull => _symbolRemapping != null && _symbolRemapping.Count == 0;
+        public ObjectDataInterner(bool genericsOnly)
+        {
+            _genericsOnly = genericsOnly;
+        }
+
+        public bool CanFold(MethodDesc method)
+        {
+            return this != Null
+                && (!_genericsOnly || method.HasInstantiation || method.OwningType.HasInstantiation);
+        }
 
         private void EnsureMap(NodeFactory factory)
         {
@@ -34,11 +46,14 @@ private void EnsureMap(NodeFactory factory)
             {
                 previousMethodHash = methodHash;
                 previousSymbolRemapping = symbolRemapping;
-                methodHash = new HashSet<MethodInternKey>(previousMethodHash?.Count ?? 0, new MethodInternComparer(factory, previousSymbolRemapping));
+                methodHash = new HashSet<MethodInternKey>(previousMethodHash?.Count ?? 0, new MethodInternComparer(factory, previousSymbolRemapping, _genericsOnly));
                 symbolRemapping = new Dictionary<ISymbolNode, ISymbolNode>((int)(1.05 * (previousSymbolRemapping?.Count ?? 0)));
 
                 foreach (IMethodBodyNode body in factory.MetadataManager.GetCompiledMethodBodies())
                 {
+                    if (!CanFold(body.Method))
+                        continue;
+
                     // We don't track special unboxing thunks as virtual method use related so ignore them
                     if (body is ISpecialUnboxThunkNode unboxThunk && unboxThunk.IsSpecialUnboxingThunk)
                         continue;
@@ -107,9 +122,10 @@ private sealed class MethodInternComparer : IEqualityComparer<MethodInternKey>
         {
             private readonly NodeFactory _factory;
             private readonly Dictionary<ISymbolNode, ISymbolNode> _interner;
+            private readonly bool _genericsOnly;
 
-            public MethodInternComparer(NodeFactory factory, Dictionary<ISymbolNode, ISymbolNode> interner)
-                => (_factory, _interner) = (factory, interner);
+            public MethodInternComparer(NodeFactory factory, Dictionary<ISymbolNode, ISymbolNode> interner, bool genericsOnly)
+                => (_factory, _interner, _genericsOnly) = (factory, interner, genericsOnly);
 
             public int GetHashCode(MethodInternKey key) => key.HashCode;
 
@@ -161,6 +177,10 @@ public bool Equals(MethodInternKey a, MethodInternKey b)
                 if (a.HashCode != b.HashCode)
                     return false;
 
+                if (_genericsOnly
+                    && a.Method.Method.GetTypicalMethodDefinition() != b.Method.Method.GetTypicalMethodDefinition())
+                    return false;
+
                 ObjectNode.ObjectData o1data = ((ObjectNode)a.Method).GetData(_factory, relocsOnly: false);
                 ObjectNode.ObjectData o2data = ((ObjectNode)b.Method).GetData(_factory, relocsOnly: false);
 

src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs

@@ -136,7 +136,12 @@ public override ICompilation ToCompilation()
             if (_resilient)
                 options |= RyuJitCompilationOptions.UseResilience;
 
-            ObjectDataInterner interner = _methodBodyFolding ? new ObjectDataInterner() : ObjectDataInterner.Null;
+            ObjectDataInterner interner = _methodBodyFolding switch
+            {
+                MethodBodyFoldingMode.Generic => new ObjectDataInterner(genericsOnly: true),
+                MethodBodyFoldingMode.All => new ObjectDataInterner(genericsOnly: false),
+                _ => ObjectDataInterner.Null,
+            };
 
             var factory = new RyuJitNodeFactory(_context, _compilationGroup, _metadataManager, _interopStubManager, _nameMangler, _vtableSliceProvider, _dictionaryLayoutProvider, _inlinedThreadStatics, GetPreinitializationManager(), _devirtualizationManager, interner, _typeMapManager);
 

src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs

@@ -101,8 +101,8 @@ internal sealed class ILCompilerRootCommand : RootCommand
             new("--noinlinetls") { Description = "Do not generate inline thread local statics" };
         public Option<bool> EmitStackTraceData { get; } =
             new("--stacktracedata") { Description = "Emit data to support generating stack trace strings at runtime" };
-        public Option<bool> MethodBodyFolding { get; } =
-            new("--methodbodyfolding") { Description = "Fold identical method bodies" };
+        public Option<string> MethodBodyFolding { get; } =
+            new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" };
         public Option<string[]> InitAssemblies { get; } =
             new("--initassembly") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "Assembly(ies) with a library initializer" };
         public Option<string[]> FeatureSwitches { get; } =

src/coreclr/tools/aot/ILCompiler/Program.cs

@@ -604,10 +604,14 @@ void RunScanner()
             compilationRoots.Add(metadataManager);
             compilationRoots.Add(interopStubManager);
 
+            MethodBodyFoldingMode foldingMode = string.IsNullOrEmpty(Get(_command.MethodBodyFolding))
+                ? MethodBodyFoldingMode.None
+                : Enum.Parse<MethodBodyFoldingMode>(Get(_command.MethodBodyFolding), ignoreCase: true);
+
             builder
                 .UseInstructionSetSupport(instructionSetSupport)
                 .UseBackendOptions(Get(_command.CodegenOptions))
-                .UseMethodBodyFolding(enable: Get(_command.MethodBodyFolding))
+                .UseMethodBodyFolding(foldingMode)
                 .UseParallelism(parallelism)
                 .UseMetadataManager(metadataManager)
                 .UseInteropStubManager(interopStubManager)