From 872a436fdd417775f9a5d9d0557a6f9f3c184e53 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 24 Mar 2025 11:35:12 -0700 Subject: [PATCH 01/10] Block inlines with different exception wrapping behavior Fixes #113815 --- src/coreclr/vm/jitinterface.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 7fa84d674018d4..011703ebaf6d7e 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7890,6 +7890,25 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller, } } + // If the root level caller and callee modules do not have the same runtime + // wrapped exception behavior, and the callee has EH, we cannot inline. + _ASSERTE(!pCallee->IsDynamicMethod()); + { + COR_ILMETHOD_DECODER header(pCallee->GetILHeader(), pCallee->GetMDImport(), NULL); + if (header.EHCount() > 0) + { + Module* pCalleeModule = pCallee->GetModule(); + Module* pRootModule = pOrigCaller->GetModule(); + + if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions()) + { + result = INLINE_FAIL; + szFailReason = "Inlinee and root method have different wrapped exception behavior"; + goto exit; + } + } + } + #ifdef PROFILING_SUPPORTED if (CORProfilerPresent()) { From 443eda1f022ad859afa47384f98dc23c22684447 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 24 Mar 2025 17:35:09 -0700 Subject: [PATCH 02/10] revise order of checks; handle transient methods --- src/coreclr/vm/jitinterface.cpp | 61 ++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 011703ebaf6d7e..484946b079fa78 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7894,17 +7894,60 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller, // wrapped exception behavior, and the callee has EH, we cannot inline. _ASSERTE(!pCallee->IsDynamicMethod()); { - COR_ILMETHOD_DECODER header(pCallee->GetILHeader(), pCallee->GetMDImport(), NULL); - if (header.EHCount() > 0) - { - Module* pCalleeModule = pCallee->GetModule(); - Module* pRootModule = pOrigCaller->GetModule(); + Module* pCalleeModule = pCallee->GetModule(); + Module* pRootModule = pOrigCaller->GetModule(); - if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions()) + if (pRootModule->IsRuntimeWrapExceptions() != pCalleeModule->IsRuntimeWrapExceptions()) + { + if (pCallee->HasILHeader()) { - result = INLINE_FAIL; - szFailReason = "Inlinee and root method have different wrapped exception behavior"; - goto exit; + COR_ILMETHOD_DECODER header(pCallee->GetILHeader(), pCallee->GetMDImport(), NULL); + if (header.EHCount() > 0) + { + result = INLINE_FAIL; + szFailReason = "Inlinee and root method have different wrapped exception behavior"; + goto exit; + } + } + else if (pCallee->IsIL() && pCallee->GetRVA() == 0) + { + MethodInfoHelperContext cxt{ pCallee }; + // We will either find or create transient method details. + _ASSERTE(!cxt.HasTransientMethodDetails()); + + // IL methods with no RVA indicate there is no implementation defined in metadata. + // Check if we previously generated details/implementation for this method. + TransientMethodDetails* detailsMaybe = NULL; + if (FindTransientMethodDetails(pCallee, &detailsMaybe)) + cxt.UpdateWith(*detailsMaybe); + + CORINFO_METHOD_INFO methodInfo; + getMethodInfoHelper(cxt, &methodInfo); + + if (cxt.Header->EHCount() > 0) + { + result = INLINE_FAIL; + szFailReason = "Inlinee and root method have different wrapped exception behavior"; + } + + // If we have transient method details we need to handle + // the lifetime of the details. + if (cxt.HasTransientMethodDetails()) + { + // If we didn't find transient details, but we have them + // after getting method info, store them for cleanup. + if (detailsMaybe == NULL) + AddTransientMethodDetails(cxt.CreateTransientMethodDetails()); + + // Reset the context because ownership has either been + // transferred or was found in this instance. + cxt.UpdateWith({}); + } + + if (result != INLINE_PASS) + { + goto exit; + } } } } From d40c90f2850ba65725620541b60784910a6c993b Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 25 Mar 2025 11:03:58 -0700 Subject: [PATCH 03/10] first part of crossgen2 support --- .../Common/Compiler/CompilationModuleGroup.cs | 7 ++++--- .../tools/Common/JitInterface/CorInfoImpl.cs | 8 ++++++-- .../Compiler/Compilation.cs | 4 ++-- .../Compiler/IInliningPolicy.cs | 2 +- .../ILCompiler.Compiler/Compiler/ILScanner.cs | 4 ++-- .../Compiler/TypePreinit.cs | 4 ++-- .../Compiler/ReadyToRunCodegenCompilation.cs | 20 +++++++++++++++++-- .../ReadyToRunCompilationModuleGroupBase.cs | 2 +- 8 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs b/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs index a72e5c65b0dbd8..5a3d7777774d41 100644 --- a/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs +++ b/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs @@ -18,8 +18,9 @@ public abstract partial class CompilationModuleGroup /// /// Decide whether a given call may get inlined by JIT. /// - /// Calling method the assembly code of is about to receive the callee code - /// The called method to be inlined into the caller - public virtual bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod) => true; + /// Root method + /// Immediate caller of the calleeMethod (either root method, or a method already inlined into the root method) + /// The method to be inlined into root method + public virtual bool CanInline(MethodDesc root, MethodDesc callerMethod, MethodDesc calleeMethod) => true; } } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index b27b0c5e81b746..8461f726539880 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1234,7 +1234,7 @@ private bool getMethodInfo(CORINFO_METHOD_STRUCT_* ftn, CORINFO_METHOD_INFO* inf // Add an early CanInline check to see if referring to the IL of the target methods is // permitted from within this MethodBeingCompiled, the full CanInline check will be performed // later. - if (!_compilation.CanInline(MethodBeingCompiled, method)) + if (!_compilation.CanInline(MethodBeingCompiled, MethodBeingCompiled, method)) return false; MethodIL methodIL = method.IsUnboxingThunk() ? null : _compilation.GetMethodIL(method); @@ -1258,7 +1258,7 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO MethodDesc callerMethod = HandleToObject(callerHnd); MethodDesc calleeMethod = HandleToObject(calleeHnd); - if (_compilation.CanInline(callerMethod, calleeMethod)) + if (_compilation.CanInline(MethodBeingCompiled, callerMethod, calleeMethod)) { // No restrictions on inlining return CorInfoInline.INLINE_PASS; @@ -1266,6 +1266,10 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO else { // Call may not be inlined + // + // Note despite returning INLINE_NEVER here, in compilations where jitting is possible + // the jit may still be able to inline this method. So we rely on reportInliningDecision + // to not propagate this into metadata to short-circuit future inlining attempts. return CorInfoInline.INLINE_NEVER; } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index ecd3df3a3915c8..80a1955c95835f 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -98,9 +98,9 @@ public virtual IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type) return _nodeFactory.NecessaryTypeSymbol(type); } - public bool CanInline(MethodDesc caller, MethodDesc callee) + public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) { - return _inliningPolicy.CanInline(caller, callee); + return _inliningPolicy.CanInline(root, caller, callee); } public bool CanReferenceConstructedMethodTable(TypeDesc type) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs index 9515f8e206c25e..c50c52bd1684fb 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs @@ -7,6 +7,6 @@ namespace ILCompiler { public interface IInliningPolicy { - bool CanInline(MethodDesc caller, MethodDesc callee); + bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee); } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index efad0516f5a354..4435d9e02bcba1 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -800,9 +800,9 @@ public ScannedInliningPolicy(CompilationModuleGroup baseGroup, ImmutableArray> obj); - public bool CanInline(MethodDesc caller, MethodDesc callee) + public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) { if (JitConfigProvider.Instance.HasFlag(CorJitFlag.CORJIT_FLAG_DEBUG_CODE)) { @@ -107,9 +107,25 @@ public bool CanInline(MethodDesc caller, MethodDesc callee) } } + EcmaModule rootModule = (root.OwningType as MetadataType)?.Module as EcmaModule; + EcmaModule calleeModule = (callee.OwningType as MetadataType)?.Module as EcmaModule; + + if ((rootModule != null) && (calleeModule != null)) + { + //if (rootModule.IsRuntimeWrapExceptions != calleeModule.IsRuntimeWrapExceptions) + //{ + // var calleeIL = GetMethodIL(callee); + // if (calleeIL.GetExceptionRegions().Length != 0) + // { + // // Fail inlining if root method and callee have different exception wrapping behavior + // return false; + // } + //} + } + _nodeFactory.DetectGenericCycles(caller, callee); - return NodeFactory.CompilationModuleGroup.CanInline(caller, callee); + return NodeFactory.CompilationModuleGroup.CanInline(root, caller, callee); } public virtual MethodIL GetMethodIL(MethodDesc method) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs index 09c12867e50b16..7790c914130479 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs @@ -404,7 +404,7 @@ private bool VersionsWithMethodUncached(MethodDesc method) return ComputeInstantiationVersionsWithCode(method.Instantiation, method, VersionsWithType); } - public sealed override bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod) + public sealed override bool CanInline(MethodDesc root, MethodDesc callerMethod, MethodDesc calleeMethod) { // Allow inlining if the caller is within the current version bubble // (because otherwise we may not be able to encode its tokens) From f1c72f3031914c0050b30f1be5561d3c3a20b065 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 1 Apr 2025 11:51:26 -0700 Subject: [PATCH 04/10] rest of crossgen2 support --- .../Common/TypeSystem/Ecma/EcmaModule.cs | 38 +++++++++++++++++++ .../Compiler/ReadyToRunCodegenCompilation.cs | 18 ++++----- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs index 6758f4a549866c..dec98ecbaad87c 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs @@ -16,6 +16,8 @@ public partial class EcmaModule : ModuleDesc { private readonly PEReader _peReader; protected readonly MetadataReader _metadataReader; + private bool _isWrapNonExceptionThrowsKnown; + private bool _isWrapNonExceptionThrows; internal interface IEntityHandleObject { @@ -690,5 +692,41 @@ public override string ToString() ModuleDefinition moduleDefinition = _metadataReader.GetModuleDefinition(); return _metadataReader.GetString(moduleDefinition.Name); } + + public bool IsWrapNonExceptionThrows + { + get + { + if (!_isWrapNonExceptionThrowsKnown) + { + var reader = MetadataReader; + var c = reader.StringComparer; + foreach (var attr in reader.GetModuleDefinition().GetCustomAttributes()) + { + if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n)) + { + if (c.Equals(ns, "System.Runtime.CompilerServices") && c.Equals(n, "RuntimeCompatibilityAttribute")) + { + var dec = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider(this)); + + foreach (var arg in dec.NamedArguments) + { + if (arg.Name == "WrapNonExceptionThrows") + { + if (!(arg.Value is bool)) + ThrowHelper.ThrowBadImageFormatException(); + _isWrapNonExceptionThrows = (bool)arg.Value; + break; + } + } + } + } + } + + _isWrapNonExceptionThrowsKnown = true; + } + return _isWrapNonExceptionThrows; + } + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs index 6223c34bcd510f..5468aeaddc8224 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs @@ -112,15 +112,15 @@ public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) if ((rootModule != null) && (calleeModule != null)) { - //if (rootModule.IsRuntimeWrapExceptions != calleeModule.IsRuntimeWrapExceptions) - //{ - // var calleeIL = GetMethodIL(callee); - // if (calleeIL.GetExceptionRegions().Length != 0) - // { - // // Fail inlining if root method and callee have different exception wrapping behavior - // return false; - // } - //} + if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows) + { + var calleeIL = GetMethodIL(callee); + if (calleeIL.GetExceptionRegions().Length != 0) + { + // Fail inlining if root method and callee have different exception wrapping behavior + return false; + } + } } _nodeFactory.DetectGenericCycles(caller, callee); From ec2b2cd64f99882a77e67c3d2b4b62c9cb712bda Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 1 Apr 2025 11:58:30 -0700 Subject: [PATCH 05/10] add bypass for same module inlines --- .../Compiler/ReadyToRunCodegenCompilation.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs index 5468aeaddc8224..5b076007684892 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs @@ -110,7 +110,8 @@ public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) EcmaModule rootModule = (root.OwningType as MetadataType)?.Module as EcmaModule; EcmaModule calleeModule = (callee.OwningType as MetadataType)?.Module as EcmaModule; - if ((rootModule != null) && (calleeModule != null)) + // If this inline crosses module boundaries, ensure the modules agree on exception wrapping behavior. + if ((rootModule != calleeModule) && (rootModule != null) && (calleeModule != null)) { if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows) { From 49ff8b7de093d115e824a506d1481586e6227a34 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 1 Apr 2025 13:23:59 -0700 Subject: [PATCH 06/10] move the check to CorInfoImpl, add volatile --- .../tools/Common/JitInterface/CorInfoImpl.cs | 17 +++++++++++++++++ .../tools/Common/TypeSystem/Ecma/EcmaModule.cs | 4 ++-- .../Compiler/ReadyToRunCodegenCompilation.cs | 17 ----------------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 5c7c96b50be2b3..0b5651dff2fb50 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1258,6 +1258,23 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO MethodDesc callerMethod = HandleToObject(callerHnd); MethodDesc calleeMethod = HandleToObject(calleeHnd); + EcmaModule rootModule = (MethodBeingCompiled.OwningType as MetadataType)?.Module as EcmaModule; + EcmaModule calleeModule = (calleeMethod.OwningType as MetadataType)?.Module as EcmaModule; + + // If this inline crosses module boundaries, ensure the modules agree on exception wrapping behavior. + if ((rootModule != calleeModule) && (rootModule != null) && (calleeModule != null)) + { + if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows) + { + var calleeIL = _compilation.GetMethodIL(calleeMethod); + if (calleeIL.GetExceptionRegions().Length != 0) + { + // Fail inlining if root method and callee have different exception wrapping behavior + return CorInfoInline.INLINE_FAIL; + } + } + } + if (_compilation.CanInline(MethodBeingCompiled, callerMethod, calleeMethod)) { // No restrictions on inlining diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs index dec98ecbaad87c..a1b3985f591472 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs @@ -16,8 +16,8 @@ public partial class EcmaModule : ModuleDesc { private readonly PEReader _peReader; protected readonly MetadataReader _metadataReader; - private bool _isWrapNonExceptionThrowsKnown; - private bool _isWrapNonExceptionThrows; + private volatile bool _isWrapNonExceptionThrowsKnown; + private volatile bool _isWrapNonExceptionThrows; internal interface IEntityHandleObject { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs index 5b076007684892..4673de6783a5af 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs @@ -107,23 +107,6 @@ public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) } } - EcmaModule rootModule = (root.OwningType as MetadataType)?.Module as EcmaModule; - EcmaModule calleeModule = (callee.OwningType as MetadataType)?.Module as EcmaModule; - - // If this inline crosses module boundaries, ensure the modules agree on exception wrapping behavior. - if ((rootModule != calleeModule) && (rootModule != null) && (calleeModule != null)) - { - if (rootModule.IsWrapNonExceptionThrows != calleeModule.IsWrapNonExceptionThrows) - { - var calleeIL = GetMethodIL(callee); - if (calleeIL.GetExceptionRegions().Length != 0) - { - // Fail inlining if root method and callee have different exception wrapping behavior - return false; - } - } - } - _nodeFactory.DetectGenericCycles(caller, callee); return NodeFactory.CompilationModuleGroup.CanInline(root, caller, callee); From ccd6a8deaf4e5be07da12ed78dac99abcefb64fd Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 1 Apr 2025 13:35:35 -0700 Subject: [PATCH 07/10] remove root method arg from policy checks --- .../tools/Common/Compiler/CompilationModuleGroup.cs | 7 +++---- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 4 ++-- .../tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs | 4 ++-- .../aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs | 2 +- .../tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs | 4 ++-- .../tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs | 4 ++-- .../Compiler/ReadyToRunCodegenCompilation.cs | 4 ++-- .../Compiler/ReadyToRunCompilationModuleGroupBase.cs | 2 +- 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs b/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs index 5a3d7777774d41..a72e5c65b0dbd8 100644 --- a/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs +++ b/src/coreclr/tools/Common/Compiler/CompilationModuleGroup.cs @@ -18,9 +18,8 @@ public abstract partial class CompilationModuleGroup /// /// Decide whether a given call may get inlined by JIT. /// - /// Root method - /// Immediate caller of the calleeMethod (either root method, or a method already inlined into the root method) - /// The method to be inlined into root method - public virtual bool CanInline(MethodDesc root, MethodDesc callerMethod, MethodDesc calleeMethod) => true; + /// Calling method the assembly code of is about to receive the callee code + /// The called method to be inlined into the caller + public virtual bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod) => true; } } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 0b5651dff2fb50..2ee4217799b003 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -1234,7 +1234,7 @@ private bool getMethodInfo(CORINFO_METHOD_STRUCT_* ftn, CORINFO_METHOD_INFO* inf // Add an early CanInline check to see if referring to the IL of the target methods is // permitted from within this MethodBeingCompiled, the full CanInline check will be performed // later. - if (!_compilation.CanInline(MethodBeingCompiled, MethodBeingCompiled, method)) + if (!_compilation.CanInline(MethodBeingCompiled, method)) return false; MethodIL methodIL = method.IsUnboxingThunk() ? null : _compilation.GetMethodIL(method); @@ -1275,7 +1275,7 @@ private CorInfoInline canInline(CORINFO_METHOD_STRUCT_* callerHnd, CORINFO_METHO } } - if (_compilation.CanInline(MethodBeingCompiled, callerMethod, calleeMethod)) + if (_compilation.CanInline(callerMethod, calleeMethod)) { // No restrictions on inlining return CorInfoInline.INLINE_PASS; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs index 80a1955c95835f..ecd3df3a3915c8 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs @@ -98,9 +98,9 @@ public virtual IEETypeNode NecessaryTypeSymbolIfPossible(TypeDesc type) return _nodeFactory.NecessaryTypeSymbol(type); } - public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) + public bool CanInline(MethodDesc caller, MethodDesc callee) { - return _inliningPolicy.CanInline(root, caller, callee); + return _inliningPolicy.CanInline(caller, callee); } public bool CanReferenceConstructedMethodTable(TypeDesc type) diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs index c50c52bd1684fb..9515f8e206c25e 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/IInliningPolicy.cs @@ -7,6 +7,6 @@ namespace ILCompiler { public interface IInliningPolicy { - bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee); + bool CanInline(MethodDesc caller, MethodDesc callee); } } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs index 4435d9e02bcba1..efad0516f5a354 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ILScanner.cs @@ -800,9 +800,9 @@ public ScannedInliningPolicy(CompilationModuleGroup baseGroup, ImmutableArray> obj); - public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) + public bool CanInline(MethodDesc caller, MethodDesc callee) { if (JitConfigProvider.Instance.HasFlag(CorJitFlag.CORJIT_FLAG_DEBUG_CODE)) { @@ -109,7 +109,7 @@ public bool CanInline(MethodDesc root, MethodDesc caller, MethodDesc callee) _nodeFactory.DetectGenericCycles(caller, callee); - return NodeFactory.CompilationModuleGroup.CanInline(root, caller, callee); + return NodeFactory.CompilationModuleGroup.CanInline(caller, callee); } public virtual MethodIL GetMethodIL(MethodDesc method) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs index 7790c914130479..09c12867e50b16 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs @@ -404,7 +404,7 @@ private bool VersionsWithMethodUncached(MethodDesc method) return ComputeInstantiationVersionsWithCode(method.Instantiation, method, VersionsWithType); } - public sealed override bool CanInline(MethodDesc root, MethodDesc callerMethod, MethodDesc calleeMethod) + public sealed override bool CanInline(MethodDesc callerMethod, MethodDesc calleeMethod) { // Allow inlining if the caller is within the current version bubble // (because otherwise we may not be able to encode its tokens) From c5b83c68ce205927c15f91001adc059914ea97c8 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 8 Apr 2025 17:49:18 -0700 Subject: [PATCH 08/10] look for the attribute on the assembly, not the module --- src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs index a1b3985f591472..e553fd70a08603 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs @@ -701,7 +701,7 @@ public bool IsWrapNonExceptionThrows { var reader = MetadataReader; var c = reader.StringComparer; - foreach (var attr in reader.GetModuleDefinition().GetCustomAttributes()) + foreach (var attr in reader.GetAssemblyDefinition().GetCustomAttributes()) { if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n)) { From 6ac2da9c19e37ea4298cfc57e6f7e01d08a2c097 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 10 Apr 2025 13:23:28 -0700 Subject: [PATCH 09/10] revise crossgen2 check --- .../Common/TypeSystem/Ecma/EcmaModule.cs | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs index e553fd70a08603..cd87728b53e852 100644 --- a/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs +++ b/src/coreclr/tools/Common/TypeSystem/Ecma/EcmaModule.cs @@ -16,7 +16,7 @@ public partial class EcmaModule : ModuleDesc { private readonly PEReader _peReader; protected readonly MetadataReader _metadataReader; - private volatile bool _isWrapNonExceptionThrowsKnown; + private volatile bool _isWrapNonExceptionThrowsComputed; private volatile bool _isWrapNonExceptionThrows; internal interface IEntityHandleObject @@ -697,35 +697,46 @@ public bool IsWrapNonExceptionThrows { get { - if (!_isWrapNonExceptionThrowsKnown) + if (!_isWrapNonExceptionThrowsComputed) { - var reader = MetadataReader; - var c = reader.StringComparer; - foreach (var attr in reader.GetAssemblyDefinition().GetCustomAttributes()) + ComputeIsWrapNonExceptionThrows(); + _isWrapNonExceptionThrowsComputed = true; + } + return _isWrapNonExceptionThrows; + } + } + + private void ComputeIsWrapNonExceptionThrows() + { + var reader = MetadataReader; + var c = reader.StringComparer; + bool foundAttribute = false; + foreach (var attr in reader.GetAssemblyDefinition().GetCustomAttributes()) + { + if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n)) + { + if (c.Equals(ns, "System.Runtime.CompilerServices") && c.Equals(n, "RuntimeCompatibilityAttribute")) { - if (reader.GetAttributeNamespaceAndName(attr, out var ns, out var n)) + var dec = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider(this)); + + foreach (var arg in dec.NamedArguments) { - if (c.Equals(ns, "System.Runtime.CompilerServices") && c.Equals(n, "RuntimeCompatibilityAttribute")) + if (arg.Name == "WrapNonExceptionThrows") { - var dec = reader.GetCustomAttribute(attr).DecodeValue(new CustomAttributeTypeProvider(this)); - - foreach (var arg in dec.NamedArguments) - { - if (arg.Name == "WrapNonExceptionThrows") - { - if (!(arg.Value is bool)) - ThrowHelper.ThrowBadImageFormatException(); - _isWrapNonExceptionThrows = (bool)arg.Value; - break; - } - } + if (!(arg.Value is bool)) + ThrowHelper.ThrowBadImageFormatException(); + _isWrapNonExceptionThrows = (bool)arg.Value; + foundAttribute = true; + break; } } } + } - _isWrapNonExceptionThrowsKnown = true; + if (foundAttribute) + { + break; } - return _isWrapNonExceptionThrows; } } } From d0fd40fee8cc83f8988c2b50aa4a94c5e62d1e6a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 10 Apr 2025 14:34:16 -0700 Subject: [PATCH 10/10] revise jit interface code --- src/coreclr/vm/jitinterface.cpp | 89 +++++++++-------------- src/coreclr/vm/jitinterface.h | 3 + src/tests/JIT/Directed/throwbox/filter.il | 2 +- 3 files changed, 38 insertions(+), 56 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 484946b079fa78..057ee8d2c19561 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7668,6 +7668,37 @@ static void getMethodInfoHelper( &methInfo->locals); } // getMethodInfoHelper + +void CEEInfo::getTransientMethodInfo(MethodDesc* pMD, CORINFO_METHOD_INFO* methInfo) +{ + MethodInfoHelperContext cxt{ pMD }; + + // We will either find or create transient method details. + _ASSERTE(!cxt.HasTransientMethodDetails()); + + // IL methods with no RVA indicate there is no implementation defined in metadata. + // Check if we previously generated details/implementation for this method. + TransientMethodDetails* detailsMaybe = NULL; + if (FindTransientMethodDetails(pMD, &detailsMaybe)) + cxt.UpdateWith(*detailsMaybe); + + getMethodInfoHelper(cxt, methInfo); + + // If we have transient method details we need to handle + // the lifetime of the details. + if (cxt.HasTransientMethodDetails()) + { + // If we didn't find transient details, but we have them + // after getting method info, store them for cleanup. + if (detailsMaybe == NULL) + AddTransientMethodDetails(cxt.CreateTransientMethodDetails()); + + // Reset the context because ownership has either been + // transferred or was found in this instance. + cxt.UpdateWith({}); + } +} + //--------------------------------------------------------------------------------------- // bool @@ -7704,31 +7735,7 @@ CEEInfo::getMethodInfo( } else if (ftn->IsIL() && ftn->GetRVA() == 0) { - // We will either find or create transient method details. - _ASSERTE(!cxt.HasTransientMethodDetails()); - - // IL methods with no RVA indicate there is no implementation defined in metadata. - // Check if we previously generated details/implementation for this method. - TransientMethodDetails* detailsMaybe = NULL; - if (FindTransientMethodDetails(ftn, &detailsMaybe)) - cxt.UpdateWith(*detailsMaybe); - - getMethodInfoHelper(cxt, methInfo); - - // If we have transient method details we need to handle - // the lifetime of the details. - if (cxt.HasTransientMethodDetails()) - { - // If we didn't find transient details, but we have them - // after getting method info, store them for cleanup. - if (detailsMaybe == NULL) - AddTransientMethodDetails(cxt.CreateTransientMethodDetails()); - - // Reset the context because ownership has either been - // transferred or was found in this instance. - cxt.UpdateWith({}); - } - + getTransientMethodInfo(ftn, methInfo); result = true; } @@ -7911,41 +7918,13 @@ CorInfoInline CEEInfo::canInline (CORINFO_METHOD_HANDLE hCaller, } else if (pCallee->IsIL() && pCallee->GetRVA() == 0) { - MethodInfoHelperContext cxt{ pCallee }; - // We will either find or create transient method details. - _ASSERTE(!cxt.HasTransientMethodDetails()); - - // IL methods with no RVA indicate there is no implementation defined in metadata. - // Check if we previously generated details/implementation for this method. - TransientMethodDetails* detailsMaybe = NULL; - if (FindTransientMethodDetails(pCallee, &detailsMaybe)) - cxt.UpdateWith(*detailsMaybe); - CORINFO_METHOD_INFO methodInfo; - getMethodInfoHelper(cxt, &methodInfo); + getTransientMethodInfo(pCallee, &methodInfo); - if (cxt.Header->EHCount() > 0) + if (methodInfo.EHcount > 0) { result = INLINE_FAIL; szFailReason = "Inlinee and root method have different wrapped exception behavior"; - } - - // If we have transient method details we need to handle - // the lifetime of the details. - if (cxt.HasTransientMethodDetails()) - { - // If we didn't find transient details, but we have them - // after getting method info, store them for cleanup. - if (detailsMaybe == NULL) - AddTransientMethodDetails(cxt.CreateTransientMethodDetails()); - - // Reset the context because ownership has either been - // transferred or was found in this instance. - cxt.UpdateWith({}); - } - - if (result != INLINE_PASS) - { goto exit; } } diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 7aedbbb0338292..0d85ebddfdff2f 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -472,6 +472,9 @@ class CEEInfo : public ICorJitInfo TransientMethodDetails RemoveTransientMethodDetails(MethodDesc* pMD); bool FindTransientMethodDetails(MethodDesc* pMD, TransientMethodDetails** details); + // Get method info for a transient method + void getTransientMethodInfo(MethodDesc* pMD, CORINFO_METHOD_INFO* methInfo); + protected: SArray* m_pJitHandles; // GC handles used by JIT MethodDesc* m_pMethodBeingCompiled; // Top-level method being compiled diff --git a/src/tests/JIT/Directed/throwbox/filter.il b/src/tests/JIT/Directed/throwbox/filter.il index d98bd4e2e680eb..d70da72f192aee 100644 --- a/src/tests/JIT/Directed/throwbox/filter.il +++ b/src/tests/JIT/Directed/throwbox/filter.il @@ -14,7 +14,7 @@ .class public auto ansi beforefieldinit Test extends [mscorlib]System.Object { - .method public hidebysig static int32 Main() cil managed + .method public hidebysig static int32 Main() cil managed aggressiveinlining { .custom instance void [xunit.core]Xunit.FactAttribute::.ctor() = ( 01 00 00 00