From 2361c00717a54a5dd9b0cf727102d64f783855b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Strehovsk=C3=BD?= Date: Sat, 3 Feb 2024 11:15:15 +0900 Subject: [PATCH] Move `DevirtualizationManager` to `NodeFactory` (#97855) This is prep work - I want to be able to use this from dependency analysis, not just codegen (`Compilation` is not available to dependency analysis, `NodeFactory` is). I think there are optimization opportunities around this. --- .../ILCompiler.Compiler/Compiler/Compilation.cs | 15 ++++++--------- .../DependencyAnalysis/ILScanNodeFactory.cs | 2 +- .../Compiler/DependencyAnalysis/NodeFactory.cs | 9 ++++++++- .../aot/ILCompiler.Compiler/Compiler/ILScanner.cs | 2 +- .../DependencyAnalysis/RyuJitNodeFactory.cs | 5 +++-- .../Compiler/RyuJitCompilation.cs | 9 +++------ .../Compiler/RyuJitCompilationBuilder.cs | 4 ++-- 7 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index 1e350711742454..98d2e96faadfa1 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -25,7 +25,6 @@ public abstract class Compilation : ICompilation protected readonly NodeFactory _nodeFactory; protected readonly Logger _logger; protected readonly DebugInformationProvider _debugInformationProvider; - protected readonly DevirtualizationManager _devirtualizationManager; private readonly IInliningPolicy _inliningPolicy; public NameMangler NameMangler => _nodeFactory.NameMangler; @@ -44,7 +43,6 @@ protected Compilation( IEnumerable compilationRoots, ILProvider ilProvider, DebugInformationProvider debugInformationProvider, - DevirtualizationManager devirtualizationManager, IInliningPolicy inliningPolicy, Logger logger) { @@ -52,7 +50,6 @@ protected Compilation( _nodeFactory = nodeFactory; _logger = logger; _debugInformationProvider = debugInformationProvider; - _devirtualizationManager = devirtualizationManager; _inliningPolicy = inliningPolicy; _dependencyGraph.ComputeDependencyRoutine += ComputeDependencyNodeDependencies; @@ -108,7 +105,7 @@ public bool CanInline(MethodDesc caller, MethodDesc callee) public bool CanConstructType(TypeDesc type) { - return _devirtualizationManager.CanConstructType(type); + return NodeFactory.DevirtualizationManager.CanConstructType(type); } public DelegateCreationInfo GetDelegateCtor(TypeDesc delegateType, MethodDesc target, TypeDesc constrainedType, bool followVirtualDispatch) @@ -215,22 +212,22 @@ public bool HasFixedSlotVTable(TypeDesc type) public bool IsEffectivelySealed(TypeDesc type) { - return _devirtualizationManager.IsEffectivelySealed(type); + return NodeFactory.DevirtualizationManager.IsEffectivelySealed(type); } public TypeDesc[] GetImplementingClasses(TypeDesc type) { - return _devirtualizationManager.GetImplementingClasses(type); + return NodeFactory.DevirtualizationManager.GetImplementingClasses(type); } public bool IsEffectivelySealed(MethodDesc method) { - return _devirtualizationManager.IsEffectivelySealed(method); + return NodeFactory.DevirtualizationManager.IsEffectivelySealed(method); } public MethodDesc ResolveVirtualMethod(MethodDesc declMethod, TypeDesc implType, out CORINFO_DEVIRTUALIZATION_DETAIL devirtualizationDetail) { - return _devirtualizationManager.ResolveVirtualMethod(declMethod, implType, out devirtualizationDetail); + return NodeFactory.DevirtualizationManager.ResolveVirtualMethod(declMethod, implType, out devirtualizationDetail); } public bool NeedsRuntimeLookup(ReadyToRunHelperId lookupKind, object targetOfLookup) @@ -264,7 +261,7 @@ public bool NeedsRuntimeLookup(ReadyToRunHelperId lookupKind, object targetOfLoo public ReadyToRunHelperId GetLdTokenHelperForType(TypeDesc type) { - bool canConstructPerWholeProgramAnalysis = _devirtualizationManager == null ? true : _devirtualizationManager.CanConstructType(type); + bool canConstructPerWholeProgramAnalysis = NodeFactory.DevirtualizationManager.CanConstructType(type); bool creationAllowed = ConstructedEETypeNode.CreationAllowed(type); return (canConstructPerWholeProgramAnalysis && creationAllowed) ? ReadyToRunHelperId.TypeHandle 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 738f76f27f11d4..2b2a0313a397d1 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) + : base(context, compilationModuleGroup, metadataManager, interopStubManager, nameMangler, new LazyGenericsDisabledPolicy(), new LazyVTableSliceProvider(), new LazyDictionaryLayoutProvider(), new InlinedThreadStatics(), new ExternSymbolsImportedNodeProvider(), preinitManager, new DevirtualizationManager()) { } 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 5e3674acd39123..0028ab9b188e0b 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -39,7 +39,8 @@ public NodeFactory( DictionaryLayoutProvider dictionaryLayoutProvider, InlinedThreadStatics inlinedThreadStatics, ImportedNodeProvider importedNodeProvider, - PreinitializationManager preinitializationManager) + PreinitializationManager preinitializationManager, + DevirtualizationManager devirtualizationManager) { _target = context.Target; _context = context; @@ -54,6 +55,7 @@ public NodeFactory( LazyGenericsPolicy = lazyGenericsPolicy; _importedNodeProvider = importedNodeProvider; PreinitializationManager = preinitializationManager; + DevirtualizationManager = devirtualizationManager; } public void SetMarkingComplete() @@ -103,6 +105,11 @@ public PreinitializationManager PreinitializationManager get; } + public DevirtualizationManager DevirtualizationManager + { + get; + } + public InteropStubManager InteropStubManager { get; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index 5355c4be9ff127..630de4a7470645 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -36,7 +36,7 @@ internal ILScanner( DebugInformationProvider debugInformationProvider, Logger logger, int parallelism) - : base(dependencyGraph, nodeFactory, roots, ilProvider, debugInformationProvider, null, nodeFactory.CompilationModuleGroup, logger) + : base(dependencyGraph, nodeFactory, roots, ilProvider, debugInformationProvider, nodeFactory.CompilationModuleGroup, logger) { _helperCache = new HelperCache(this); _parallelism = parallelism; 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 abae360824375f..beed49149f6d86 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs @@ -10,8 +10,9 @@ namespace ILCompiler.DependencyAnalysis 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) - : base(context, compilationModuleGroup, metadataManager, interopStubManager, nameMangler, new LazyGenericsDisabledPolicy(), vtableSliceProvider, dictionaryLayoutProvider, inlinedThreadStatics, new ExternSymbolsImportedNodeProvider(), preinitializationManager) + 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) { } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs index b20a3a470d0378..a0dd7850ae8806 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilation.cs @@ -38,7 +38,6 @@ internal RyuJitCompilation( ILProvider ilProvider, DebugInformationProvider debugInformationProvider, Logger logger, - DevirtualizationManager devirtualizationManager, IInliningPolicy inliningPolicy, InstructionSetSupport instructionSetSupport, ProfileDataManager profileDataManager, @@ -46,7 +45,7 @@ internal RyuJitCompilation( ReadOnlyFieldPolicy readOnlyFieldPolicy, RyuJitCompilationOptions options, int parallelism) - : base(dependencyGraph, nodeFactory, roots, ilProvider, debugInformationProvider, devirtualizationManager, inliningPolicy, logger) + : base(dependencyGraph, nodeFactory, roots, ilProvider, debugInformationProvider, inliningPolicy, logger) { _compilationOptions = options; InstructionSetSupport = instructionSetSupport; @@ -73,8 +72,7 @@ public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type) // information proving that it isn't, give RyuJIT the constructed symbol even // though we just need the unconstructed one. // https://github.com/dotnet/runtimelab/issues/1128 - bool canPotentiallyConstruct = _devirtualizationManager == null - ? true : _devirtualizationManager.CanConstructType(type); + bool canPotentiallyConstruct = NodeFactory.DevirtualizationManager.CanConstructType(type); if (canPotentiallyConstruct) return _nodeFactory.MaximallyConstructableType(type); @@ -83,8 +81,7 @@ public override IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type) public FrozenRuntimeTypeNode NecessaryRuntimeTypeIfPossible(TypeDesc type) { - bool canPotentiallyConstruct = _devirtualizationManager == null - ? true : _devirtualizationManager.CanConstructType(type); + bool canPotentiallyConstruct = NodeFactory.DevirtualizationManager.CanConstructType(type); if (canPotentiallyConstruct) return _nodeFactory.SerializedMaximallyConstructableRuntimeTypeObject(type); diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs index 227b8f69cd98c3..943dc9dc912800 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/RyuJitCompilationBuilder.cs @@ -123,11 +123,11 @@ public override ICompilation ToCompilation() if (_resilient) options |= RyuJitCompilationOptions.UseResilience; - var factory = new RyuJitNodeFactory(_context, _compilationGroup, _metadataManager, _interopStubManager, _nameMangler, _vtableSliceProvider, _dictionaryLayoutProvider, _inlinedThreadStatics, GetPreinitializationManager()); + var factory = new RyuJitNodeFactory(_context, _compilationGroup, _metadataManager, _interopStubManager, _nameMangler, _vtableSliceProvider, _dictionaryLayoutProvider, _inlinedThreadStatics, GetPreinitializationManager(), _devirtualizationManager); JitConfigProvider.Initialize(_context.Target, jitFlagBuilder.ToArray(), _ryujitOptions, _jitPath); DependencyAnalyzerBase graph = CreateDependencyGraph(factory, new ObjectNode.ObjectNodeComparer(CompilerComparer.Instance)); - return new RyuJitCompilation(graph, factory, _compilationRoots, _ilProvider, _debugInformationProvider, _logger, _devirtualizationManager, _inliningPolicy ?? _compilationGroup, _instructionSetSupport, _profileDataManager, _methodImportationErrorProvider, _readOnlyFieldPolicy, options, _parallelism); + return new RyuJitCompilation(graph, factory, _compilationRoots, _ilProvider, _debugInformationProvider, _logger, _inliningPolicy ?? _compilationGroup, _instructionSetSupport, _profileDataManager, _methodImportationErrorProvider, _readOnlyFieldPolicy, options, _parallelism); } } }