Skip to content

Commit

Permalink
JIT: Support profiles with only histograms (#77721)
Browse files Browse the repository at this point in the history
Split up "we have a profile" and "we have profile weights" questions.
  • Loading branch information
jakobbotsch authored Nov 2, 2022
1 parent 7cb6fe6 commit 9a138bf
Show file tree
Hide file tree
Showing 12 changed files with 78 additions and 44 deletions.
18 changes: 16 additions & 2 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2712,6 +2712,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
fgPgoQueryResult = E_FAIL;
fgPgoFailReason = nullptr;
fgPgoSource = ICorJitInfo::PgoSource::Unknown;
fgPgoHaveWeights = false;

if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT))
{
Expand Down Expand Up @@ -2767,6 +2768,19 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
if (SUCCEEDED(fgPgoQueryResult))
{
assert(fgPgoSchema != nullptr);

for (UINT32 i = 0; i < fgPgoSchemaCount; i++)
{
ICorJitInfo::PgoInstrumentationKind kind = fgPgoSchema[i].InstrumentationKind;
if (kind == ICorJitInfo::PgoInstrumentationKind::BasicBlockIntCount ||
kind == ICorJitInfo::PgoInstrumentationKind::BasicBlockLongCount ||
kind == ICorJitInfo::PgoInstrumentationKind::EdgeIntCount ||
kind == ICorJitInfo::PgoInstrumentationKind::EdgeLongCount)
{
fgPgoHaveWeights = true;
break;
}
}
}

// A failed result implies a NULL fgPgoSchema
Expand Down Expand Up @@ -6038,7 +6052,7 @@ void Compiler::compCompileFinish()

printf("%08X | ", currentMethodToken);

if (fgHaveProfileData())
if (fgHaveProfileWeights())
{
if (fgCalledCount < 1000)
{
Expand Down Expand Up @@ -6491,7 +6505,7 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
InlineResult prejitResult(this, methodHnd, "prejit");

// Profile data allows us to avoid early "too many IL bytes" outs.
prejitResult.NoteBool(InlineObservation::CALLSITE_HAS_PROFILE, fgHaveSufficientProfileData());
prejitResult.NoteBool(InlineObservation::CALLSITE_HAS_PROFILE_WEIGHTS, fgHaveSufficientProfileWeights());

// Do the initial inline screen.
impCanInlineIL(methodHnd, methodInfo, forceInline, &prejitResult);
Expand Down
10 changes: 6 additions & 4 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5405,7 +5405,7 @@ class Compiler
void fgDebugCheckDispFlags(GenTree* tree, GenTreeFlags dispFlags, GenTreeDebugFlags debugFlags);
void fgDebugCheckFlagsHelper(GenTree* tree, GenTreeFlags actualFlags, GenTreeFlags expectedFlags);
void fgDebugCheckTryFinallyExits();
void fgDebugCheckProfileData();
void fgDebugCheckProfileWeights();
bool fgDebugCheckIncomingProfileData(BasicBlock* block);
bool fgDebugCheckOutgoingProfileData(BasicBlock* block);

Expand Down Expand Up @@ -5512,6 +5512,7 @@ class Compiler
}

bool fgHaveProfileData();
bool fgHaveProfileWeights();
bool fgGetProfileWeightForBasicBlock(IL_OFFSET offset, weight_t* weight);

Instrumentor* fgCountInstrumentor;
Expand Down Expand Up @@ -5539,18 +5540,19 @@ class Compiler
unsigned fgPgoInlineePgo;
unsigned fgPgoInlineeNoPgo;
unsigned fgPgoInlineeNoPgoSingleBlock;
bool fgPgoHaveWeights;

void WalkSpanningTree(SpanningTreeVisitor* visitor);
void fgSetProfileWeight(BasicBlock* block, weight_t weight);
void fgApplyProfileScale();
bool fgHaveSufficientProfileData();
bool fgHaveTrustedProfileData();
bool fgHaveSufficientProfileWeights();
bool fgHaveTrustedProfileWeights();

// fgIsUsingProfileWeights - returns true if we have real profile data for this method
// or if we have some fake profile data for the stress mode
bool fgIsUsingProfileWeights()
{
return (fgHaveProfileData() || fgStressBBProf());
return (fgHaveProfileWeights() || fgStressBBProf());
}

// fgProfileRunsCount - returns total number of scenario runs for the profile data
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
{
// Set default values for profile (to avoid NoteFailed in CALLEE_IL_CODE_SIZE's handler)
// these will be overridden later.
compInlineResult->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE, true);
compInlineResult->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE_WEIGHTS, true);
compInlineResult->NoteDouble(InlineObservation::CALLSITE_PROFILE_FREQUENCY, 1.0);
// Observe force inline state and code size.
compInlineResult->NoteBool(InlineObservation::CALLEE_IS_FORCE_INLINE, isForceInline);
Expand Down Expand Up @@ -4902,7 +4902,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)

// When adding a new jmpBlk we will set the bbWeight and bbFlags
//
if (fgHaveValidEdgeWeights && fgHaveProfileData())
if (fgHaveValidEdgeWeights && fgHaveProfileWeights())
{
noway_assert(fgComputePredsDone);

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos)
fprintf(fgxFile, "\n bytesOfIL=\"%d\"", info.compILCodeSize);
fprintf(fgxFile, "\n localVarCount=\"%d\"", lvaCount);

if (fgHaveProfileData())
if (fgHaveProfileWeights())
{
fprintf(fgxFile, "\n calledCount=\"%f\"", fgCalledCount);
fprintf(fgxFile, "\n profileData=\"true\"");
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1726,7 +1726,7 @@ PhaseStatus Compiler::fgPostImportationCleanup()
// We may have previously though this try entry was unreachable, but now we're going to
// step through it on the way to the OSR entry. So ensure it has plausible profile weight.
//
if (fgHaveProfileData() && !fromBlock->hasProfileWeight())
if (fgHaveProfileWeights() && !fromBlock->hasProfileWeight())
{
JITDUMP("Updating block weight for now-reachable try entry " FMT_BB " via " FMT_BB "\n",
fromBlock->bbNum, fgFirstBB->bbNum);
Expand Down
42 changes: 29 additions & 13 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,20 @@ bool Compiler::fgHaveProfileData()
}

//------------------------------------------------------------------------
// fgHaveSufficientProfileData: check if profile data is available
// fgHaveProfileWeights: Check if we have a profile that has weights.
//
bool Compiler::fgHaveProfileWeights()
{
if (!fgHaveProfileData())
{
return false;
}

return fgPgoHaveWeights;
}

//------------------------------------------------------------------------
// fgHaveSufficientProfileWeights: check if profile data is available
// and is sufficient enough to be trustful.
//
// Returns:
Expand All @@ -57,9 +70,9 @@ bool Compiler::fgHaveProfileData()
// Note:
// See notes for fgHaveProfileData.
//
bool Compiler::fgHaveSufficientProfileData()
bool Compiler::fgHaveSufficientProfileWeights()
{
if (!fgHaveProfileData())
if (!fgHaveProfileWeights())
{
return false;
}
Expand All @@ -73,7 +86,7 @@ bool Compiler::fgHaveSufficientProfileData()
}

//------------------------------------------------------------------------
// fgHaveTrustedProfileData: check if profile data source is one
// fgHaveTrustedProfileWeights: check if profile data source is one
// that can be trusted to faithfully represent the current program
// behavior.
//
Expand All @@ -83,9 +96,9 @@ bool Compiler::fgHaveSufficientProfileData()
// Note:
// See notes for fgHaveProfileData.
//
bool Compiler::fgHaveTrustedProfileData()
bool Compiler::fgHaveTrustedProfileWeights()
{
if (!fgHaveProfileData())
if (!fgHaveProfileWeights())
{
return false;
}
Expand Down Expand Up @@ -119,7 +132,7 @@ void Compiler::fgApplyProfileScale()

// Callee has profile data?
//
if (!fgHaveProfileData())
if (!fgHaveProfileWeights())
{
// No; we will carry on nonetheless.
//
Expand All @@ -140,7 +153,7 @@ void Compiler::fgApplyProfileScale()
//
if (calleeWeight == BB_ZERO_WEIGHT)
{
calleeWeight = fgHaveProfileData() ? 1.0 : BB_UNITY_WEIGHT;
calleeWeight = fgHaveProfileWeights() ? 1.0 : BB_UNITY_WEIGHT;
JITDUMP(" ... callee entry has weight zero, will use weight of " FMT_WT " to scale\n", calleeWeight);
}

Expand Down Expand Up @@ -243,7 +256,7 @@ bool Compiler::fgGetProfileWeightForBasicBlock(IL_OFFSET offset, weight_t* weigh
}
#endif // DEBUG

if (!fgHaveProfileData())
if (!fgHaveProfileWeights())
{
return false;
}
Expand Down Expand Up @@ -2209,9 +2222,12 @@ PhaseStatus Compiler::fgIncorporateProfileData()
const bool haveBlockCounts = fgPgoBlockCounts > 0;
const bool haveEdgeCounts = fgPgoEdgeCounts > 0;

// We expect one or the other but not both.
fgPgoHaveWeights = haveBlockCounts || haveEdgeCounts;

// We expect not to have both block and edge counts. We may have other
// forms of profile data even if we do not have any counts.
//
assert(haveBlockCounts != haveEdgeCounts);
assert(!haveBlockCounts || !haveEdgeCounts);

if (haveBlockCounts)
{
Expand Down Expand Up @@ -4174,7 +4190,7 @@ bool Compiler::fgProfileWeightsConsistent(weight_t weight1, weight_t weight2)
#ifdef DEBUG

//------------------------------------------------------------------------
// fgDebugCheckProfileData: verify profile data is self-consistent
// fgDebugCheckProfileWeights: verify profile weights are self-consistent
// (or nearly so)
//
// Notes:
Expand All @@ -4185,7 +4201,7 @@ bool Compiler::fgProfileWeightsConsistent(weight_t weight1, weight_t weight2)
// we expect EH edge counts to be small, so errors from ignoring
// them should be rare.
//
void Compiler::fgDebugCheckProfileData()
void Compiler::fgDebugCheckProfileWeights()
{
assert(fgComputePredsDone);

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13216,7 +13216,7 @@ void Compiler::impMakeDiscretionaryInlineObservations(InlineInfo* pInlineInfo, I

// If the call site has profile data, report the relative frequency of the site.
//
if ((pInlineInfo != nullptr) && rootCompiler->fgHaveSufficientProfileData())
if ((pInlineInfo != nullptr) && rootCompiler->fgHaveSufficientProfileWeights())
{
const weight_t callSiteWeight = pInlineInfo->iciBlock->bbWeight;
const weight_t entryWeight = rootCompiler->fgFirstBB->bbWeight;
Expand All @@ -13233,7 +13233,7 @@ void Compiler::impMakeDiscretionaryInlineObservations(InlineInfo* pInlineInfo, I
profileFreq = 1.0;
}

inlineResult->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE, hasProfile);
inlineResult->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE_WEIGHTS, hasProfile);
inlineResult->NoteDouble(InlineObservation::CALLSITE_PROFILE_FREQUENCY, profileFreq);
}

Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6636,7 +6636,8 @@ void Compiler::impCheckCanInline(GenTreeCall* call,

// Profile data allows us to avoid early "too many IL bytes" outs.
//
inlineResult->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE, compiler->fgHaveSufficientProfileData());
inlineResult->NoteBool(InlineObservation::CALLSITE_HAS_PROFILE_WEIGHTS,
compiler->fgHaveSufficientProfileWeights());

bool const forceInline = (pParam->methAttr & CORINFO_FLG_FORCEINLINE) != 0;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/inline.def
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ INLINE_OBSERVATION(DIV_BY_CNS, int, "dividy by const",
INLINE_OBSERVATION(CONSTANT_ARG_FEEDS_TEST, bool, "constant argument feeds test", INFORMATION, CALLSITE)
INLINE_OBSERVATION(DEPTH, int, "depth", INFORMATION, CALLSITE)
INLINE_OBSERVATION(FREQUENCY, int, "rough call site frequency", INFORMATION, CALLSITE)
INLINE_OBSERVATION(HAS_PROFILE, bool, "profile data is available", INFORMATION, CALLSITE)
INLINE_OBSERVATION(HAS_PROFILE_WEIGHTS, bool, "profile weights are available", INFORMATION, CALLSITE)
INLINE_OBSERVATION(IN_LOOP, bool, "call site is in a loop", INFORMATION, CALLSITE)
INLINE_OBSERVATION(IN_TRY_REGION, bool, "call site is in a try region", INFORMATION, CALLSITE)
INLINE_OBSERVATION(IN_NORETURN_REGION, bool, "call site is in a no-return region", INFORMATION, CALLSITE)
Expand Down
25 changes: 13 additions & 12 deletions src/coreclr/jit/inlinepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1313,8 +1313,8 @@ void ExtendedDefaultPolicy::NoteBool(InlineObservation obs, bool value)
m_DivByCns++;
break;

case InlineObservation::CALLSITE_HAS_PROFILE:
m_HasProfile = value;
case InlineObservation::CALLSITE_HAS_PROFILE_WEIGHTS:
m_HasProfileWeights = value;
break;

case InlineObservation::CALLSITE_IN_NORETURN_REGION:
Expand Down Expand Up @@ -1346,7 +1346,7 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
unsigned maxCodeSize = static_cast<unsigned>(JitConfig.JitExtDefaultPolicyMaxIL());

// TODO: Enable for PgoSource::Static as well if it's not the generic profile we bundle.
if (m_HasProfile && (m_RootCompiler->fgHaveTrustedProfileData()))
if (m_HasProfileWeights && (m_RootCompiler->fgHaveTrustedProfileWeights()))
{
maxCodeSize = static_cast<unsigned>(JitConfig.JitExtDefaultPolicyMaxILProf());
}
Expand Down Expand Up @@ -1379,7 +1379,8 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
{
SetNever(InlineObservation::CALLEE_DOES_NOT_RETURN);
}
else if (!m_IsForceInline && !m_HasProfile && !m_ConstArgFeedsIsKnownConst && !m_ArgFeedsIsKnownConst)
else if (!m_IsForceInline && !m_HasProfileWeights && !m_ConstArgFeedsIsKnownConst &&
!m_ArgFeedsIsKnownConst)
{
unsigned bbLimit = (unsigned)JitConfig.JitExtDefaultPolicyMaxBB();
if (m_IsPrejitRoot)
Expand Down Expand Up @@ -1681,7 +1682,7 @@ double ExtendedDefaultPolicy::DetermineMultiplier()
}
}

if (m_HasProfile)
if (m_HasProfileWeights)
{
// There are cases when Profile Data can be misleading or polluted:
// 1) We don't support context-sensitive instrumentation
Expand All @@ -1693,7 +1694,7 @@ double ExtendedDefaultPolicy::DetermineMultiplier()
const double profileTrustCoef = (double)JitConfig.JitExtDefaultPolicyProfTrust() / 10.0;
const double profileScale = (double)JitConfig.JitExtDefaultPolicyProfScale() / 10.0;

if (m_RootCompiler->fgHaveTrustedProfileData())
if (m_RootCompiler->fgHaveTrustedProfileWeights())
{
multiplier *= (1.0 - profileTrustCoef) + min(m_ProfileFrequency, 1.0) * profileScale;
}
Expand Down Expand Up @@ -1791,7 +1792,7 @@ void ExtendedDefaultPolicy::OnDumpXml(FILE* file, unsigned indent) const
XATTR_B(m_IsFromValueClass)
XATTR_B(m_NonGenericCallsGeneric)
XATTR_B(m_IsCallsiteInNoReturnRegion)
XATTR_B(m_HasProfile)
XATTR_B(m_HasProfileWeights)
}
#endif

Expand Down Expand Up @@ -1846,7 +1847,7 @@ DiscretionaryPolicy::DiscretionaryPolicy(Compiler* compiler, bool isPrejitRoot)
, m_CallSiteWeight(0)
, m_ModelCodeSizeEstimate(0)
, m_PerCallInstructionEstimate(0)
, m_HasProfile(false)
, m_HasProfileWeights(false)
, m_IsClassCtor(false)
, m_IsSameThis(false)
, m_CallerHasNewArray(false)
Expand Down Expand Up @@ -1893,8 +1894,8 @@ void DiscretionaryPolicy::NoteBool(InlineObservation obs, bool value)
// hotness for all candidates. So ignore.
break;

case InlineObservation::CALLSITE_HAS_PROFILE:
m_HasProfile = value;
case InlineObservation::CALLSITE_HAS_PROFILE_WEIGHTS:
m_HasProfileWeights = value;
break;

default:
Expand Down Expand Up @@ -2931,7 +2932,7 @@ void ProfilePolicy::NoteInt(InlineObservation obs, int value)
// If we're mimicking the default policy because there's no PGO
// data for this call, also fail if thereare too many basic blocks.
//
if (!m_HasProfile && !m_IsForceInline && (value > MAX_BASIC_BLOCKS))
if (!m_HasProfileWeights && !m_IsForceInline && (value > MAX_BASIC_BLOCKS))
{
SetNever(InlineObservation::CALLEE_TOO_MANY_BASIC_BLOCKS);
return;
Expand All @@ -2956,7 +2957,7 @@ void ProfilePolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
// We expect to have profile data, otherwise we should not
// have used this policy.
//
if (!m_HasProfile)
if (!m_HasProfileWeights)
{
// Todo: investigate these cases more carefully.
//
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/inlinepolicy.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class ExtendedDefaultPolicy : public DefaultPolicy
, m_IsFromValueClass(false)
, m_NonGenericCallsGeneric(false)
, m_IsCallsiteInNoReturnRegion(false)
, m_HasProfile(false)
, m_HasProfileWeights(false)
{
// Empty
}
Expand Down Expand Up @@ -265,7 +265,7 @@ class ExtendedDefaultPolicy : public DefaultPolicy
bool m_IsFromValueClass : 1;
bool m_NonGenericCallsGeneric : 1;
bool m_IsCallsiteInNoReturnRegion : 1;
bool m_HasProfile : 1;
bool m_HasProfileWeights : 1;
};

// DiscretionaryPolicy is a variant of the default policy. It
Expand Down Expand Up @@ -360,7 +360,7 @@ class DiscretionaryPolicy : public DefaultPolicy
unsigned m_CallSiteWeight;
int m_ModelCodeSizeEstimate;
int m_PerCallInstructionEstimate;
bool m_HasProfile;
bool m_HasProfileWeights;
bool m_IsClassCtor;
bool m_IsSameThis;
bool m_CallerHasNewArray;
Expand Down
Loading

0 comments on commit 9a138bf

Please sign in to comment.