Skip to content

Commit

Permalink
JIT: start working on profile consistency (#81936)
Browse files Browse the repository at this point in the history
Dump flow graph before running post-phase checks so the flow graph
visualization can be used in conjunction with the profile checker output
to spot profile problems.

Various provisional fixes to try and shore up the inevitably inconsistent
OSR profile. Add a compensating pseudo-edges to the OSR entry to help
explain where the missing flow counts have gone. Relax checking for
the OSR entry. Add likelhood when we redirect flow to the OSR entry.
Locate the OSR entry and original method entry early so we can
special-case them in diagnostics.

Defer marking interesting blocks (say for switch peeling) until we've
set edge likelihoods, since (someday) those decisions should use edge
likelihoods.

Disable profile checking after profile incorporation. The plan is to
walk this disable back incrementally and fix issues in each succesive
phase, ultimately checking them all. But currently we still have a lot
of check failures right after this first phase.

Contributes to #46885.
  • Loading branch information
AndyAyersMS committed Feb 13, 2023
1 parent 6f36be5 commit f857468
Show file tree
Hide file tree
Showing 5 changed files with 379 additions and 146 deletions.
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4433,6 +4433,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
activePhaseChecks |= PhaseChecks::CHECK_PROFILE;
DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData);
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;

// If we're going to instrument code, we may need to prepare before
// we import.
Expand Down
58 changes: 34 additions & 24 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ bool Compiler::fgEnsureFirstBBisScratch()
fgFirstBB->bbRefs--;

// The new scratch bb will fall through to the old first bb
fgAddRefPred(fgFirstBB, block);
FlowEdge* const edge = fgAddRefPred(fgFirstBB, block);
edge->setLikelihood(1.0);
fgInsertBBbefore(fgFirstBB, block);
}
else
Expand Down Expand Up @@ -2812,6 +2813,18 @@ void Compiler::fgLinkBasicBlocks()
}
}

// If this is an OSR compile, note the original entry and
// the OSR entry block.
//
// We don't yet alter flow; see fgFixEntryFlowForOSR.
//
if (opts.IsOSR())
{
assert(info.compILEntry >= 0);
fgEntryBB = fgLookupBB(0);
fgOSREntryBB = fgLookupBB(info.compILEntry);
}

// Pred lists now established.
//
fgPredsComputed = true;
Expand Down Expand Up @@ -3943,39 +3956,36 @@ void Compiler::fgCheckForLoopsInHandlers()
//
void Compiler::fgFixEntryFlowForOSR()
{
// Ensure lookup IL->BB lookup table is valid
//
fgInitBBLookup();

// Remember the original entry block in case this method is tail recursive.
//
fgEntryBB = fgLookupBB(0);

// Find the OSR entry block.
// We should have looked for these blocks in fgLinkBasicBlocks.
//
assert(info.compILEntry >= 0);
BasicBlock* const osrEntry = fgLookupBB(info.compILEntry);
assert(fgEntryBB != nullptr);
assert(fgOSREntryBB != nullptr);

// Remember the OSR entry block so we can find it again later.
//
fgOSREntryBB = osrEntry;

// Now branch from method start to the right spot.
// Now branch from method start to the OSR entry.
//
fgEnsureFirstBBisScratch();
assert(fgFirstBB->bbJumpKind == BBJ_NONE);
fgRemoveRefPred(fgFirstBB->bbNext, fgFirstBB);
fgFirstBB->bbJumpKind = BBJ_ALWAYS;
fgFirstBB->bbJumpDest = osrEntry;
fgAddRefPred(osrEntry, fgFirstBB);
fgFirstBB->bbJumpDest = fgOSREntryBB;
FlowEdge* const edge = fgAddRefPred(fgOSREntryBB, fgFirstBB);
edge->setLikelihood(1.0);

// Give the method entry (which will be a scratch BB)
// the same weight as the OSR Entry.
// We don't know the right weight for this block, since
// execution of the method was interrupted within the
// loop containing fgOSREntryBB.
//
// A plausible guess might be to sum the non-backedge
// weights of fgOSREntryBB and use those, but we don't
// have edge weights available yet. Note that might be
// an underestimate.
//
// For now we just guess that the loop will execute 100x.
//
fgFirstBB->inheritWeight(fgOSREntryBB);
fgFirstBB->inheritWeightPercentage(fgOSREntryBB, 1);

JITDUMP("OSR: redirecting flow at entry from entry " FMT_BB " to OSR entry " FMT_BB " for the importer\n",
fgFirstBB->bbNum, osrEntry->bbNum);
JITDUMP("OSR: redirecting flow at method entry from " FMT_BB " to OSR entry " FMT_BB " for the importer\n",
fgFirstBB->bbNum, fgOSREntryBB->bbNum);
}

/*****************************************************************************
Expand Down
Loading

0 comments on commit f857468

Please sign in to comment.