From 09e4f37b4735402526b0086d514ff010a2343f09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 8 Jul 2025 09:51:55 +0200 Subject: [PATCH 1/5] Fold generic method bodies by default 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). --- .../Microsoft.NETCore.Native.targets | 8 ++++- .../Compiler/CompilationBuilder.Aot.cs | 13 ++++++-- .../DependencyAnalysis/NodeFactory.cs | 4 +-- .../Compiler/ObjectDataInterner.cs | 30 +++++++++++++++---- .../Compiler/RyuJitCompilationBuilder.cs | 7 ++++- .../aot/ILCompiler/ILCompilerRootCommand.cs | 4 +-- src/coreclr/tools/aot/ILCompiler/Program.cs | 6 +++- 7 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 348c20b9fcbf33..77bb18a72d226b 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/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)'))" /> + + <_IlcMethodBodyFoldingValue Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'">all + <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false'">generic + <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == ''">none + + @@ -275,7 +281,7 @@ The .NET Foundation licenses this file to you under the MIT license. - + diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.cs index 5eb049c74e442b..03818be44eb2a3 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilationBuilder.Aot.cs +++ b/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, + } } 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 a6add520521126..34335295ff9fdf 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -1059,7 +1059,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)); @@ -1125,7 +1125,7 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type) private NodeCache _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); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs index bbaf09e2c8cdc6..f54d68cf0fc154 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs +++ b/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 _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(previousMethodHash?.Count ?? 0, new MethodInternComparer(factory, previousSymbolRemapping)); + methodHash = new HashSet(previousMethodHash?.Count ?? 0, new MethodInternComparer(factory, previousSymbolRemapping, _genericsOnly)); symbolRemapping = new Dictionary((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 { private readonly NodeFactory _factory; private readonly Dictionary _interner; + private readonly bool _genericsOnly; - public MethodInternComparer(NodeFactory factory, Dictionary interner) - => (_factory, _interner) = (factory, interner); + public MethodInternComparer(NodeFactory factory, Dictionary 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); diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs index b1291ac837dce8..140d120ebd523f 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs @@ -129,7 +129,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); diff --git a/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs b/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs index 55b670ee7213da..efd422e2a0d788 100644 --- a/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs +++ b/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs @@ -99,8 +99,8 @@ internal sealed class ILCompilerRootCommand : RootCommand new("--noinlinetls") { Description = "Do not generate inline thread local statics" }; public Option EmitStackTraceData { get; } = new("--stacktracedata") { Description = "Emit data to support generating stack trace strings at runtime" }; - public Option MethodBodyFolding { get; } = - new("--methodbodyfolding") { Description = "Fold identical method bodies" }; + public Option MethodBodyFolding { get; } = + new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" }; public Option InitAssemblies { get; } = new("--initassembly") { DefaultValueFactory = _ => Array.Empty(), Description = "Assembly(ies) with a library initializer" }; public Option FeatureSwitches { get; } = diff --git a/src/coreclr/tools/aot/ILCompiler/Program.cs b/src/coreclr/tools/aot/ILCompiler/Program.cs index 0cf9f83308e207..3d537bb8bb2a5e 100644 --- a/src/coreclr/tools/aot/ILCompiler/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/Program.cs @@ -603,10 +603,14 @@ void RunScanner() compilationRoots.Add(metadataManager); compilationRoots.Add(interopStubManager); + MethodBodyFoldingMode foldingMode = string.IsNullOrEmpty(Get(_command.MethodBodyFolding)) + ? MethodBodyFoldingMode.None + : Enum.Parse(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) From 55811fd90e2e2b5b7b16ba0e60ac30b2f1630bd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 8 Jul 2025 09:59:35 +0200 Subject: [PATCH 2/5] temp change to run rt-sz --- .../BuildIntegration/Microsoft.NETCore.Native.targets | 8 +------- src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs | 4 ++-- src/coreclr/tools/aot/ILCompiler/Program.cs | 6 +----- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 77bb18a72d226b..348c20b9fcbf33 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -237,12 +237,6 @@ The .NET Foundation licenses this file to you under the MIT license. <_IlcNoSingleWarnAssemblies Include="@(_IlcNoSingleWarnAssembliesRaw)" Condition="!$([System.IO.File]::Exists('%(Identity)'))" /> - - <_IlcMethodBodyFoldingValue Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'">all - <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false'">generic - <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == ''">none - - @@ -281,7 +275,7 @@ The .NET Foundation licenses this file to you under the MIT license. - + diff --git a/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs b/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs index efd422e2a0d788..55b670ee7213da 100644 --- a/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs +++ b/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs @@ -99,8 +99,8 @@ internal sealed class ILCompilerRootCommand : RootCommand new("--noinlinetls") { Description = "Do not generate inline thread local statics" }; public Option EmitStackTraceData { get; } = new("--stacktracedata") { Description = "Emit data to support generating stack trace strings at runtime" }; - public Option MethodBodyFolding { get; } = - new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" }; + public Option MethodBodyFolding { get; } = + new("--methodbodyfolding") { Description = "Fold identical method bodies" }; public Option InitAssemblies { get; } = new("--initassembly") { DefaultValueFactory = _ => Array.Empty(), Description = "Assembly(ies) with a library initializer" }; public Option FeatureSwitches { get; } = diff --git a/src/coreclr/tools/aot/ILCompiler/Program.cs b/src/coreclr/tools/aot/ILCompiler/Program.cs index 3d537bb8bb2a5e..6131625d2ab254 100644 --- a/src/coreclr/tools/aot/ILCompiler/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/Program.cs @@ -603,14 +603,10 @@ void RunScanner() compilationRoots.Add(metadataManager); compilationRoots.Add(interopStubManager); - MethodBodyFoldingMode foldingMode = string.IsNullOrEmpty(Get(_command.MethodBodyFolding)) - ? MethodBodyFoldingMode.None - : Enum.Parse(Get(_command.MethodBodyFolding), ignoreCase: true); - builder .UseInstructionSetSupport(instructionSetSupport) .UseBackendOptions(Get(_command.CodegenOptions)) - .UseMethodBodyFolding(foldingMode) + .UseMethodBodyFolding(Get(_command.MethodBodyFolding) ? MethodBodyFoldingMode.All : MethodBodyFoldingMode.Generic) .UseParallelism(parallelism) .UseMetadataManager(metadataManager) .UseInteropStubManager(interopStubManager) From 9a31ef4371cd61df4322e5f6423b30e4038eaa24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 8 Jul 2025 10:32:52 +0200 Subject: [PATCH 3/5] Revert "temp change to run rt-sz" This reverts commit 55811fd90e2e2b5b7b16ba0e60ac30b2f1630bd0. --- .../BuildIntegration/Microsoft.NETCore.Native.targets | 8 +++++++- src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs | 4 ++-- src/coreclr/tools/aot/ILCompiler/Program.cs | 6 +++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 348c20b9fcbf33..77bb18a72d226b 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/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)'))" /> + + <_IlcMethodBodyFoldingValue Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'">all + <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false'">generic + <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == ''">none + + @@ -275,7 +281,7 @@ The .NET Foundation licenses this file to you under the MIT license. - + diff --git a/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs b/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs index 55b670ee7213da..efd422e2a0d788 100644 --- a/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs +++ b/src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs @@ -99,8 +99,8 @@ internal sealed class ILCompilerRootCommand : RootCommand new("--noinlinetls") { Description = "Do not generate inline thread local statics" }; public Option EmitStackTraceData { get; } = new("--stacktracedata") { Description = "Emit data to support generating stack trace strings at runtime" }; - public Option MethodBodyFolding { get; } = - new("--methodbodyfolding") { Description = "Fold identical method bodies" }; + public Option MethodBodyFolding { get; } = + new("--methodbodyfolding") { Description = "Fold identical method bodies (one of: none, generic, all" }; public Option InitAssemblies { get; } = new("--initassembly") { DefaultValueFactory = _ => Array.Empty(), Description = "Assembly(ies) with a library initializer" }; public Option FeatureSwitches { get; } = diff --git a/src/coreclr/tools/aot/ILCompiler/Program.cs b/src/coreclr/tools/aot/ILCompiler/Program.cs index 6131625d2ab254..3d537bb8bb2a5e 100644 --- a/src/coreclr/tools/aot/ILCompiler/Program.cs +++ b/src/coreclr/tools/aot/ILCompiler/Program.cs @@ -603,10 +603,14 @@ void RunScanner() compilationRoots.Add(metadataManager); compilationRoots.Add(interopStubManager); + MethodBodyFoldingMode foldingMode = string.IsNullOrEmpty(Get(_command.MethodBodyFolding)) + ? MethodBodyFoldingMode.None + : Enum.Parse(Get(_command.MethodBodyFolding), ignoreCase: true); + builder .UseInstructionSetSupport(instructionSetSupport) .UseBackendOptions(Get(_command.CodegenOptions)) - .UseMethodBodyFolding(Get(_command.MethodBodyFolding) ? MethodBodyFoldingMode.All : MethodBodyFoldingMode.Generic) + .UseMethodBodyFolding(foldingMode) .UseParallelism(parallelism) .UseMetadataManager(metadataManager) .UseInteropStubManager(interopStubManager) From 784b183376741641af204b475dbe86886ca8fbdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 8 Jul 2025 12:21:33 +0200 Subject: [PATCH 4/5] Update Microsoft.NETCore.Native.targets --- .../nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 77bb18a72d226b..64c6e35d41d97e 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -239,7 +239,7 @@ The .NET Foundation licenses this file to you under the MIT license. <_IlcMethodBodyFoldingValue Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'">all - <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false'">generic + <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false' and $(IlcMultiModule) != 'true'">generic <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == ''">none From 4747a321154afd55fd69c0be6d169357784f1ae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Tue, 8 Jul 2025 12:33:44 +0200 Subject: [PATCH 5/5] Update Microsoft.NETCore.Native.targets --- .../nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets index 64c6e35d41d97e..e57a63950b4009 100644 --- a/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets +++ b/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets @@ -240,7 +240,7 @@ The .NET Foundation licenses this file to you under the MIT license. <_IlcMethodBodyFoldingValue Condition="$(IlcFoldIdenticalMethodBodies) == 'true' or $(StackTraceSupport) == 'false'">all <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' and $(IlcFoldIdenticalMethodBodies) != 'false' and $(IlcMultiModule) != 'true'">generic - <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == ''">none + <_IlcMethodBodyFoldingValue Condition="$(_IlcMethodBodyFoldingValue) == '' or $(Optimize) != 'true'">none