Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

JIT: make suitably optimistic prejit inline assessments #14850

Merged
merged 1 commit into from
Nov 8, 2017
Merged

JIT: make suitably optimistic prejit inline assessments #14850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6074,6 +6074,8 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
prejitResult.DetermineProfitability(methodInfo);
}

m_inlineStrategy->NotePrejitDecision(prejitResult);

// Handle the results of the inline analysis.
if (prejitResult.IsFailure())
{
Expand Down
4 changes: 4 additions & 0 deletions src/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5107,6 +5107,8 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo
{
if (FgStack::IsArgument(slot0))
{
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_TEST);

unsigned varNum = FgStack::SlotTypeToArgNum(slot0);
if (impInlineInfo->inlArgInfo[varNum].argNode->OperIsConst())
{
Expand All @@ -5116,6 +5118,8 @@ void Compiler::fgObserveInlineConstants(OPCODE opcode, const FgStack& stack, boo

if (FgStack::IsArgument(slot1))
{
compInlineResult->Note(InlineObservation::CALLEE_ARG_FEEDS_TEST);

unsigned varNum = FgStack::SlotTypeToArgNum(slot1);
if (impInlineInfo->inlArgInfo[varNum].argNode->OperIsConst())
{
Expand Down
19 changes: 18 additions & 1 deletion src/jit/inline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,12 @@ void InlineContext::DumpXml(FILE* file, unsigned indent)
m_Sibling->DumpXml(file, indent);
}

// Optionally suppress failing inline records
if ((JitConfig.JitInlineDumpXml() == 3) && !m_Success)
{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add an enum for the three values used by JitInlineDumpXml()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a bunch of enhancements I have in mind for the inline XML, though as far as I know nobody but me has ever used it.... also I have some analysis tooling lying about that I should publish somewhere, maybe over in jitutils.

I should really change this into a flags-style thing.... you should be able to choose:

  • get XML for all methods or only those with inlines or inline attempts
  • get XML for all inline attempts or only those with successful inlines
  • get XML with full details or just summary details
  • specify filename to contain the results (currently always goes to stderr)

So maybe this is best done as a follow-up change.

return;
}

const bool isRoot = m_Parent == nullptr;
const bool hasChild = m_Child != nullptr;
const char* inlineType = m_Success ? "Inline" : "FailedInline";
Expand Down Expand Up @@ -739,6 +745,8 @@ InlineStrategy::InlineStrategy(Compiler* compiler)
: m_Compiler(compiler)
, m_RootContext(nullptr)
, m_LastSuccessfulPolicy(nullptr)
, m_LastContext(nullptr)
, m_PrejitRootDecision(InlineDecision::UNDECIDED)
, m_CallCount(0)
, m_CandidateCount(0)
, m_AlwaysCandidateCount(0)
Expand Down Expand Up @@ -1493,7 +1501,7 @@ void InlineStrategy::DumpXml(FILE* file, unsigned indent)
// If we're dumping "minimal" Xml, and we didn't do
// any inlines into this method, then there's nothing
// to emit here.
if ((m_InlineCount == 0) && (JitConfig.JitInlineDumpXml() == 2))
if ((m_InlineCount == 0) && (JitConfig.JitInlineDumpXml() >= 2))
{
return;
}
Expand Down Expand Up @@ -1566,6 +1574,15 @@ void InlineStrategy::DumpXml(FILE* file, unsigned indent)
fprintf(file, "%*s<SizeEstimate>%u</SizeEstimate>\n", indent + 2, "", m_CurrentSizeEstimate / 10);
fprintf(file, "%*s<TimeEstimate>%u</TimeEstimate>\n", indent + 2, "", m_CurrentTimeEstimate);

// For prejit roots also propagate out the assessment of the root method
if (isPrejitRoot)
{
fprintf(file, "%*s<PrejitDecision>%s</PrejitDecision>\n", indent + 2, "",
InlGetDecisionString(m_PrejitRootDecision));
fprintf(file, "%*s<PrejitObservation>%s</PrejitObservation>\n", indent + 2, "",
InlGetObservationString(m_PrejitRootObservation));
}

// Root context will be null if we're not optimizing the method.
//
// Note there are cases of this in mscorlib even in release builds,
Expand Down
1 change: 1 addition & 0 deletions src/jit/inline.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ INLINE_OBSERVATION(TOO_MUCH_IL, bool, "too many il bytes",
// ------ Callee Information -------

INLINE_OBSERVATION(ARG_FEEDS_CONSTANT_TEST, bool, "argument feeds constant test", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_TEST, bool, "argument feeds test", INFORMATION, CALLEE)
INLINE_OBSERVATION(ARG_FEEDS_RANGE_CHECK, bool, "argument feeds range check", INFORMATION, CALLEE)
INLINE_OBSERVATION(BEGIN_OPCODE_SCAN, bool, "prepare to look at opcodes", INFORMATION, CALLEE)
INLINE_OBSERVATION(BELOW_ALWAYS_INLINE_SIZE, bool, "below ALWAYS_INLINE size", INFORMATION, CALLEE)
Expand Down
51 changes: 30 additions & 21 deletions src/jit/inline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,13 @@ class InlineStrategy
m_ImportCount++;
}

// Inform strategy about the inline decision for a prejit root
void NotePrejitDecision(const InlineResult& r)
{
m_PrejitRootDecision = r.GetPolicy()->GetDecision();
m_PrejitRootObservation = r.GetPolicy()->GetObservation();
}

// Dump csv header for inline stats to indicated file.
static void DumpCsvHeader(FILE* f);

Expand Down Expand Up @@ -866,27 +873,29 @@ class InlineStrategy
static CritSecObject s_XmlWriterLock;
#endif // defined(DEBUG) || defined(INLINE_DATA)

Compiler* m_Compiler;
InlineContext* m_RootContext;
InlinePolicy* m_LastSuccessfulPolicy;
InlineContext* m_LastContext;
unsigned m_CallCount;
unsigned m_CandidateCount;
unsigned m_AlwaysCandidateCount;
unsigned m_ForceCandidateCount;
unsigned m_DiscretionaryCandidateCount;
unsigned m_UnprofitableCandidateCount;
unsigned m_ImportCount;
unsigned m_InlineCount;
unsigned m_MaxInlineSize;
unsigned m_MaxInlineDepth;
int m_InitialTimeBudget;
int m_InitialTimeEstimate;
int m_CurrentTimeBudget;
int m_CurrentTimeEstimate;
int m_InitialSizeEstimate;
int m_CurrentSizeEstimate;
bool m_HasForceViaDiscretionary;
Compiler* m_Compiler;
InlineContext* m_RootContext;
InlinePolicy* m_LastSuccessfulPolicy;
InlineContext* m_LastContext;
InlineDecision m_PrejitRootDecision;
InlineObservation m_PrejitRootObservation;
unsigned m_CallCount;
unsigned m_CandidateCount;
unsigned m_AlwaysCandidateCount;
unsigned m_ForceCandidateCount;
unsigned m_DiscretionaryCandidateCount;
unsigned m_UnprofitableCandidateCount;
unsigned m_ImportCount;
unsigned m_InlineCount;
unsigned m_MaxInlineSize;
unsigned m_MaxInlineDepth;
int m_InitialTimeBudget;
int m_InitialTimeEstimate;
int m_CurrentTimeBudget;
int m_CurrentTimeEstimate;
int m_InitialSizeEstimate;
int m_CurrentSizeEstimate;
bool m_HasForceViaDiscretionary;

#if defined(DEBUG) || defined(INLINE_DATA)
long m_MethodXmlFilePosition;
Expand Down
55 changes: 15 additions & 40 deletions src/jit/inlinepolicy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,37 +267,24 @@ void DefaultPolicy::NoteBool(InlineObservation obs, bool value)
break;

case InlineObservation::CALLEE_LOOKS_LIKE_WRAPPER:
// DefaultPolicy ignores this for prejit roots.
if (!m_IsPrejitRoot)
{
m_LooksLikeWrapperMethod = value;
}
m_LooksLikeWrapperMethod = value;
break;

case InlineObservation::CALLEE_ARG_FEEDS_TEST:
m_ArgFeedsTest++;
break;

case InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST:
// DefaultPolicy ignores this for prejit roots.
if (!m_IsPrejitRoot)
{
m_ArgFeedsConstantTest++;
}
m_ArgFeedsConstantTest++;
break;

case InlineObservation::CALLEE_ARG_FEEDS_RANGE_CHECK:
// DefaultPolicy ignores this for prejit roots.
if (!m_IsPrejitRoot)
{
m_ArgFeedsRangeCheck++;
}
m_ArgFeedsRangeCheck++;
break;

case InlineObservation::CALLEE_HAS_SWITCH:
case InlineObservation::CALLEE_UNSUPPORTED_OPCODE:
// DefaultPolicy ignores these for prejit roots.
if (!m_IsPrejitRoot)
{
// Pass these on, they should cause inlining to fail.
propagate = true;
}
propagate = true;
break;

case InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST:
Expand Down Expand Up @@ -649,6 +636,13 @@ double DefaultPolicy::DetermineMultiplier()
multiplier += 3.0;
JITDUMP("\nInline candidate has const arg that feeds a conditional. Multiplier increased to %g.", multiplier);
}
// For prejit roots we do not see the call sites. To be suitably optimisitic
// assume that call sites may pass constants.
else if (m_IsPrejitRoot && ((m_ArgFeedsConstantTest > 0) || (m_ArgFeedsTest > 0)))
{
multiplier += 3.0;
JITDUMP("\nPrejit root candidate has arg that feeds a conditional. Multiplier increased to %g.", multiplier);
}

switch (m_CallsiteFrequency)
{
Expand Down Expand Up @@ -1158,25 +1152,6 @@ void DiscretionaryPolicy::NoteBool(InlineObservation obs, bool value)
{
switch (obs)
{
case InlineObservation::CALLEE_LOOKS_LIKE_WRAPPER:
m_LooksLikeWrapperMethod = value;
break;

case InlineObservation::CALLEE_ARG_FEEDS_CONSTANT_TEST:
assert(value);
m_ArgFeedsConstantTest++;
break;

case InlineObservation::CALLEE_ARG_FEEDS_RANGE_CHECK:
assert(value);
m_ArgFeedsRangeCheck++;
break;

case InlineObservation::CALLSITE_CONSTANT_ARG_FEEDS_TEST:
assert(value);
m_ConstantArgFeedsConstantTest++;
break;

case InlineObservation::CALLEE_IS_CLASS_CTOR:
m_IsClassCtor = value;
break;
Expand Down
2 changes: 2 additions & 0 deletions src/jit/inlinepolicy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class DefaultPolicy : public LegalPolicy
, m_CallsiteFrequency(InlineCallsiteFrequency::UNUSED)
, m_InstructionCount(0)
, m_LoadStoreCount(0)
, m_ArgFeedsTest(0)
, m_ArgFeedsConstantTest(0)
, m_ArgFeedsRangeCheck(0)
, m_ConstantArgFeedsConstantTest(0)
Expand Down Expand Up @@ -148,6 +149,7 @@ class DefaultPolicy : public LegalPolicy
InlineCallsiteFrequency m_CallsiteFrequency;
unsigned m_InstructionCount;
unsigned m_LoadStoreCount;
unsigned m_ArgFeedsTest;
unsigned m_ArgFeedsConstantTest;
unsigned m_ArgFeedsRangeCheck;
unsigned m_ConstantArgFeedsConstantTest;
Expand Down
5 changes: 3 additions & 2 deletions src/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,9 @@ CONFIG_STRING(JitMeasureNowayAssertFile,

#if defined(DEBUG) || defined(INLINE_DATA)
CONFIG_INTEGER(JitInlineDumpData, W("JitInlineDumpData"), 0)
CONFIG_INTEGER(JitInlineDumpXml, W("JitInlineDumpXml"), 0) // 1 = full xml (all methods), 2 = minimal xml (only method
// with inlines)
CONFIG_INTEGER(JitInlineDumpXml, W("JitInlineDumpXml"), 0) // 1 = full xml (+ failures in DEBUG)
// 2 = only methods with inlines (+ failures in DEBUG)
// 3 = only methods with inlines, no failures
CONFIG_INTEGER(JitInlineLimit, W("JitInlineLimit"), -1)
CONFIG_INTEGER(JitInlinePolicyDiscretionary, W("JitInlinePolicyDiscretionary"), 0)
CONFIG_INTEGER(JitInlinePolicyFull, W("JitInlinePolicyFull"), 0)
Expand Down