From: Michal Strehovský Date: Tue, 10 Jan 2023 07:48:18 +0000 (+0900) Subject: Generate fewer TentativeInstanceMethod nodes (#80414) X-Git-Tag: accepted/tizen/unified/riscv/20231226.055536~4730 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a4cd3224726e97bd5fa2a1303fde584931e707d3;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Generate fewer TentativeInstanceMethod nodes (#80414) In order for the compiler to be able to eliminate bodies of instance methods on types that were never allocated, the compiler has a concept of "tentative instance methods". E.g. ```csharp Foo f = null; f?.DoSomething(); class Foo { public void DoSomething() { expensive stuff } } ``` When compiling `Main()` instead of generating code that refers to the entrypoint of `DoSomething` (that we would then have to compile), we generate it as a reference to a "tentative method". This tentative method will either turn into a throw helper, or the real instance method. This depends on whether the owning type of the method was seen as allocated - the tentative method conditionally depends on the real body if the type was allocated. Before this PR, tentative methods were dispensed from `MethodEntrypoint` method on `NodeFactory`. I.e. anyone asking for a method entrypoint could potentially get a tentative entrypoint. But this also means that places that refer to method bodies and already know the owning type is allocated still introduce an unnecessary tentative entrypoint node. They should refer to the entrypoint directly. This pull request does that. One now needs to explicitly ask for a tentative entrypoint - `MethodEntrypoint`. We actually only have two places that really need to do it. This is pretty much a zero diff change - we only change bookkeeping within the compiler. I saw one difference in WebApi - references to finalizers from unconstructed MethodTables now bring the real method. We can fix that multiple ways. I couldn't decide the best one, so I left it out for now (should we ask for a tentative entrypoint? should we fill it out as zero? should we say unconstructed types don't have finalizers and skip it?) --- 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 c17647d070a..90e29d20bcc 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ILScanNodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ILScanNodeFactory.cs @@ -42,10 +42,6 @@ namespace ILCompiler.DependencyAnalysis if (CompilationModuleGroup.ContainsMethodBody(method, false)) { - // We might be able to optimize the method body away if the owning type was never seen as allocated. - if (method.NotCallableWithoutOwningEEType() && CompilationModuleGroup.AllowInstanceMethodOptimization(method)) - return new TentativeInstanceMethodNode(new ScannedMethodNode(method)); - return new ScannedMethodNode(method); } else 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 be70251c991..df4367fde97 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs @@ -255,6 +255,11 @@ namespace ILCompiler.DependencyAnalysis _methodEntrypoints = new MethodEntrypointHashtable(this); + _tentativeMethods = new NodeCache(method => + { + return new TentativeInstanceMethodNode((IMethodBodyNode)MethodEntrypoint(method)); + }); + _unboxingStubs = new NodeCache(CreateUnboxingStubNode); _methodAssociatedData = new NodeCache(methodNode => @@ -839,6 +844,19 @@ namespace ILCompiler.DependencyAnalysis return _methodEntrypoints.GetOrCreateValue(method); } + private NodeCache _tentativeMethods; + public IMethodNode MethodEntrypointOrTentativeMethod(MethodDesc method, bool unboxingStub = false) + { + // We might be able to optimize the method body away if the owning type was never seen as allocated. + if (method.NotCallableWithoutOwningEEType() && CompilationModuleGroup.AllowInstanceMethodOptimization(method)) + { + Debug.Assert(!unboxingStub); + return _tentativeMethods.GetOrAdd(method); + } + + return MethodEntrypoint(method, unboxingStub); + } + public MethodAssociatedDataNode MethodAssociatedData(IMethodNode methodNode) { return _methodAssociatedData.GetOrAdd(methodNode); diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MultiFileCompilationModuleGroup.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MultiFileCompilationModuleGroup.cs index 05888aebfef..80b32320c38 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MultiFileCompilationModuleGroup.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MultiFileCompilationModuleGroup.cs @@ -119,9 +119,12 @@ namespace ILCompiler // Both the instance methods and the owning type are homed in a single compilation group // so if we're able to generate the body, we would also generate the owning type here // and nowhere else. - Debug.Assert(ContainsMethodBody(method, unboxingStub: false)); - TypeDesc owningType = method.OwningType; - return owningType.IsDefType && !owningType.HasInstantiation && !method.HasInstantiation; + if (ContainsMethodBody(method, unboxingStub: false)) + { + TypeDesc owningType = method.OwningType; + return owningType.IsDefType && !owningType.HasInstantiation && !method.HasInstantiation; + } + return false; } } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs index d1946c3f5bf..1c5c06d2c3c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs @@ -247,7 +247,7 @@ namespace Internal.IL _compilation.DetectGenericCycles(_canonMethod, method); } - return _factory.MethodEntrypoint(method); + return _factory.MethodEntrypointOrTentativeMethod(method); } private void ImportCall(ILOpcode opcode, int token) 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 45060d7ffa3..a51508005f5 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/Compiler/DependencyAnalysis/RyuJitNodeFactory.cs @@ -43,10 +43,6 @@ namespace ILCompiler.DependencyAnalysis if (CompilationModuleGroup.ContainsMethodBody(method, false)) { - // We might be able to optimize the method body away if the owning type was never seen as allocated. - if (method.NotCallableWithoutOwningEEType() && CompilationModuleGroup.AllowInstanceMethodOptimization(method)) - return new TentativeInstanceMethodNode(new MethodCodeNode(method)); - return new MethodCodeNode(method); } else diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 5cda6a6ce53..895d9cc38d7 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -1139,7 +1139,7 @@ namespace Internal.JitInterface _compilation.DetectGenericCycles(methodIL.OwningMethod, method); } - return _compilation.NodeFactory.MethodEntrypoint(method, isUnboxingThunk); + return _compilation.NodeFactory.MethodEntrypointOrTentativeMethod(method, isUnboxingThunk); } private static bool IsTypeSpecForTypicalInstantiation(TypeDesc t) diff --git a/src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/TestInfraMultiFileCompilationModuleGroup.cs b/src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/TestInfraMultiFileCompilationModuleGroup.cs index e774cf7c50b..94d34de0ba5 100644 --- a/src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/TestInfraMultiFileCompilationModuleGroup.cs +++ b/src/coreclr/tools/aot/Mono.Linker.Tests/TestCasesRunner/TestInfraMultiFileCompilationModuleGroup.cs @@ -41,9 +41,11 @@ namespace Mono.Linker.Tests.TestCasesRunner // Both the instance methods and the owning type are homed in a single compilation group // so if we're able to generate the body, we would also generate the owning type here // and nowhere else. - Debug.Assert (ContainsMethodBody (method, unboxingStub: false)); - TypeDesc owningType = method.OwningType; - return owningType.IsDefType && !owningType.HasInstantiation && !method.HasInstantiation; + if (ContainsMethodBody (method, unboxingStub: false)) { + TypeDesc owningType = method.OwningType; + return owningType.IsDefType && !owningType.HasInstantiation && !method.HasInstantiation; + } + return false; } } }