Skip to content

Commit

Permalink
JIT: treat synthesized profile data as proper pgo data, fix finally p…
Browse files Browse the repository at this point in the history 
…ropagation

Once synthesis arrives on the scene, we're not going to want phases in the JIT
to arbitrarily modifing block weights. There is already a guard of this sort
for regular profile data, so it makes sense to extend that to synthesized data
as well.

When synthesizing counts, propagate counts to finallies from the associated
trys. This needs to be done carefully as we have make sure not to visit the
finally until the count in the try is set. We rely on some of the properties
of DFS pre and post number bracketing to do this efficiently, without needing
to track extra state.

Contributes to dotnet#82964.
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Mar 9, 2023
1 parent eff9b15 commit 99e8779
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 118 deletions.
3 changes: 2 additions & 1 deletion src/coreclr/inc/corjit.h
View file
Expand Up @@ -419,7 +419,8 @@ class ICorJitInfo : public ICorDynamicInfo
Blend = 3, // PGO data comes from blend of prior runs and current run
Text = 4, // PGO data comes from text file
IBC = 5, // PGO data from classic IBC
Sampling= 6, // PGO data derived from sampling
Sampling = 6, // PGO data derived from sampling
Synthesis = 7, // PGO data derived from synthesis
};

#define DEFAULT_UNKNOWN_HANDLE 1
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/codegencommon.cpp
View file
Expand Up @@ -1835,9 +1835,9 @@ void CodeGen::genGenerateMachineCode()
{
printf("; instrumented for collecting profile data\n");
}
else if (compiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT) && compiler->fgHaveProfileData())
else if (compiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT) && compiler->fgHaveProfileWeights())
{
printf("; optimized using profile data\n");
printf("; optimized using %s\n", compiler->compGetPgoSourceName());
}

#if DOUBLE_ALIGN
Expand All @@ -1856,7 +1856,7 @@ void CodeGen::genGenerateMachineCode()
printf("; partially interruptible\n");
}

if (compiler->fgHaveProfileData())
if (compiler->fgHaveProfileWeights())
{
printf("; with %s: edge weights are %s, and fgCalledCount is " FMT_WT "\n",
compiler->compGetPgoSourceName(), compiler->fgHaveValidEdgeWeights ? "valid" : "invalid",
Expand Down
17 changes: 8 additions & 9 deletions src/coreclr/jit/compiler.cpp
View file
Expand Up @@ -3343,14 +3343,9 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
printf("OPTIONS: compProcedureSplitting = %s\n", dspBool(opts.compProcedureSplitting));
printf("OPTIONS: compProcedureSplittingEH = %s\n", dspBool(opts.compProcedureSplittingEH));

if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT) && fgHaveProfileData())
if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT))
{
printf("OPTIONS: optimized using %s profile data\n", pgoSourceToString(fgPgoSource));
}

if (fgPgoFailReason != nullptr)
{
printf("OPTIONS: %s\n", fgPgoFailReason);
printf("OPTIONS: optimizer should use profile data\n");
}

if (jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT))
Expand Down Expand Up @@ -4266,13 +4261,17 @@ const char* Compiler::compGetPgoSourceName() const
case ICorJitInfo::PgoSource::Dynamic:
return "Dynamic PGO";
case ICorJitInfo::PgoSource::Blend:
return "Blend PGO";
return "Blended PGO";
case ICorJitInfo::PgoSource::Text:
return "Textual PGO";
case ICorJitInfo::PgoSource::Sampling:
return "Sample-based PGO";
case ICorJitInfo::PgoSource::IBC:
return "Classic IBC";
case ICorJitInfo::PgoSource::Synthesis:
return "Synthesized PGO";
default:
return "";
return "Unknown PGO";
}
}

Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Expand Up @@ -9731,7 +9731,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
}
}

const char* pgoSourceToString(ICorJitInfo::PgoSource p);
const char* devirtualizationDetailToString(CORINFO_DEVIRTUALIZATION_DETAIL detail);

#endif // DEBUG
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/fgbasic.cpp
View file
Expand Up @@ -3924,7 +3924,7 @@ void Compiler::fgCheckForLoopsInHandlers()
return;
}

// Walk blocks in handlers and filters, looing for a backedge target.
// Walk blocks in handlers and filters, looking for a backedge target.
//
assert(!compHasBackwardJumpInHandler);
for (BasicBlock* const blk : Blocks())
Expand Down
94 changes: 37 additions & 57 deletions src/coreclr/jit/fgprofile.cpp
View file
Expand Up @@ -54,11 +54,6 @@ bool Compiler::fgHaveProfileData()
//
bool Compiler::fgHaveProfileWeights()
{
if (!fgHaveProfileData())
{
return false;
}

return fgPgoHaveWeights;
}

Expand All @@ -79,12 +74,23 @@ bool Compiler::fgHaveSufficientProfileWeights()
return false;
}

if ((fgFirstBB != nullptr) && (fgPgoSource == ICorJitInfo::PgoSource::Static))
switch (fgPgoSource)
{
const weight_t sufficientSamples = 1000;
return fgFirstBB->bbWeight > sufficientSamples;
case ICorJitInfo::PgoSource::Dynamic:
case ICorJitInfo::PgoSource::Text:
return true;

case ICorJitInfo::PgoSource::Static:
case ICorJitInfo::PgoSource::Blend:
{
assert(fgFirstBB != nullptr);
const weight_t sufficientSamples = 1000;
return fgFirstBB->bbWeight > sufficientSamples;
}

default:
return false;
}
return true;
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -2493,7 +2499,6 @@ PhaseStatus Compiler::fgIncorporateProfileData()
{
JITDUMP("Synthesizing profile data and writing it out as the actual profile data\n");
ProfileSynthesis::Run(this, ProfileSynthesisOption::AssignLikelihoods);
fgPgoHaveWeights = false;
return PhaseStatus::MODIFIED_EVERYTHING;
}
#endif
Expand Down Expand Up @@ -2522,8 +2527,8 @@ PhaseStatus Compiler::fgIncorporateProfileData()

// Summarize profile data
//
JITDUMP("Have %s profile data: %d schema records (schema at %p, data at %p)\n", pgoSourceToString(fgPgoSource),
fgPgoSchemaCount, dspPtr(fgPgoSchema), dspPtr(fgPgoData));
JITDUMP("Have %s: %d schema records (schema at %p, data at %p)\n", compGetPgoSourceName(), fgPgoSchemaCount,
dspPtr(fgPgoSchema), dspPtr(fgPgoData));

fgNumProfileRuns = 0;
unsigned otherRecords = 0;
Expand Down Expand Up @@ -3431,15 +3436,29 @@ void EfficientEdgeCountReconstructor::Propagate()
//
if (m_badcode || m_mismatch || m_failedToConverge || m_allWeightsZero)
{
JITDUMP("... discarding profile data because of %s\n",
m_badcode ? "badcode" : m_mismatch ? "mismatch" : m_allWeightsZero ? "zero counts"
: "failed to converge");

// Make sure nothing else in the jit looks at the profile data.
//
m_comp->fgPgoSchema = nullptr;
m_comp->fgPgoFailReason = "PGO data available, but there was a reconstruction problem";
m_comp->fgPgoHaveWeights = false;
m_comp->fgPgoSchema = nullptr;

if (m_badcode)
{
m_comp->fgPgoFailReason = "PGO data available, but IL was malformed";
}
else if (m_mismatch)
{
m_comp->fgPgoFailReason = "PGO data available, but IL did not match";
}
else if (m_failedToConverge)
{
m_comp->fgPgoFailReason = "PGO data available, but solver did not converge";
}
else
{
m_comp->fgPgoFailReason = "PGO data available, profile data was all zero";
}

JITDUMP("... discarding profile data: %s\n", m_comp->fgPgoFailReason);
return;
}

Expand Down Expand Up @@ -5333,43 +5352,4 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks
return classicWeightsValid && likelyWeightsValid;
}

//------------------------------------------------------------------------------
// pgoSourceToString: describe source of pgo data
//
// Arguments:
// r - source enum to describe
//
// Returns:
// descriptive string
//
const char* Compiler::pgoSourceToString(ICorJitInfo::PgoSource p)
{
const char* pgoSource = "unknown";
switch (fgPgoSource)
{
case ICorJitInfo::PgoSource::Dynamic:
pgoSource = "dynamic";
break;
case ICorJitInfo::PgoSource::Static:
pgoSource = "static";
break;
case ICorJitInfo::PgoSource::Text:
pgoSource = "text";
break;
case ICorJitInfo::PgoSource::Blend:
pgoSource = "static+dynamic";
break;
case ICorJitInfo::PgoSource::IBC:
pgoSource = "IBC";
break;
case ICorJitInfo::PgoSource::Sampling:
pgoSource = "Sampling";
break;
default:
break;
}

return pgoSource;
}

#endif // DEBUG

0 comments on commit 99e8779

Please sign in to comment.