Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: fix interaction of PGO and jitstress #47876

Merged
merged 1 commit into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 9 additions & 15 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2879,24 +2879,20 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
fgPgoSchema = nullptr;
fgPgoData = nullptr;
fgPgoSchemaCount = 0;
fgPgoQueryResult = E_FAIL;
fgProfileData_ILSizeMismatch = false;
if (jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT))
{
HRESULT hr;
hr = info.compCompHnd->getPgoInstrumentationResults(info.compMethodHnd, &fgPgoSchema, &fgPgoSchemaCount,
&fgPgoData);

JITDUMP(
"BBOPT set; query for profile data returned hr %0x, schema at %p, counts at %p, schema element count %d\n",
hr, dspPtr(fgPgoSchema), dspPtr(fgPgoData), fgPgoSchemaCount);
fgPgoQueryResult = info.compCompHnd->getPgoInstrumentationResults(info.compMethodHnd, &fgPgoSchema,
&fgPgoSchemaCount, &fgPgoData);

// a failed result that also has a non-NULL fgPgoSchema
// indicates that the ILSize for the method no longer matches
// the ILSize for the method when profile data was collected.
//
// We will discard the IBC data in this case
//
if (FAILED(hr) && (fgPgoSchema != nullptr))
if (FAILED(fgPgoQueryResult) && (fgPgoSchema != nullptr))
{
fgProfileData_ILSizeMismatch = true;
fgPgoData = nullptr;
Expand All @@ -2905,15 +2901,15 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
#ifdef DEBUG
// A successful result implies a non-NULL fgPgoSchema
//
if (SUCCEEDED(hr))
if (SUCCEEDED(fgPgoQueryResult))
{
assert(fgPgoSchema != nullptr);
}

// A failed result implies a NULL fgPgoSchema
// see implementation of Compiler::fgHaveProfileData()
//
if (FAILED(hr))
if (FAILED(fgPgoQueryResult))
{
assert(fgPgoSchema == nullptr);
}
Expand Down Expand Up @@ -4400,14 +4396,12 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags

compFunctionTraceStart();

// If profile data is available, incorporate it into the flowgraph.
// Incorporate profile data.
//
// Note: the importer is sensitive to block weights, so this has
// to happen before importation.
//
if (compileFlags->IsSet(JitFlags::JIT_FLAG_BBOPT) && fgHaveProfileData())
{
DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData);
}
DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData);

// Import: convert the instrs in each basic block to a tree based intermediate representation
//
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5539,6 +5539,7 @@ class Compiler
ICorJitInfo::PgoInstrumentationSchema* fgPgoSchema;
BYTE* fgPgoData;
UINT32 fgPgoSchemaCount;
HRESULT fgPgoQueryResult;
UINT32 fgNumProfileRuns;
UINT32 fgPgoBlockCounts;
UINT32 fgPgoClassProfiles;
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2251,6 +2251,10 @@ void Compiler::fgFindBasicBlocks()
{
printf("*************** In fgFindBasicBlocks() for %s\n", info.compFullName);
}

// Call this here so any dump printing it inspires doesn't appear in the bb table.
//
fgStressBBProf();
#endif

// Allocate the 'jump target' bit vector
Expand Down
40 changes: 34 additions & 6 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -885,11 +885,38 @@ PhaseStatus Compiler::fgInstrumentMethod()
//
PhaseStatus Compiler::fgIncorporateProfileData()
{
assert(fgHaveProfileData());
// Are we doing profile stress?
//
if (fgStressBBProf() > 0)
{
JITDUMP("JitStress -- incorporating random profile data\n");
fgIncorporateBlockCounts();
return PhaseStatus::MODIFIED_EVERYTHING;
}

// Do we have profile data?
//
if (!fgHaveProfileData())
{
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBOPT))
{
JITDUMP("BBOPT set, but no profile data available (hr=%08x)\n", fgPgoQueryResult);
}
else
{
JITDUMP("BBOPT not set\n");
}
return PhaseStatus::MODIFIED_NOTHING;
}

// Summarize profile data
//
fgNumProfileRuns = 0;
JITDUMP("Have profile data: %d schema records (schema at %p, data at %p)\n", fgPgoSchemaCount, dspPtr(fgPgoSchema),
dspPtr(fgPgoData));

fgNumProfileRuns = 0;
unsigned otherRecords = 0;

for (UINT32 iSchema = 0; iSchema < fgPgoSchemaCount; iSchema++)
{
switch (fgPgoSchema[iSchema].InstrumentationKind)
Expand All @@ -907,19 +934,20 @@ PhaseStatus Compiler::fgIncorporateProfileData()
break;

default:
otherRecords++;
break;
}
}

assert(fgPgoBlockCounts > 0);

if (fgNumProfileRuns == 0)
{
fgNumProfileRuns = 1;
}

JITDUMP("Profile summary: %d runs, %d block probes, %d class profiles\n", fgNumProfileRuns, fgPgoBlockCounts,
fgPgoClassProfiles);
JITDUMP("Profile summary: %d runs, %d block probes, %d class profiles, %d other records\n", fgNumProfileRuns,
fgPgoBlockCounts, fgPgoClassProfiles, otherRecords);

assert(fgPgoBlockCounts > 0);

fgIncorporateBlockCounts();
return PhaseStatus::MODIFIED_EVERYTHING;
Expand Down