From f59b84ff797d94042693a7c5b04b064ac7a51b75 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 12 Jul 2023 09:50:36 -0700 Subject: [PATCH] JIT: fix recursive inline check The existing check was too conservative, and blocked inlines of one instantation of a generic method into a different instantiation of the same method, or of two different methods that share the exact same IL stream. Generalize the check to also compare the method handle and runtime context. Fixes #88667 Fixes #58824 --- src/coreclr/jit/fginline.cpp | 8 ++++--- src/coreclr/jit/inline.cpp | 6 ++++++ src/coreclr/jit/inline.h | 41 +++++++++++++++++++++--------------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 84eeb4981a4c46..82c3a3328b93f2 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -42,13 +42,15 @@ unsigned Compiler::fgCheckInlineDepthAndRecursion(InlineInfo* inlineInfo) assert(inlineContext->GetCode() != nullptr); depth++; - if (inlineContext->GetCode() == candidateCode) + if ((inlineContext->GetCode() == candidateCode) && (inlineContext->GetCallee() == inlineInfo->fncHandle) && + (inlineContext->GetRuntimeContext() == inlineInfo->inlineCandidateInfo->exactContextHnd)) { - // This inline candidate has the same IL code buffer as an already - // inlined method does. + // This is a recursive inline + // inlineResult->NoteFatal(InlineObservation::CALLSITE_IS_RECURSIVE); // No need to note CALLSITE_DEPTH we're already rejecting this candidate + // return depth; } diff --git a/src/coreclr/jit/inline.cpp b/src/coreclr/jit/inline.cpp index 57f8fe1a7d1ce5..bae5755707594d 100644 --- a/src/coreclr/jit/inline.cpp +++ b/src/coreclr/jit/inline.cpp @@ -331,6 +331,7 @@ InlineContext::InlineContext(InlineStrategy* strategy) , m_Sibling(nullptr) , m_Code(nullptr) , m_Callee(nullptr) + , m_RuntimeContext(nullptr) , m_ILSize(0) , m_ImportedILSize(0) , m_ActualCallOffset(BAD_IL_OFFSET) @@ -1263,6 +1264,10 @@ InlineContext* InlineStrategy::NewRoot() rootContext->m_Code = m_Compiler->info.compCode; rootContext->m_Callee = m_Compiler->info.compMethodHnd; + // May fail to block recursion for normal methods + // Might need the actual context handle here + rootContext->m_RuntimeContext = METHOD_BEING_COMPILED_CONTEXT(); + return rootContext; } @@ -1286,6 +1291,7 @@ InlineContext* InlineStrategy::NewContext(InlineContext* parentContext, Statemen context->m_Code = info->methInfo.ILCode; context->m_ILSize = info->methInfo.ILCodeSize; context->m_ActualCallOffset = info->ilOffset; + context->m_RuntimeContext = info->exactContextHnd; #ifdef DEBUG // All inline candidates should get their own statements that have diff --git a/src/coreclr/jit/inline.h b/src/coreclr/jit/inline.h index e350393cfd29c0..b0e28478b52422 100644 --- a/src/coreclr/jit/inline.h +++ b/src/coreclr/jit/inline.h @@ -754,6 +754,12 @@ class InlineContext return m_Callee; } + // Get the callee's exact context handle + CORINFO_CONTEXT_HANDLE GetRuntimeContext() const + { + return m_RuntimeContext; + } + unsigned GetOrdinal() const { return m_Ordinal; @@ -857,23 +863,24 @@ class InlineContext private: InlineContext(InlineStrategy* strategy); - InlineStrategy* m_InlineStrategy; // overall strategy - InlineContext* m_Parent; // logical caller (parent) - InlineContext* m_Child; // first child - InlineContext* m_Sibling; // next child of the parent - const BYTE* m_Code; // address of IL buffer for the method - CORINFO_METHOD_HANDLE m_Callee; // handle to the method - unsigned m_ILSize; // size of IL buffer for the method - unsigned m_ImportedILSize; // estimated size of imported IL - ILLocation m_Location; // inlining statement location within parent - IL_OFFSET m_ActualCallOffset; // IL offset of actual call instruction leading to the inline - InlineObservation m_Observation; // what lead to this inline success or failure - int m_CodeSizeEstimate; // in bytes * 10 - unsigned m_Ordinal; // Ordinal number of this inline - bool m_Success : 1; // true if this was a successful inline - bool m_Devirtualized : 1; // true if this was a devirtualized call - bool m_Guarded : 1; // true if this was a guarded call - bool m_Unboxed : 1; // true if this call now invokes the unboxed entry + InlineStrategy* m_InlineStrategy; // overall strategy + InlineContext* m_Parent; // logical caller (parent) + InlineContext* m_Child; // first child + InlineContext* m_Sibling; // next child of the parent + const BYTE* m_Code; // address of IL buffer for the method + CORINFO_METHOD_HANDLE m_Callee; // handle to the method + CORINFO_CONTEXT_HANDLE m_RuntimeContext; // handle to the exact context + unsigned m_ILSize; // size of IL buffer for the method + unsigned m_ImportedILSize; // estimated size of imported IL + ILLocation m_Location; // inlining statement location within parent + IL_OFFSET m_ActualCallOffset; // IL offset of actual call instruction leading to the inline + InlineObservation m_Observation; // what lead to this inline success or failure + int m_CodeSizeEstimate; // in bytes * 10 + unsigned m_Ordinal; // Ordinal number of this inline + bool m_Success : 1; // true if this was a successful inline + bool m_Devirtualized : 1; // true if this was a devirtualized call + bool m_Guarded : 1; // true if this was a guarded call + bool m_Unboxed : 1; // true if this call now invokes the unboxed entry #if defined(DEBUG) || defined(INLINE_DATA)