Skip to content

Commit

Permalink
JIT: minor inliner refactoring (#44215)
Browse files Browse the repository at this point in the history 
Extract out the budget check logic so it can vary by inlining policy.
Use this to exempt the FullPolicy from budget checking.

Fix inline xml to dump the proper (full name) hash for inlinees.

Update range dumper to dump ranges in hex.
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Nov 5, 2020
1 parent 82e5039 commit 8ed2e95
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 44 deletions.
42 changes: 42 additions & 0 deletions src/coreclr/src/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5508,6 +5508,12 @@ int Compiler::compCompile(CORINFO_MODULE_HANDLE classPtr,
}

#if defined(DEBUG) || defined(INLINE_DATA)
//------------------------------------------------------------------------
// compMethodHash: get hash code for currently jitted method
//
// Returns:
// Hash based on method's full name
//
unsigned Compiler::Info::compMethodHash() const
{
if (compMethodHashPrivate == 0)
Expand All @@ -5521,6 +5527,42 @@ unsigned Compiler::Info::compMethodHash() const
}
return compMethodHashPrivate;
}

//------------------------------------------------------------------------
// compMethodHash: get hash code for specified method
//
// Arguments:
// methodHnd - method of interest
//
// Returns:
// Hash based on method's full name
//
unsigned Compiler::compMethodHash(CORINFO_METHOD_HANDLE methodHnd)
{
// If this is the root method, delegate to the caching version
//
if (methodHnd == info.compMethodHnd)
{
return info.compMethodHash();
}

// Else compute from scratch. Might consider caching this too.
//
unsigned methodHash = 0;
const char* calleeName = eeGetMethodFullName(methodHnd);

if (calleeName != nullptr)
{
methodHash = HashStringA(calleeName);
}
else
{
methodHash = info.compCompHnd->getMethodHash(methodHnd);
}

return methodHash;
}

#endif // defined(DEBUG) || defined(INLINE_DATA)

void Compiler::compCompileFinish()
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7370,6 +7370,7 @@ class Compiler

const char* eeGetMethodName(CORINFO_METHOD_HANDLE hnd, const char** className);
const char* eeGetMethodFullName(CORINFO_METHOD_HANDLE hnd);
unsigned compMethodHash(CORINFO_METHOD_HANDLE methodHandle);

bool eeIsNativeMethod(CORINFO_METHOD_HANDLE method);
CORINFO_METHOD_HANDLE eeGetMethodHandleForNative(CORINFO_METHOD_HANDLE method);
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/src/jit/inline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,8 @@ void InlineContext::DumpXml(FILE* file, unsigned indent)
{
Compiler* compiler = m_InlineStrategy->GetCompiler();

mdMethodDef calleeToken = compiler->info.compCompHnd->getMethodDefFromMethod(m_Callee);
unsigned calleeHash = compiler->info.compCompHnd->getMethodHash(m_Callee);

mdMethodDef calleeToken = compiler->info.compCompHnd->getMethodDefFromMethod(m_Callee);
unsigned calleeHash = compiler->compMethodHash(m_Callee);
const char* inlineReason = InlGetObservationString(m_Observation);

int offset = -1;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/src/jit/inline.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ class InlinePolicy

// Policy determinations
virtual void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) = 0;
virtual bool BudgetCheck() const = 0;

// Policy policies
virtual bool PropagateNeverToRuntime() const = 0;
Expand Down
108 changes: 71 additions & 37 deletions src/coreclr/src/jit/inlinepolicy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,35 +369,13 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
// "reestablishes" candidacy rather than alters
// candidacy ... so instead we bail out here.
//
if (!m_IsPrejitRoot)
{
InlineStrategy* strategy = m_RootCompiler->m_inlineStrategy;
const bool overBudget = strategy->BudgetCheck(m_CodeSize);
bool overBudget = this->BudgetCheck();

if (overBudget)
{
// If the candidate is a forceinline and the callsite is
// not too deep, allow the inline even if it goes over budget.
//
// For now, "not too deep" means a top-level inline. Note
// depth 0 is used for the root method, so inline candidate depth
// will be 1 or more.
//
assert(m_IsForceInlineKnown);
assert(m_CallsiteDepth > 0);
const bool allowOverBudget = m_IsForceInline && (m_CallsiteDepth == 1);

if (allowOverBudget)
{
JITDUMP("Allowing over-budget top-level forceinline\n");
}
else
{
SetFailure(InlineObservation::CALLSITE_OVER_BUDGET);
}
}
if (overBudget)
{
SetFailure(InlineObservation::CALLSITE_OVER_BUDGET);
return;
}

break;
}

Expand Down Expand Up @@ -461,6 +439,53 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
}
}

//------------------------------------------------------------------------
// BudgetCheck: see if this inline would exceed the current budget
//
// Returns:
// True if inline would exceed the budget.
//
bool DefaultPolicy::BudgetCheck() const
{
// Only relevant if we're actually inlining.
//
if (m_IsPrejitRoot)
{
return false;
}

// The strategy tracks the amout of inlining done so far,
// so it performs the actual check.
//
InlineStrategy* strategy = m_RootCompiler->m_inlineStrategy;
const bool overBudget = strategy->BudgetCheck(m_CodeSize);

if (overBudget)
{
// If the candidate is a forceinline and the callsite is
// not too deep, allow the inline even if it goes over budget.
//
// For now, "not too deep" means a top-level inline. Note
// depth 0 is used for the root method, so inline candidate depth
// will be 1 or more.
//
assert(m_IsForceInlineKnown);
assert(m_CallsiteDepth > 0);
const bool allowOverBudget = m_IsForceInline && (m_CallsiteDepth == 1);

if (allowOverBudget)
{
JITDUMP("Allowing over-budget top-level forceinline\n");
}
else
{
return true;
}
}

return false;
}

//------------------------------------------------------------------------
// NoteInt: handle an observed integer value
//
Expand Down Expand Up @@ -1012,15 +1037,11 @@ void RandomPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
assert(m_Observation == InlineObservation::CALLEE_IS_DISCRETIONARY_INLINE);

// Budget check.
if (!m_IsPrejitRoot)
const bool overBudget = this->BudgetCheck();
if (overBudget)
{
InlineStrategy* strategy = m_RootCompiler->m_inlineStrategy;
bool overBudget = strategy->BudgetCheck(m_CodeSize);
if (overBudget)
{
SetFailure(InlineObservation::CALLSITE_OVER_BUDGET);
return;
}
SetFailure(InlineObservation::CALLSITE_OVER_BUDGET);
return;
}

// If we're also dumping inline data, make additional observations
Expand Down Expand Up @@ -2180,6 +2201,19 @@ FullPolicy::FullPolicy(Compiler* compiler, bool isPrejitRoot) : DiscretionaryPol
// Empty
}

//------------------------------------------------------------------------
// BudgetCheck: see if this inline would exceed the current budget
//
// Returns:
// True if inline would exceed the budget.
//
bool FullPolicy::BudgetCheck() const
{
// There are no budget restrictions for the full policy.
//
return false;
}

//------------------------------------------------------------------------
// DetermineProfitability: determine if this inline is profitable
//
Expand Down Expand Up @@ -2482,7 +2516,7 @@ bool ReplayPolicy::FindContext(InlineContext* context)
//
// Token and Hash we're looking for.
mdMethodDef contextToken = m_RootCompiler->info.compCompHnd->getMethodDefFromMethod(context->GetCallee());
unsigned contextHash = m_RootCompiler->info.compCompHnd->getMethodHash(context->GetCallee());
unsigned contextHash = m_RootCompiler->compMethodHash(context->GetCallee());
unsigned contextOffset = (unsigned)context->GetOffset();

return FindInline(contextToken, contextHash, contextOffset);
Expand Down Expand Up @@ -2666,7 +2700,7 @@ bool ReplayPolicy::FindInline(CORINFO_METHOD_HANDLE callee)
{
// Token and Hash we're looking for
mdMethodDef calleeToken = m_RootCompiler->info.compCompHnd->getMethodDefFromMethod(callee);
unsigned calleeHash = m_RootCompiler->info.compCompHnd->getMethodHash(callee);
unsigned calleeHash = m_RootCompiler->compMethodHash(callee);

// Abstract this or just pass through raw bits
// See matching code in xml writer
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/jit/inlinepolicy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class DefaultPolicy : public LegalPolicy

// Policy determinations
void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) override;
bool BudgetCheck() const override;

// Policy policies
bool PropagateNeverToRuntime() const override;
Expand Down Expand Up @@ -350,6 +351,7 @@ class FullPolicy : public DiscretionaryPolicy

// Policy determinations
void DetermineProfitability(CORINFO_METHOD_INFO* methodInfo) override;
bool BudgetCheck() const override;

// Miscellaneous
const char* GetName() const override
Expand Down
12 changes: 8 additions & 4 deletions src/coreclr/src/jit/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,9 +846,6 @@ void ConfigMethodRange::InitRanges(const WCHAR* rangeStr, unsigned capacity)
//------------------------------------------------------------------------
// Dump: dump hash ranges to stdout
//
// Arguments:
// hash -- hash value to check

void ConfigMethodRange::Dump()
{
if (m_inited != 1)
Expand All @@ -866,7 +863,14 @@ void ConfigMethodRange::Dump()
printf("<method range with %d entries>\n", m_lastRange);
for (unsigned i = 0; i < m_lastRange; i++)
{
printf("%i [%u-%u]\n", i, m_ranges[i].m_low, m_ranges[i].m_high);
if (m_ranges[i].m_low == m_ranges[i].m_high)
{
printf("%i [0x%08x]\n", i, m_ranges[i].m_low);
}
else
{
printf("%i [0x%08x-0x%08x]\n", i, m_ranges[i].m_low, m_ranges[i].m_high);
}
}
}

Expand Down

0 comments on commit 8ed2e95

Please sign in to comment.