Skip to content

Commit f857468

Browse files
authored
JIT: start working on profile consistency (#81936)
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.
1 parent 6f36be5 commit f857468

File tree

5 files changed

+379
-146
lines changed

5 files changed

+379
-146
lines changed

src/coreclr/jit/compiler.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4433,6 +4433,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
44334433
//
44344434
activePhaseChecks |= PhaseChecks::CHECK_PROFILE;
44354435
DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData);
4436+
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;
44364437

44374438
// If we're going to instrument code, we may need to prepare before
44384439
// we import.

src/coreclr/jit/fgbasic.cpp

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,8 @@ bool Compiler::fgEnsureFirstBBisScratch()
276276
fgFirstBB->bbRefs--;
277277

278278
// The new scratch bb will fall through to the old first bb
279-
fgAddRefPred(fgFirstBB, block);
279+
FlowEdge* const edge = fgAddRefPred(fgFirstBB, block);
280+
edge->setLikelihood(1.0);
280281
fgInsertBBbefore(fgFirstBB, block);
281282
}
282283
else
@@ -2812,6 +2813,18 @@ void Compiler::fgLinkBasicBlocks()
28122813
}
28132814
}
28142815

2816+
// If this is an OSR compile, note the original entry and
2817+
// the OSR entry block.
2818+
//
2819+
// We don't yet alter flow; see fgFixEntryFlowForOSR.
2820+
//
2821+
if (opts.IsOSR())
2822+
{
2823+
assert(info.compILEntry >= 0);
2824+
fgEntryBB = fgLookupBB(0);
2825+
fgOSREntryBB = fgLookupBB(info.compILEntry);
2826+
}
2827+
28152828
// Pred lists now established.
28162829
//
28172830
fgPredsComputed = true;
@@ -3943,39 +3956,36 @@ void Compiler::fgCheckForLoopsInHandlers()
39433956
//
39443957
void Compiler::fgFixEntryFlowForOSR()
39453958
{
3946-
// Ensure lookup IL->BB lookup table is valid
3947-
//
3948-
fgInitBBLookup();
3949-
3950-
// Remember the original entry block in case this method is tail recursive.
3951-
//
3952-
fgEntryBB = fgLookupBB(0);
3953-
3954-
// Find the OSR entry block.
3959+
// We should have looked for these blocks in fgLinkBasicBlocks.
39553960
//
3956-
assert(info.compILEntry >= 0);
3957-
BasicBlock* const osrEntry = fgLookupBB(info.compILEntry);
3961+
assert(fgEntryBB != nullptr);
3962+
assert(fgOSREntryBB != nullptr);
39583963

3959-
// Remember the OSR entry block so we can find it again later.
3960-
//
3961-
fgOSREntryBB = osrEntry;
3962-
3963-
// Now branch from method start to the right spot.
3964+
// Now branch from method start to the OSR entry.
39643965
//
39653966
fgEnsureFirstBBisScratch();
39663967
assert(fgFirstBB->bbJumpKind == BBJ_NONE);
39673968
fgRemoveRefPred(fgFirstBB->bbNext, fgFirstBB);
39683969
fgFirstBB->bbJumpKind = BBJ_ALWAYS;
3969-
fgFirstBB->bbJumpDest = osrEntry;
3970-
fgAddRefPred(osrEntry, fgFirstBB);
3970+
fgFirstBB->bbJumpDest = fgOSREntryBB;
3971+
FlowEdge* const edge = fgAddRefPred(fgOSREntryBB, fgFirstBB);
3972+
edge->setLikelihood(1.0);
39713973

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

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

39813991
/*****************************************************************************

0 commit comments

Comments
 (0)