Skip to content

Commit

Permalink
JIT: synthesize PGO if no schema, when dynamic PGO is active (#101739)
Browse files Browse the repository at this point in the history
If we know dynamic PGO is active, and we do not find a PGO schema for a method,
synthesize PGO data. The schema may be missing if the method was prejitted but
not covered by static PGO, or was considered too simple to need profiling (aka
minimal profiling).

This synthesis removes the possibility of a mixed PGO/no PGO situation. These
are problematic, especially in methods that do a lot of inlining. Now when
dynamic PGO is active all methods that get optimized will have some form of
PGO data.

Only run profile incorporation when optimizing. Reset BBOPT/pgo vars if we
switch away from optimization or have a min opts failover.

Contributes to #93020.
  • Loading branch information
AndyAyersMS authored May 3, 2024
1 parent a2635e1 commit af889b9
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 19 deletions.
12 changes: 12 additions & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4088,7 +4088,18 @@ void Compiler::compSetOptimizationLevel()
{
info.compCompHnd->setMethodAttribs(info.compMethodHnd, CORINFO_FLG_SWITCHED_TO_MIN_OPT);
opts.jitFlags->Clear(JitFlags::JIT_FLAG_TIER1);
opts.jitFlags->Clear(JitFlags::JIT_FLAG_BBOPT);
compSwitchedToMinOpts = true;

// We may have read PGO data. Clear it out because we won't be using it.
//
fgPgoFailReason = "method switched to min-opts";
fgPgoQueryResult = E_FAIL;
fgPgoHaveWeights = false;
fgPgoData = nullptr;
fgPgoSchema = nullptr;
fgPgoDisabled = true;
fgPgoDynamic = false;
}

#ifdef DEBUG
Expand Down Expand Up @@ -8056,6 +8067,7 @@ if (!inlineInfo &&
compileFlags->Set(JitFlags::JIT_FLAG_MIN_OPT);
compileFlags->Clear(JitFlags::JIT_FLAG_SIZE_OPT);
compileFlags->Clear(JitFlags::JIT_FLAG_SPEED_OPT);
compileFlags->Clear(JitFlags::JIT_FLAG_BBOPT);

goto START;
}
Expand Down
44 changes: 26 additions & 18 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,15 @@
// true if so
//
// Note:
// This now returns true for inlinees. We might consider preserving the
// old behavior for crossgen, since crossgen BBINSTRs still do inlining
// and don't instrument the inlinees.
// In most cases it is more appropriate to call fgHaveProfileWeights,
// since that tells you if blocks have profile-based weights.
//
// Thus if BBINSTR and BBOPT do the same inlines (which can happen)
// profile data for an inlinee (if available) will not fully reflect
// the behavior of the inlinee when called from this method.
// This method literally checks if the runtime had a profile schema,
// from which we can derive weights.
//
// If this inlinee was not inlined by the BBINSTR run then the
// profile data for the inlinee will reflect this method's influence.
//
// * for ALWAYS_INLINE and FORCE_INLINE cases it is unlikely we'll find
// any profile data, as BBINSTR and BBOPT callers will both inline;
// only indirect callers will invoke the instrumented version to run.
// * for DISCRETIONARY_INLINE cases we may or may not find relevant
// data, depending, but chances are the data is relevant.
//
// TieredPGO data comes from Tier0 methods, which currently do not do
// any inlining; thus inlinee profile data should be available and
// representative.
// Schema-based data comes from Tier0 methods, which currently do not do
// any inlining; thus inlinee profile data should be available and
// representative.
//
bool Compiler::fgHaveProfileData()
{
Expand All @@ -47,6 +36,9 @@ bool Compiler::fgHaveProfileData()
//------------------------------------------------------------------------
// fgHaveProfileWeights: Check if we have a profile that has weights.
//
// Notes:
// These weights may come from instrumentation or from synthesis.
//
bool Compiler::fgHaveProfileWeights()
{
return fgPgoHaveWeights;
Expand Down Expand Up @@ -2848,6 +2840,14 @@ PhaseStatus Compiler::fgIncorporateProfileData()
return PhaseStatus::MODIFIED_EVERYTHING;
}

// For now we only rely on profile data when optimizing.
//
if (!opts.OptimizationEnabled())
{
JITDUMP("not optimizing, so not incorporating any profile data\n");
return PhaseStatus::MODIFIED_NOTHING;
}

#ifdef DEBUG
// Optionally run synthesis
//
Expand Down Expand Up @@ -2887,6 +2887,14 @@ PhaseStatus Compiler::fgIncorporateProfileData()
JITDUMP("BBOPT not set\n");
}

// Is dynamic PGO active? If so, run synthesis.
//
if (fgPgoDynamic)
{
JITDUMP("Dynamic PGO active, synthesizing profile data\n");
ProfileSynthesis::Run(this, ProfileSynthesisOption::AssignLikelihoods);
}

// Scale the "synthetic" block weights.
//
fgApplyProfileScale();
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ void Compiler::optScaleLoopBlocks(BasicBlock* begBlk, BasicBlock* endBlk)
for (BasicBlock* const curBlk : BasicBlockRangeList(begBlk, endBlk))
{
// Don't change the block weight if it came from profile data.
if (curBlk->hasProfileWeight() && fgHaveProfileData())
if (curBlk->hasProfileWeight() && fgHaveProfileWeights())
{
reportBlockWeight(curBlk, "; unchanged: has profile weight");
continue;
Expand Down

0 comments on commit af889b9

Please sign in to comment.