Skip to content

Commit 2fc497b

Browse files
authored
JIT: Refactor post phase checks (#80194)
Add finer-grained notions of what post-phase checks are enabled. Start checking some IR and FG invariants earlier than we did before. This is largely done in anticipation of moving pred list building earlier (#80193). We should also consider moving some of the IR checking earlier as well.
1 parent 42a82dd commit 2fc497b

File tree

4 files changed

+89
-28
lines changed

4 files changed

+89
-28
lines changed

src/coreclr/jit/compiler.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1848,13 +1848,12 @@ void Compiler::compInit(ArenaAllocator* pAlloc,
18481848

18491849
fgInit();
18501850
lvaInit();
1851+
optInit();
18511852

18521853
if (!compIsForInlining())
18531854
{
18541855
codeGen = getCodeGenerator(this);
1855-
optInit();
18561856
hashBv::Init(this);
1857-
18581857
compVarScopeMap = nullptr;
18591858

18601859
// If this method were a real constructor for Compiler, these would
@@ -4390,6 +4389,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
43904389
DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
43914390
}
43924391

4392+
// Enable the post-phase checks that use internal logic to decide when checking makes sense.
4393+
//
4394+
activePhaseChecks =
4395+
PhaseChecks::CHECK_EH | PhaseChecks::CHECK_LOOPS | PhaseChecks::CHECK_UNIQUE | PhaseChecks::CHECK_PROFILE;
4396+
43934397
// Import: convert the instrs in each basic block to a tree based intermediate representation
43944398
//
43954399
DoPhase(this, PHASE_IMPORTATION, &Compiler::fgImport);
@@ -4566,6 +4570,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
45664570
fgRenumberBlocks();
45674571
noway_assert(!fgComputePredsDone);
45684572
fgComputePreds();
4573+
// Enable flow graph checks
4574+
activePhaseChecks |= PhaseChecks::CHECK_FG;
45694575
};
45704576
DoPhase(this, PHASE_COMPUTE_PREDS, computePredsPhase);
45714577

@@ -4647,8 +4653,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
46474653
fgRenumberBlocks();
46484654
}
46494655

4650-
// We can now enable all phase checking
4651-
activePhaseChecks = PhaseChecks::CHECK_ALL;
4656+
// Enable IR checks
4657+
activePhaseChecks |= PhaseChecks::CHECK_IR;
46524658
};
46534659
DoPhase(this, PHASE_MORPH_GLOBAL, morphGlobalPhase);
46544660

src/coreclr/jit/compiler.h

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1451,12 +1451,49 @@ extern const char* PhaseEnums[];
14511451

14521452
// Specify which checks should be run after each phase
14531453
//
1454-
enum class PhaseChecks
1454+
// clang-format off
1455+
enum class PhaseChecks : unsigned int
14551456
{
1456-
CHECK_NONE,
1457-
CHECK_ALL
1457+
CHECK_NONE = 0,
1458+
CHECK_IR = 1 << 0, // ir flags, etc
1459+
CHECK_UNIQUE = 1 << 1, // tree node uniqueness
1460+
CHECK_FG = 1 << 2, // flow graph integrity
1461+
CHECK_EH = 1 << 3, // eh table integrity
1462+
CHECK_LOOPS = 1 << 4, // loop table integrity
1463+
CHECK_PROFILE = 1 << 5, // profile data integrity
14581464
};
14591465

1466+
inline constexpr PhaseChecks operator ~(PhaseChecks a)
1467+
{
1468+
return (PhaseChecks)(~(unsigned int)a);
1469+
}
1470+
1471+
inline constexpr PhaseChecks operator |(PhaseChecks a, PhaseChecks b)
1472+
{
1473+
return (PhaseChecks)((unsigned int)a | (unsigned int)b);
1474+
}
1475+
1476+
inline constexpr PhaseChecks operator &(PhaseChecks a, PhaseChecks b)
1477+
{
1478+
return (PhaseChecks)((unsigned int)a & (unsigned int)b);
1479+
}
1480+
1481+
inline PhaseChecks& operator |=(PhaseChecks& a, PhaseChecks b)
1482+
{
1483+
return a = (PhaseChecks)((unsigned int)a | (unsigned int)b);
1484+
}
1485+
1486+
inline PhaseChecks& operator &=(PhaseChecks& a, PhaseChecks b)
1487+
{
1488+
return a = (PhaseChecks)((unsigned int)a & (unsigned int)b);
1489+
}
1490+
1491+
inline PhaseChecks& operator ^=(PhaseChecks& a, PhaseChecks b)
1492+
{
1493+
return a = (PhaseChecks)((unsigned int)a ^ (unsigned int)b);
1494+
}
1495+
// clang-format on
1496+
14601497
// Specify which dumps should be run after each phase
14611498
//
14621499
enum class PhaseDumps

src/coreclr/jit/fgprofile.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4201,9 +4201,18 @@ bool Compiler::fgProfileWeightsConsistent(weight_t weight1, weight_t weight2)
42014201
// we expect EH edge counts to be small, so errors from ignoring
42024202
// them should be rare.
42034203
//
4204+
// There's no point checking until we've built pred lists, as
4205+
// we can't easily reason about consistency without them.
4206+
//
42044207
void Compiler::fgDebugCheckProfileWeights()
42054208
{
4206-
assert(fgComputePredsDone);
4209+
// Optionally check profile data, if we have any.
4210+
//
4211+
const bool enabled = (JitConfig.JitProfileChecks() > 0) && fgHaveProfileWeights() && fgComputePredsDone;
4212+
if (!enabled)
4213+
{
4214+
return;
4215+
}
42074216

42084217
// We can't check before we have computed edge weights.
42094218
//

src/coreclr/jit/phase.cpp

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ void Phase::PostPhase(PhaseStatus status)
106106
//
107107
const bool madeChanges = (status != PhaseStatus::MODIFIED_NOTHING);
108108
const bool doPostPhase = madeChanges;
109-
const bool doPostPhaseChecks = (comp->activePhaseChecks == PhaseChecks::CHECK_ALL);
109+
const bool doPostPhaseChecks = (comp->activePhaseChecks != PhaseChecks::CHECK_NONE);
110110
const bool doPostPhaseDumps = (comp->activePhaseDumps == PhaseDumps::DUMP_ALL);
111111

112112
const char* const statusMessage = madeChanges ? "" : " [no changes]";
@@ -132,27 +132,36 @@ void Phase::PostPhase(PhaseStatus status)
132132

133133
if (doPostPhase && doPostPhaseChecks)
134134
{
135-
comp->fgDebugCheckBBlist();
136-
comp->fgDebugCheckLinks();
137-
comp->fgDebugCheckNodesUniqueness();
138-
comp->fgVerifyHandlerTab();
139-
comp->fgDebugCheckLoopTable();
140-
}
135+
if ((comp->activePhaseChecks & PhaseChecks::CHECK_UNIQUE) == PhaseChecks::CHECK_UNIQUE)
136+
{
137+
comp->fgDebugCheckNodesUniqueness();
138+
}
141139

142-
// Optionally check profile data, if we have any.
143-
//
144-
// There's no point checking until we've built pred lists, as
145-
// we can't easily reason about consistency without them.
146-
//
147-
// Bypass the "doPostPhase" filter until we're sure all
148-
// phases that mess with profile counts set their phase status
149-
// appropriately.
150-
//
151-
if ((JitConfig.JitProfileChecks() > 0) && comp->fgHaveProfileWeights() && comp->fgComputePredsDone)
152-
{
153-
comp->fgDebugCheckProfileWeights();
154-
}
140+
if ((comp->activePhaseChecks & PhaseChecks::CHECK_FG) == PhaseChecks::CHECK_FG)
141+
{
142+
comp->fgDebugCheckBBlist();
143+
}
144+
145+
if ((comp->activePhaseChecks & PhaseChecks::CHECK_IR) == PhaseChecks::CHECK_IR)
146+
{
147+
comp->fgDebugCheckLinks();
148+
}
149+
150+
if ((comp->activePhaseChecks & PhaseChecks::CHECK_EH) == PhaseChecks::CHECK_EH)
151+
{
152+
comp->fgVerifyHandlerTab();
153+
}
155154

155+
if ((comp->activePhaseChecks & PhaseChecks::CHECK_LOOPS) == PhaseChecks::CHECK_LOOPS)
156+
{
157+
comp->fgDebugCheckLoopTable();
158+
}
159+
160+
if ((comp->activePhaseChecks & PhaseChecks::CHECK_PROFILE) == PhaseChecks::CHECK_PROFILE)
161+
{
162+
comp->fgDebugCheckProfileWeights();
163+
}
164+
}
156165
#endif // DEBUG
157166

158167
#if DUMP_FLOWGRAPHS

0 commit comments

Comments
 (0)