Skip to content

Commit

Permalink
JIT: fix recursive inline check (#88749)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
AndyAyersMS authored Jul 12, 2023
1 parent a23c7a1 commit ead96d6
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 20 deletions.
8 changes: 5 additions & 3 deletions src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/jit/inline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand All @@ -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
Expand Down
41 changes: 24 additions & 17 deletions src/coreclr/jit/inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit ead96d6

Please sign in to comment.