Skip to content

Commit a5d4518

Browse files
AndyAyersMSmichaelgsharp
authored andcommitted
JIT: synthesize PGO if no schema, when dynamic PGO is active (dotnet#101739)
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 dotnet#93020.
1 parent 12312c2 commit a5d4518

File tree

3 files changed

+39
-19
lines changed

3 files changed

+39
-19
lines changed

src/coreclr/jit/compiler.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4088,7 +4088,18 @@ void Compiler::compSetOptimizationLevel()
40884088
{
40894089
info.compCompHnd->setMethodAttribs(info.compMethodHnd, CORINFO_FLG_SWITCHED_TO_MIN_OPT);
40904090
opts.jitFlags->Clear(JitFlags::JIT_FLAG_TIER1);
4091+
opts.jitFlags->Clear(JitFlags::JIT_FLAG_BBOPT);
40914092
compSwitchedToMinOpts = true;
4093+
4094+
// We may have read PGO data. Clear it out because we won't be using it.
4095+
//
4096+
fgPgoFailReason = "method switched to min-opts";
4097+
fgPgoQueryResult = E_FAIL;
4098+
fgPgoHaveWeights = false;
4099+
fgPgoData = nullptr;
4100+
fgPgoSchema = nullptr;
4101+
fgPgoDisabled = true;
4102+
fgPgoDynamic = false;
40924103
}
40934104

40944105
#ifdef DEBUG
@@ -8056,6 +8067,7 @@ if (!inlineInfo &&
80568067
compileFlags->Set(JitFlags::JIT_FLAG_MIN_OPT);
80578068
compileFlags->Clear(JitFlags::JIT_FLAG_SIZE_OPT);
80588069
compileFlags->Clear(JitFlags::JIT_FLAG_SPEED_OPT);
8070+
compileFlags->Clear(JitFlags::JIT_FLAG_BBOPT);
80598071

80608072
goto START;
80618073
}

src/coreclr/jit/fgprofile.cpp

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,15 @@
1818
// true if so
1919
//
2020
// Note:
21-
// This now returns true for inlinees. We might consider preserving the
22-
// old behavior for crossgen, since crossgen BBINSTRs still do inlining
23-
// and don't instrument the inlinees.
21+
// In most cases it is more appropriate to call fgHaveProfileWeights,
22+
// since that tells you if blocks have profile-based weights.
2423
//
25-
// Thus if BBINSTR and BBOPT do the same inlines (which can happen)
26-
// profile data for an inlinee (if available) will not fully reflect
27-
// the behavior of the inlinee when called from this method.
24+
// This method literally checks if the runtime had a profile schema,
25+
// from which we can derive weights.
2826
//
29-
// If this inlinee was not inlined by the BBINSTR run then the
30-
// profile data for the inlinee will reflect this method's influence.
31-
//
32-
// * for ALWAYS_INLINE and FORCE_INLINE cases it is unlikely we'll find
33-
// any profile data, as BBINSTR and BBOPT callers will both inline;
34-
// only indirect callers will invoke the instrumented version to run.
35-
// * for DISCRETIONARY_INLINE cases we may or may not find relevant
36-
// data, depending, but chances are the data is relevant.
37-
//
38-
// TieredPGO data comes from Tier0 methods, which currently do not do
39-
// any inlining; thus inlinee profile data should be available and
40-
// representative.
27+
// Schema-based data comes from Tier0 methods, which currently do not do
28+
// any inlining; thus inlinee profile data should be available and
29+
// representative.
4130
//
4231
bool Compiler::fgHaveProfileData()
4332
{
@@ -47,6 +36,9 @@ bool Compiler::fgHaveProfileData()
4736
//------------------------------------------------------------------------
4837
// fgHaveProfileWeights: Check if we have a profile that has weights.
4938
//
39+
// Notes:
40+
// These weights may come from instrumentation or from synthesis.
41+
//
5042
bool Compiler::fgHaveProfileWeights()
5143
{
5244
return fgPgoHaveWeights;
@@ -2848,6 +2840,14 @@ PhaseStatus Compiler::fgIncorporateProfileData()
28482840
return PhaseStatus::MODIFIED_EVERYTHING;
28492841
}
28502842

2843+
// For now we only rely on profile data when optimizing.
2844+
//
2845+
if (!opts.OptimizationEnabled())
2846+
{
2847+
JITDUMP("not optimizing, so not incorporating any profile data\n");
2848+
return PhaseStatus::MODIFIED_NOTHING;
2849+
}
2850+
28512851
#ifdef DEBUG
28522852
// Optionally run synthesis
28532853
//
@@ -2887,6 +2887,14 @@ PhaseStatus Compiler::fgIncorporateProfileData()
28872887
JITDUMP("BBOPT not set\n");
28882888
}
28892889

2890+
// Is dynamic PGO active? If so, run synthesis.
2891+
//
2892+
if (fgPgoDynamic)
2893+
{
2894+
JITDUMP("Dynamic PGO active, synthesizing profile data\n");
2895+
ProfileSynthesis::Run(this, ProfileSynthesisOption::AssignLikelihoods);
2896+
}
2897+
28902898
// Scale the "synthetic" block weights.
28912899
//
28922900
fgApplyProfileScale();

src/coreclr/jit/optimizer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ void Compiler::optScaleLoopBlocks(BasicBlock* begBlk, BasicBlock* endBlk)
234234
for (BasicBlock* const curBlk : BasicBlockRangeList(begBlk, endBlk))
235235
{
236236
// Don't change the block weight if it came from profile data.
237-
if (curBlk->hasProfileWeight() && fgHaveProfileData())
237+
if (curBlk->hasProfileWeight() && fgHaveProfileWeights())
238238
{
239239
reportBlockWeight(curBlk, "; unchanged: has profile weight");
240240
continue;

0 commit comments

Comments
 (0)