Skip to content

Commit

Permalink
JIT: Enable edge based profiles for all scenarios (#80481)
Browse files Browse the repository at this point in the history
Enable edge based profiles for OSR, partial compilation, and optimized plus
instrumented cases.

For OSR this requires deferring flow graph modifications until after we have
built the initial probe list, so that the initial list reflects the entirety
of the method. This set of candidate edge probes is thus the same no matter how
the method is compiled. A given compile may schematize a subset of these probes
and materialize a subset of what gets schematized; this is tolerated by the PGO
mechanism provided that the initial instrumented jitting produces a schema which
is a superset of the schema produced by any subsequent instrumented rejitting.
This is normally the case.

Partial compilation may still need some work to ensure full schematization but
it is currently off by default. Will address this subsequently.

For optimized compiles we give the EfficientEdgeCountInstrumentor the same kind
of probe relocation abilities that we have in the BlockCountInstrumentor. In
particular we need to move probes that might appear in return blocks that follow
implicit tail call blocks, since those return blocks must remain empty.

The details on how we do this are a bit different but the idea is the same: we
create duplicate copies of any probe that was going to appear in the return block
and instead instrument each pred. If the pred reached the return via a critical
edge, we split the edge and put the probe there. This analysis relies on cheap
preds, so to ensure we can use them we move all the critial edge splitting so it
happens before we need the cheap pred lists.

The ability to do block profiling is retained but will no longer be used without
special config settings.

There were also a few bug fixes in the spanning tree visitor. It must visit a
superset of the blocks we end up importing and was missing visits in some cases.

This should improve jit time and code quality for instrumented code.

Fixes #47942.
Fixes #66101.
Contributes to #74873.
  • Loading branch information
AndyAyersMS committed Jan 13, 2023
1 parent 8a5b42f commit eb76787
Show file tree
Hide file tree
Showing 3 changed files with 676 additions and 263 deletions.
22 changes: 11 additions & 11 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4389,6 +4389,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
}

// If we are doing OSR, update flow to initially reach the appropriate IL offset.
//
if (opts.IsOSR())
{
fgFixEntryFlowForOSR();
}

// Enable the post-phase checks that use internal logic to decide when checking makes sense.
//
activePhaseChecks = PhaseChecks::CHECK_EH | PhaseChecks::CHECK_LOOPS | PhaseChecks::CHECK_UNIQUE |
Expand All @@ -4398,17 +4405,17 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_IMPORTATION, &Compiler::fgImport);

// Expand any patchpoints
//
DoPhase(this, PHASE_PATCHPOINTS, &Compiler::fgTransformPatchpoints);

// If instrumenting, add block and class probes.
//
if (compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR))
{
DoPhase(this, PHASE_IBCINSTR, &Compiler::fgInstrumentMethod);
}

// Expand any patchpoints
//
DoPhase(this, PHASE_PATCHPOINTS, &Compiler::fgTransformPatchpoints);

// Transform indirect calls that require control flow expansion.
//
DoPhase(this, PHASE_INDXCALL, &Compiler::fgTransformIndirectCalls);
Expand Down Expand Up @@ -6594,13 +6601,6 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
{
// We are jitting the root method, or inlining.
fgFindBasicBlocks();

// If we are doing OSR, update flow to initially reach the appropriate IL offset.
//
if (opts.IsOSR())
{
fgFixEntryFlowForOSR();
}
}

// If we're inlining and the candidate is bad, bail out.
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3621,6 +3621,11 @@ void Compiler::fgFixEntryFlowForOSR()
fgFirstBB->bbJumpDest = osrEntry;
fgAddRefPred(osrEntry, fgFirstBB);

// Give the method entry (which will be a scratch BB)
// the same weight as the OSR Entry.
//
fgFirstBB->inheritWeight(fgOSREntryBB);

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

0 comments on commit eb76787

Please sign in to comment.