From 61aaf5f86c10964592c434df9ab3cd73a7147e14 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 11 Jan 2025 15:00:59 +0100 Subject: [PATCH 1/6] Imply NoInlining when only NoOptimization is set --- src/coreclr/vm/jitinterface.cpp | 15 +++++++++++---- src/coreclr/vm/methodtablebuilder.cpp | 5 ++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 0a5e03371994dc..bf65a40abfe3db 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8200,7 +8200,7 @@ bool CEEInfo::canTailCall (CORINFO_METHOD_HANDLE hCaller, if (!pCaller->IsNoMetadata()) { - // Do not tailcall from methods that are marked as noinline (people often use no-inline + // Do not tailcall from methods that are marked as NoInlining or NoOptimization (people often use no-inline // to mean "I want to always see this method in stacktrace") DWORD dwImplFlags = 0; IfFailThrow(pCaller->GetMDImport()->GetMethodImplProps(callerToken, NULL, &dwImplFlags)); @@ -8208,7 +8208,14 @@ bool CEEInfo::canTailCall (CORINFO_METHOD_HANDLE hCaller, if (IsMiNoInlining(dwImplFlags)) { result = false; - szFailReason = "Caller is marked as no inline"; + szFailReason = "Caller is marked as NoInlining"; + goto exit; + } + + if (IsMiNoOptimization(dwImplFlags)) + { + result = false; + szFailReason = "Caller is marked as NoOptimization"; goto exit; } } @@ -12613,8 +12620,8 @@ CorJitResult invokeCompileMethod(EEJitManager *jitMgr, flags.Set(CORJIT_FLAGS::CORJIT_FLAG_MIN_OPT); } - // Always emit frames for methods marked no-inline (see #define ETW_EBP_FRAMED in the JIT) - if (IsMiNoInlining(dwImplFlags)) + // Always emit frames for methods marked NoInlining or NoOptimization (see #define ETW_EBP_FRAMED in the JIT) + if (IsMiNoInlining(dwImplFlags) || IsMiNoOptimization(dwImplFlags)) { flags.Set(CORJIT_FLAGS::CORJIT_FLAG_FRAMED); } diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 69d5aa64d34ec1..60110263e4a274 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -5219,9 +5219,8 @@ MethodTableBuilder::InitNewMethodDesc( } } - // Turn off inlining for any calls - // that are marked in the metadata as not being inlineable. - if(IsMiNoInlining(pMethod->GetImplAttrs())) + // Turn off inlining for any calls that are marked in the metadata as NoInlining or NoOptimization. + if (IsMiNoInlining(pMethod->GetImplAttrs()) || IsMiNoOptimization(pMethod->GetImplAttrs())) { pNewMD->SetNotInline(true); } From d27c1637b5a76d48d873f91353ac1aacb6d31967 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sun, 12 Jan 2025 16:16:03 +0100 Subject: [PATCH 2/6] Do the same for AOT --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 4 ++-- .../aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs | 1 + .../Compiler/ReadyToRunCodegenCompilation.cs | 3 ++- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 4 ++-- .../aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs | 4 ++-- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 5ac150b37ec474..2dad8fda1aa729 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1117,9 +1117,9 @@ private uint getMethodAttribsInternal(MethodDesc method) // TODO: Cache inlining hits // Check for an inlining directive. - if (method.IsNoInlining) + if (method.IsNoInlining || method.IsNoOptimization) { - /* Function marked as not inlineable */ + // Function marked as not NoInlining. NoOptimization also implies NoInlining. result |= CorInfoFlag.CORINFO_FLG_DONT_INLINE; } else if (method.IsAggressiveInlining) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs index fd2442c4b6b41c..4450c53ed99584 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/SubstitutedILProvider.cs @@ -672,6 +672,7 @@ private bool TryGetMethodConstantValue(MethodDesc method, out int constant, int if (returnType is < TypeFlags.Boolean or > TypeFlags.UInt32 || method.IsIntrinsic || method.IsNoInlining + || method.IsNoOptimization || _nestedILProvider.GetMethodIL(method) is not MethodIL methodIL) { constant = 0; diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs index 66c7767dab8a81..e1803a3f5d7931 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs @@ -82,8 +82,9 @@ public bool CanInline(MethodDesc caller, MethodDesc callee) return false; } - if (callee.IsNoInlining) + if (callee.IsNoInlining || callee.IsNoOptimization) { + // NoOptimization implies NoInlining return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 500863a90d9347..174f816da161af 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1299,9 +1299,9 @@ private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUC return false; } - // Do not tailcall from methods that are marked as noinline (people often use no-inline + // Do not tailcall from methods that are marked as NoInlining or NoOptimization (people often use no-inline // to mean "I want to always see this method in stacktrace") - if (caller.IsNoInlining) + if (caller.IsNoInlining || caller.IsNoOptimization) { return false; } 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 fab907801e723b..e9533e6bf6670d 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -831,9 +831,9 @@ private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUC result = false; } - if (caller.IsNoInlining) + if (caller.IsNoInlining || caller.IsNoOptimization) { - // Do not tailcall from methods that are marked as noinline (people often use no-inline + // Do not tailcall from methods that are marked as NoInlining or NoOptimization (people often use no-inline // to mean "I want to always see this method in stacktrace") result = false; } From 0b594737aec3276b91dbf844561437e0a38d9e9e Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sun, 12 Jan 2025 18:29:29 +0100 Subject: [PATCH 3/6] Simplify inlining directive comment --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 2dad8fda1aa729..c635b416708be2 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1119,7 +1119,7 @@ private uint getMethodAttribsInternal(MethodDesc method) if (method.IsNoInlining || method.IsNoOptimization) { - // Function marked as not NoInlining. NoOptimization also implies NoInlining. + // NoOptimization implies NoInlining. result |= CorInfoFlag.CORINFO_FLG_DONT_INLINE; } else if (method.IsAggressiveInlining) From 7c6e2bf1f8577d5153236324d7fb39824f6dd6e2 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 13 Jan 2025 11:02:45 +0100 Subject: [PATCH 4/6] Remove NoOptimization check from tailcall logic --- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 174f816da161af..7cc3343a56d416 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -1299,10 +1299,12 @@ private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUC return false; } - // Do not tailcall from methods that are marked as NoInlining or NoOptimization (people often use no-inline + // Do not tailcall from methods that are marked as NoInlining (people often use no-inline // to mean "I want to always see this method in stacktrace") - if (caller.IsNoInlining || caller.IsNoOptimization) + if (caller.IsNoInlining) { + // NOTE: we don't have to handle NoOptimization here, because JIT is not expected + // to emit fast tail calls if optimizations are disabled. return false; } From ed8281b477cf7cd477efbaeacfc560b5bd80298e Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 13 Jan 2025 11:03:36 +0100 Subject: [PATCH 5/6] Refine tail call logic for NoInlining --- .../ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 e9533e6bf6670d..b4933c28b25afc 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -831,10 +831,13 @@ private bool canTailCall(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHOD_STRUC result = false; } - if (caller.IsNoInlining || caller.IsNoOptimization) + if (caller.IsNoInlining) { - // Do not tailcall from methods that are marked as NoInlining or NoOptimization (people often use no-inline + // Do not tailcall from methods that are marked as NoInlining (people often use no-inline // to mean "I want to always see this method in stacktrace") + // + // NOTE: we don't have to handle NoOptimization here, because JIT is not expected + // to emit fast tail calls if optimizations are disabled. result = false; } } From 037876745d4b5b2ce1dd0966a39e548be05c4f1c Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Mon, 13 Jan 2025 11:04:39 +0100 Subject: [PATCH 6/6] Remove NoOptimization check for tail calls --- src/coreclr/vm/jitinterface.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index bf65a40abfe3db..3354a7ec0f2ab4 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -8200,7 +8200,7 @@ bool CEEInfo::canTailCall (CORINFO_METHOD_HANDLE hCaller, if (!pCaller->IsNoMetadata()) { - // Do not tailcall from methods that are marked as NoInlining or NoOptimization (people often use no-inline + // Do not tailcall from methods that are marked as NoInlining (people often use no-inline // to mean "I want to always see this method in stacktrace") DWORD dwImplFlags = 0; IfFailThrow(pCaller->GetMDImport()->GetMethodImplProps(callerToken, NULL, &dwImplFlags)); @@ -8212,12 +8212,8 @@ bool CEEInfo::canTailCall (CORINFO_METHOD_HANDLE hCaller, goto exit; } - if (IsMiNoOptimization(dwImplFlags)) - { - result = false; - szFailReason = "Caller is marked as NoOptimization"; - goto exit; - } + // NOTE: we don't have to handle NoOptimization here, because JIT is not expected + // to emit fast tail calls if optimizations are disabled. } // Methods with StackCrawlMark depend on finding their caller on the stack.