From 918c53422837e482bca9499385fe199602aac4c7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 12:21:06 +0000 Subject: [PATCH 1/3] Initial plan From fd6c1cf2a3aac83240fea46c1634e139c0ae075f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 12:45:03 +0000 Subject: [PATCH 2/3] Remove CHK_FLOW_UPDATE stress mode and associated code Co-authored-by: jakobbotsch <7887810+jakobbotsch@users.noreply.github.com> --- src/coreclr/jit/compiler.h | 2 - src/coreclr/jit/fgdiagnostic.cpp | 98 -------------------------------- src/coreclr/jit/fgopt.cpp | 1 - 3 files changed, 101 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index c59041eb655972..f8a22b1bfee4b1 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6308,7 +6308,6 @@ class Compiler static fgWalkPreFn fgStress64RsltMulCB; void fgStress64RsltMul(); - void fgDebugCheckUpdate(); void fgDebugCheckBBNumIncreasing(); void fgDebugCheckBBlist(bool checkBBNum = false, bool checkBBRefs = true); @@ -10441,7 +10440,6 @@ class Compiler /* don't care about performance at all */ \ \ STRESS_MODE(FORCE_INLINE) /* Treat every method as AggressiveInlining */ \ - STRESS_MODE(CHK_FLOW_UPDATE) \ STRESS_MODE(EMITTER) \ STRESS_MODE(CHK_REIMPORT) \ STRESS_MODE(GENERIC_CHECK) \ diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 146866f961d696..286ff5cca62cc4 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -50,104 +50,6 @@ void Compiler::fgPrintEdgeWeights() } #endif // DEBUG -/***************************************************************************** - * Check that the flow graph is really updated - */ - -#ifdef DEBUG - -void Compiler::fgDebugCheckUpdate() -{ - if (!compStressCompile(STRESS_CHK_FLOW_UPDATE, 30)) - { - return; - } - - /* We check for these conditions: - * no unreachable blocks -> no blocks have countOfInEdges() = 0 - * no empty blocks -> !block->isEmpty(), unless non-removable or multiple in-edges - * no un-imported blocks -> no blocks have BBF_IMPORTED not set (this is - * kind of redundant with the above, but to make sure) - * no un-compacted blocks -> BBJ_ALWAYS with jump to block with no other jumps to it (countOfInEdges() = 1) - */ - - BasicBlock* prev; - BasicBlock* block; - for (prev = nullptr, block = fgFirstBB; block != nullptr; prev = block, block = block->Next()) - { - /* no unreachable blocks */ - - if ((block->countOfInEdges() == 0) && !block->HasFlag(BBF_DONT_REMOVE)) - { - noway_assert(!"Unreachable block not removed!"); - } - - /* no empty blocks */ - - if (block->isEmpty() && !block->HasFlag(BBF_DONT_REMOVE)) - { - switch (block->GetKind()) - { - case BBJ_CALLFINALLY: - case BBJ_EHFINALLYRET: - case BBJ_EHFAULTRET: - case BBJ_EHFILTERRET: - case BBJ_RETURN: - /* for BBJ_ALWAYS is probably just a GOTO, but will have to be treated */ - case BBJ_ALWAYS: - case BBJ_EHCATCHRET: - /* These jump kinds are allowed to have empty tree lists */ - break; - - default: - /* it may be the case that the block had more than one reference to it - * so we couldn't remove it */ - - if (block->countOfInEdges() == 0) - { - noway_assert(!"Empty block not removed!"); - } - break; - } - } - - /* no un-imported blocks */ - - if (!block->HasFlag(BBF_IMPORTED)) - { - /* internal blocks do not count */ - - if (!block->HasFlag(BBF_INTERNAL)) - { - noway_assert(!"Non IMPORTED block not removed!"); - } - } - - // Check for an unnecessary jumps to the next block. - // A conditional branch should never jump to the next block as it can be folded into a BBJ_ALWAYS. - if (block->KindIs(BBJ_COND) && block->TrueEdgeIs(block->GetFalseEdge())) - { - noway_assert(!"Unnecessary jump to the next block!"); - } - - // For a BBJ_CALLFINALLY block we make sure that we are followed by a BBJ_CALLFINALLYRET block - // or that it's a BBF_RETLESS_CALL. - if (block->KindIs(BBJ_CALLFINALLY)) - { - assert(block->HasFlag(BBF_RETLESS_CALL) || block->isBBCallFinallyPair()); - } - - /* no un-compacted blocks */ - - if (fgCanCompactBlock(block)) - { - noway_assert(!"Found un-compacted blocks!"); - } - } -} - -#endif // DEBUG - #if DUMP_FLOWGRAPHS struct escapeMapping_t diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 09d0c8819b68a1..70c82d4cd39af5 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -4776,7 +4776,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh fgVerifyHandlerTab(); // Make sure that the predecessor lists are accurate fgDebugCheckBBlist(); - fgDebugCheckUpdate(); } #endif // DEBUG From 54e60582d8e2d0fa6b1ea8bd1f60a24c4bb9fa29 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 13:18:54 +0000 Subject: [PATCH 3/3] Keep fgDebugCheckUpdate function, remove only the stress mode check Per feedback from @jakobbotsch, the fgDebugCheckUpdate function is now always executed in debug builds instead of being gated by the STRESS_CHK_FLOW_UPDATE stress mode. Co-authored-by: jakobbotsch <7887810+jakobbotsch@users.noreply.github.com> --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/fgdiagnostic.cpp | 93 ++++++++++++++++++++++++++++++++ src/coreclr/jit/fgopt.cpp | 1 + 3 files changed, 95 insertions(+) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f8a22b1bfee4b1..c681992ee76a08 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6308,6 +6308,7 @@ class Compiler static fgWalkPreFn fgStress64RsltMulCB; void fgStress64RsltMul(); + void fgDebugCheckUpdate(); void fgDebugCheckBBNumIncreasing(); void fgDebugCheckBBlist(bool checkBBNum = false, bool checkBBRefs = true); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 286ff5cca62cc4..18482db7261ef8 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -50,6 +50,99 @@ void Compiler::fgPrintEdgeWeights() } #endif // DEBUG +/***************************************************************************** + * Check that the flow graph is really updated + */ + +#ifdef DEBUG + +void Compiler::fgDebugCheckUpdate() +{ + /* We check for these conditions: + * no unreachable blocks -> no blocks have countOfInEdges() = 0 + * no empty blocks -> !block->isEmpty(), unless non-removable or multiple in-edges + * no un-imported blocks -> no blocks have BBF_IMPORTED not set (this is + * kind of redundant with the above, but to make sure) + * no un-compacted blocks -> BBJ_ALWAYS with jump to block with no other jumps to it (countOfInEdges() = 1) + */ + + BasicBlock* prev; + BasicBlock* block; + for (prev = nullptr, block = fgFirstBB; block != nullptr; prev = block, block = block->Next()) + { + /* no unreachable blocks */ + + if ((block->countOfInEdges() == 0) && !block->HasFlag(BBF_DONT_REMOVE)) + { + noway_assert(!"Unreachable block not removed!"); + } + + /* no empty blocks */ + + if (block->isEmpty() && !block->HasFlag(BBF_DONT_REMOVE)) + { + switch (block->GetKind()) + { + case BBJ_CALLFINALLY: + case BBJ_EHFINALLYRET: + case BBJ_EHFAULTRET: + case BBJ_EHFILTERRET: + case BBJ_RETURN: + /* for BBJ_ALWAYS is probably just a GOTO, but will have to be treated */ + case BBJ_ALWAYS: + case BBJ_EHCATCHRET: + /* These jump kinds are allowed to have empty tree lists */ + break; + + default: + /* it may be the case that the block had more than one reference to it + * so we couldn't remove it */ + + if (block->countOfInEdges() == 0) + { + noway_assert(!"Empty block not removed!"); + } + break; + } + } + + /* no un-imported blocks */ + + if (!block->HasFlag(BBF_IMPORTED)) + { + /* internal blocks do not count */ + + if (!block->HasFlag(BBF_INTERNAL)) + { + noway_assert(!"Non IMPORTED block not removed!"); + } + } + + // Check for an unnecessary jumps to the next block. + // A conditional branch should never jump to the next block as it can be folded into a BBJ_ALWAYS. + if (block->KindIs(BBJ_COND) && block->TrueEdgeIs(block->GetFalseEdge())) + { + noway_assert(!"Unnecessary jump to the next block!"); + } + + // For a BBJ_CALLFINALLY block we make sure that we are followed by a BBJ_CALLFINALLYRET block + // or that it's a BBF_RETLESS_CALL. + if (block->KindIs(BBJ_CALLFINALLY)) + { + assert(block->HasFlag(BBF_RETLESS_CALL) || block->isBBCallFinallyPair()); + } + + /* no un-compacted blocks */ + + if (fgCanCompactBlock(block)) + { + noway_assert(!"Found un-compacted blocks!"); + } + } +} + +#endif // DEBUG + #if DUMP_FLOWGRAPHS struct escapeMapping_t diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 70c82d4cd39af5..09d0c8819b68a1 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -4776,6 +4776,7 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh fgVerifyHandlerTab(); // Make sure that the predecessor lists are accurate fgDebugCheckBBlist(); + fgDebugCheckUpdate(); } #endif // DEBUG