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.

diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp
index 3a124f8..68bea74 100644 (file)
--- a/src/coreclr/jit/compiler.cpp
+++ b/src/coreclr/jit/compiler.cpp
@@ -1848,13 +1848,12 @@ void Compiler::compInit(ArenaAllocator*       pAlloc,
 
     fgInit();
     lvaInit();
+    optInit();
 
     if (!compIsForInlining())
     {
         codeGen = getCodeGenerator(this);
-        optInit();
         hashBv::Init(this);
-
         compVarScopeMap = nullptr;
 
         // If this method were a real constructor for Compiler, these would
@@ -4390,6 +4389,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         DoPhase(this, PHASE_IBCPREP, &Compiler::fgPrepareToInstrumentMethod);
     }
 
+    // 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 | PhaseChecks::CHECK_PROFILE;
+
     // Import: convert the instrs in each basic block to a tree based intermediate representation
     //
     DoPhase(this, PHASE_IMPORTATION, &Compiler::fgImport);
@@ -4566,6 +4570,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
         fgRenumberBlocks();
         noway_assert(!fgComputePredsDone);
         fgComputePreds();
+        // Enable flow graph checks
+        activePhaseChecks |= PhaseChecks::CHECK_FG;
     };
     DoPhase(this, PHASE_COMPUTE_PREDS, computePredsPhase);
 
@@ -4647,8 +4653,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
             fgRenumberBlocks();
         }
 
-        // We can now enable all phase checking
-        activePhaseChecks = PhaseChecks::CHECK_ALL;
+        // Enable IR checks
+        activePhaseChecks |= PhaseChecks::CHECK_IR;
     };
     DoPhase(this, PHASE_MORPH_GLOBAL, morphGlobalPhase);
 

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
index cf1ce73..c83c82b 100644 (file)
--- a/src/coreclr/jit/compiler.h
+++ b/src/coreclr/jit/compiler.h
@@ -1451,12 +1451,49 @@ extern const char* PhaseEnums[];
 
 // Specify which checks should be run after each phase
 //
-enum class PhaseChecks
+// clang-format off
+enum class PhaseChecks : unsigned int
 {
-    CHECK_NONE,
-    CHECK_ALL
+    CHECK_NONE    = 0,
+    CHECK_IR      = 1 << 0, // ir flags, etc
+    CHECK_UNIQUE  = 1 << 1, // tree node uniqueness
+    CHECK_FG      = 1 << 2, // flow graph integrity
+    CHECK_EH      = 1 << 3, // eh table integrity
+    CHECK_LOOPS   = 1 << 4, // loop table integrity
+    CHECK_PROFILE = 1 << 5, // profile data integrity
 };
 
+inline constexpr PhaseChecks operator ~(PhaseChecks a)
+{
+    return (PhaseChecks)(~(unsigned int)a);
+}
+
+inline constexpr PhaseChecks operator |(PhaseChecks a, PhaseChecks b)
+{
+    return (PhaseChecks)((unsigned int)a | (unsigned int)b);
+}
+
+inline constexpr PhaseChecks operator &(PhaseChecks a, PhaseChecks b)
+{
+    return (PhaseChecks)((unsigned int)a & (unsigned int)b);
+}
+
+inline PhaseChecks& operator |=(PhaseChecks& a, PhaseChecks b)
+{
+    return a = (PhaseChecks)((unsigned int)a | (unsigned int)b);
+}
+
+inline PhaseChecks& operator &=(PhaseChecks& a, PhaseChecks b)
+{
+    return a = (PhaseChecks)((unsigned int)a & (unsigned int)b);
+}
+
+inline PhaseChecks& operator ^=(PhaseChecks& a, PhaseChecks b)
+{
+    return a = (PhaseChecks)((unsigned int)a ^ (unsigned int)b);
+}
+// clang-format on
+
 // Specify which dumps should be run after each phase
 //
 enum class PhaseDumps

diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp
index 84acf41..7f095d9 100644 (file)
--- a/src/coreclr/jit/fgprofile.cpp
+++ b/src/coreclr/jit/fgprofile.cpp
@@ -4201,9 +4201,18 @@ bool Compiler::fgProfileWeightsConsistent(weight_t weight1, weight_t weight2)
 //   we expect EH edge counts to be small, so errors from ignoring
 //   them should be rare.
 //
+//   There's no point checking until we've built pred lists, as
+//   we can't easily reason about consistency without them.
+//
 void Compiler::fgDebugCheckProfileWeights()
 {
-    assert(fgComputePredsDone);
+    // Optionally check profile data, if we have any.
+    //
+    const bool enabled = (JitConfig.JitProfileChecks() > 0) && fgHaveProfileWeights() && fgComputePredsDone;
+    if (!enabled)
+    {
+        return;
+    }
 
     // We can't check before we have computed edge weights.
     //

diff --git a/src/coreclr/jit/phase.cpp b/src/coreclr/jit/phase.cpp
index 8b69570..5ec8d3e 100644 (file)
--- a/src/coreclr/jit/phase.cpp
+++ b/src/coreclr/jit/phase.cpp
@@ -106,7 +106,7 @@ void Phase::PostPhase(PhaseStatus status)
     //
     const bool madeChanges       = (status != PhaseStatus::MODIFIED_NOTHING);
     const bool doPostPhase       = madeChanges;
-    const bool doPostPhaseChecks = (comp->activePhaseChecks == PhaseChecks::CHECK_ALL);
+    const bool doPostPhaseChecks = (comp->activePhaseChecks != PhaseChecks::CHECK_NONE);
     const bool doPostPhaseDumps  = (comp->activePhaseDumps == PhaseDumps::DUMP_ALL);
 
     const char* const statusMessage = madeChanges ? "" : " [no changes]";
@@ -132,27 +132,36 @@ void Phase::PostPhase(PhaseStatus status)
 
     if (doPostPhase && doPostPhaseChecks)
     {
-        comp->fgDebugCheckBBlist();
-        comp->fgDebugCheckLinks();
-        comp->fgDebugCheckNodesUniqueness();
-        comp->fgVerifyHandlerTab();
-        comp->fgDebugCheckLoopTable();
-    }
+        if ((comp->activePhaseChecks & PhaseChecks::CHECK_UNIQUE) == PhaseChecks::CHECK_UNIQUE)
+        {
+            comp->fgDebugCheckNodesUniqueness();
+        }
 
-    // Optionally check profile data, if we have any.
-    //
-    // There's no point checking until we've built pred lists, as
-    // we can't easily reason about consistency without them.
-    //
-    // Bypass the "doPostPhase" filter until we're sure all
-    // phases that mess with profile counts set their phase status
-    // appropriately.
-    //
-    if ((JitConfig.JitProfileChecks() > 0) && comp->fgHaveProfileWeights() && comp->fgComputePredsDone)
-    {
-        comp->fgDebugCheckProfileWeights();
-    }
+        if ((comp->activePhaseChecks & PhaseChecks::CHECK_FG) == PhaseChecks::CHECK_FG)
+        {
+            comp->fgDebugCheckBBlist();
+        }
+
+        if ((comp->activePhaseChecks & PhaseChecks::CHECK_IR) == PhaseChecks::CHECK_IR)
+        {
+            comp->fgDebugCheckLinks();
+        }
+
+        if ((comp->activePhaseChecks & PhaseChecks::CHECK_EH) == PhaseChecks::CHECK_EH)
+        {
+            comp->fgVerifyHandlerTab();
+        }
 
+        if ((comp->activePhaseChecks & PhaseChecks::CHECK_LOOPS) == PhaseChecks::CHECK_LOOPS)
+        {
+            comp->fgDebugCheckLoopTable();
+        }
+
+        if ((comp->activePhaseChecks & PhaseChecks::CHECK_PROFILE) == PhaseChecks::CHECK_PROFILE)
+        {
+            comp->fgDebugCheckProfileWeights();
+        }
+    }
 #endif // DEBUG
 
 #if DUMP_FLOWGRAPHS