From ca708ac239aabbcbb44cd5170de6444c10947978 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 19 Jun 2026 17:22:24 -0700 Subject: [PATCH 1/2] JIT: add fgRemoveUnreachableTry phase Removes EH regions whose try entry has become unreachable. Run after each empty-EH-cleanup pass. Fixes an issue for wasm control flow, which is not tolerant of unreachable EH. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/compiler.cpp | 4 + src/coreclr/jit/compiler.h | 2 + src/coreclr/jit/compphases.h | 1 + src/coreclr/jit/fgehopt.cpp | 227 ++++++++++++++++++++++++++++++ src/coreclr/jit/jitconfigvalues.h | 1 + 5 files changed, 235 insertions(+) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 41a6ff91c2cc0d..a51ffc5299aace 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4936,6 +4936,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_EMPTY_TRY_CATCH_FAULT_3, &Compiler::fgRemoveEmptyTryCatchOrTryFault); + // Remove unreachable try regions + // + DoPhase(this, PHASE_REMOVE_UNREACHABLE_TRY, &Compiler::fgRemoveUnreachableTry); + // Create funclets from the EH handlers. // DoPhase(this, PHASE_CREATE_FUNCLETS, &Compiler::fgCreateFunclets); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 968a976d2846be..5bc490df9b5998 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6001,6 +6001,8 @@ class Compiler PhaseStatus fgRemoveEmptyFinally(); + PhaseStatus fgRemoveUnreachableTry(); + PhaseStatus fgMergeFinallyChains(); PhaseStatus fgCloneFinally(); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 7ed0edd5d57dd5..17f52494fb8181 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -111,6 +111,7 @@ CompPhaseNameMacro(PHASE_VN_BASED_DEAD_STORE_REMOVAL,"VN-based dead store remova CompPhaseNameMacro(PHASE_EMPTY_FINALLY_3, "Remove empty finally 3", false, -1, false) CompPhaseNameMacro(PHASE_EMPTY_TRY_3, "Remove empty try 3", false, -1, false) CompPhaseNameMacro(PHASE_EMPTY_TRY_CATCH_FAULT_3, "Remove empty try-catch-fault 3", false, -1, false) +CompPhaseNameMacro(PHASE_REMOVE_UNREACHABLE_TRY, "Remove unreachable try", false, -1, false) CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH, "Update flow graph opt pass", false, -1, false) CompPhaseNameMacro(PHASE_OPT_DFS_BLOCKS, "Remove unreachable blocks", false, -1, false) CompPhaseNameMacro(PHASE_STRESS_SPLIT_TREE, "Stress gtSplitTree", false, -1, false) diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 2897b888f659bc..6ad382fe2fd878 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -1809,6 +1809,233 @@ void Compiler::fgDebugCheckTryFinallyExits() #endif // DEBUG +//------------------------------------------------------------------------ +// fgRemoveUnreachableTry: Remove EH regions whose try entry is not +// reachable from the method entry. +// +// Returns: +// PhaseStatus indicating what, if anything, was changed. +// +// Notes: +// Walks the EH table; for each entry whose try entry is not in the DFS +// reachable set, clears the artificial bbRefs and BBF_DONT_REMOVE on the +// try/filter/handler entry blocks so they become candidates for the normal +// dead-block removal pass (fgRemoveBlocksOutsideDfsTree). After that runs, +// drops the EH table entries whose blocks have been removed. +// +// The standard DFS is already EH-aware (AllSuccessorEnumerator visits +// EH successors), so try-reachability implies filter/handler/finally +// reachability and a simple m_dfsTree->Contains(ebdTryBeg) check is +// sufficient. +// +PhaseStatus Compiler::fgRemoveUnreachableTry() +{ + JITDUMP("\n*************** In fgRemoveUnreachableTry()\n"); + + assert(!fgFuncletsCreated); + assert(fgPredsComputed); + + bool enabled = true; +#ifdef DEBUG + enabled = (JitConfig.JitEnableRemoveUnreachableTry() == 1); +#endif + + if (!enabled) + { + JITDUMP("Unreachable try removal disabled by config.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + if (compHndBBtabCount == 0) + { + JITDUMP("No EH in this method; nothing to do.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + if (opts.MinOpts()) + { + JITDUMP("Method compiled with MinOpts; skipping.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + if (opts.compDbgCode) + { + JITDUMP("Method compiled with debug codegen; skipping.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + + // Compute DFS if the caller didn't supply one; the EH-aware DFS makes + // try-reachability imply handler/filter reachability for the common case. + // + bool ownsDfs = false; + if (m_dfsTree == nullptr) + { + m_dfsTree = fgComputeDfs(); + ownsDfs = true; + } + + // PASS 1: For each dead-try region, retarget any callfinally pairs that + // target the handler so the now-dead handler has no live preds. + // + bool foundDead = false; + + for (unsigned XTnum = 0; XTnum < compHndBBtabCount; XTnum++) + { + EHblkDsc* const HBtab = &compHndBBtab[XTnum]; + BasicBlock* const tryBeg = HBtab->ebdTryBeg; + BasicBlock* const hndBeg = HBtab->ebdHndBeg; + + if (m_dfsTree->Contains(tryBeg)) + { + continue; + } + + foundDead = true; + JITDUMP("EH#%u try entry " FMT_BB " is unreachable.\n", XTnum, tryBeg->bbNum); + + // Filters are only reached via EH-succ from try blocks, so an + // unreachable try implies an unreachable filter. + // + if (HBtab->HasFilter()) + { + assert(!m_dfsTree->Contains(HBtab->ebdFilter)); + } + + if (HBtab->HasFinallyHandler()) + { + for (BasicBlock* const pred : hndBeg->PredBlocksEditing()) + { + if (!pred->KindIs(BBJ_CALLFINALLY) || !pred->isBBCallFinallyPair()) + { + continue; + } + + BasicBlock* const tail = pred->Next(); + BasicBlock* const continuation = tail->GetFinallyContinuation(); + + JITDUMP(" retargeting callfinally " FMT_BB " to continuation " FMT_BB ", removing tail " FMT_BB "\n", + pred->bbNum, continuation->bbNum, tail->bbNum); + + fgPrepareCallFinallyRetForRemoval(tail); + fgRemoveBlock(tail, /* unreachable */ true); + + fgRemoveRefPred(pred->GetTargetEdge()); + FlowEdge* const newEdge = fgAddRefPred(continuation, pred); + pred->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge); + pred->RemoveFlags(BBF_RETLESS_CALL); + + // Bypassing the finally body invalidates the local profile. + // + if (pred->hasProfileWeight()) + { + fgPgoConsistent = false; + } + } + } + + // Non-finally handlers can stay DFS-reachable via two-pass EH + // dispatch from an outer filter; PASS 3's EH-entry removal + PASS 4's + // DFS rebuild will make them unreachable. + } + + if (!foundDead) + { + JITDUMP("No unreachable EH regions found.\n"); + if (ownsDfs) + { + fgInvalidateDfsTree(); + } + return PhaseStatus::MODIFIED_NOTHING; + } + + // PASS 2: Unprotect blocks in dead regions and drop the artificial + // bbRefs on handler/filter entries so the dead-block pass can remove them. + // + for (unsigned XTnum = 0; XTnum < compHndBBtabCount; XTnum++) + { + EHblkDsc* const HBtab = &compHndBBtab[XTnum]; + if (m_dfsTree->Contains(HBtab->ebdTryBeg)) + { + continue; + } + + for (BasicBlock* const block : Blocks(HBtab->ebdTryBeg, HBtab->ebdTryLast)) + { + block->RemoveFlags(BBF_DONT_REMOVE); + } + for (BasicBlock* const block : Blocks(HBtab->ebdHndBeg, HBtab->ebdHndLast)) + { + block->RemoveFlags(BBF_DONT_REMOVE); + } + if (HBtab->HasFilter()) + { + for (BasicBlock* const block : Blocks(HBtab->ebdFilter, HBtab->BBFilterLast())) + { + block->RemoveFlags(BBF_DONT_REMOVE); + } + } + + assert(HBtab->ebdHndBeg->bbRefs >= 1); + HBtab->ebdHndBeg->bbRefs--; + + if (HBtab->HasFilter()) + { + assert(HBtab->ebdFilter->bbRefs >= 1); + HBtab->ebdFilter->bbRefs--; + } + } + + // PASS 3: Drop the EH table entries for the dead regions, clearing the + // try/hnd index on any block still claimed by them so fgRemoveEHTableEntry + // is satisfied. + // + unsigned removedCount = 0; + for (unsigned XTnum = 0; XTnum < compHndBBtabCount;) + { + EHblkDsc* const HBtab = &compHndBBtab[XTnum]; + if (m_dfsTree->Contains(HBtab->ebdTryBeg)) + { + XTnum++; + continue; + } + + for (BasicBlock* const block : Blocks()) + { + if (block->hasTryIndex() && block->getTryIndex() == XTnum) + { + block->clearTryIndex(); + } + if (block->hasHndIndex() && block->getHndIndex() == XTnum) + { + block->clearHndIndex(); + } + } + + JITDUMP("Dropping EH#%u (try entry " FMT_BB ")\n", XTnum, HBtab->ebdTryBeg->bbNum); + fgUpdateACDsBeforeEHTableEntryRemoval(XTnum); + fgRemoveEHTableEntry(XTnum); + removedCount++; + } + + // PASS 4: Rebuild the DFS; PASS 1 and PASS 3 both invalidate it. + // + fgInvalidateDfsTree(); + m_dfsTree = fgComputeDfs(); + + // PASS 5: Remove the now-unreachable blocks. + // + fgRemoveBlocksOutsideDfsTree(); + + // Leave the DFS in the same state we found it: invalidate if we computed + // it ourselves, otherwise leave a fresh one for the caller. + // + fgInvalidateDfsTree(); + if (!ownsDfs) + { + m_dfsTree = fgComputeDfs(); + } + + JITDUMP("\nfgRemoveUnreachableTry removed %u unreachable EH region(s)\n", removedCount); + return PhaseStatus::MODIFIED_EVERYTHING; +} + //------------------------------------------------------------------------ // fgMergeFinallyChains: tail merge finally invocations // diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index b5dfbe9ecc4f4d..1cc7e8aec86497 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -725,6 +725,7 @@ RELEASE_CONFIG_INTEGER(JitEECallTimingInfo, "JitEECallTimingInfo", 0) CONFIG_INTEGER(JitEnableFinallyCloning, "JitEnableFinallyCloning", 1) CONFIG_INTEGER(JitEnableRemoveEmptyTry, "JitEnableRemoveEmptyTry", 1) CONFIG_INTEGER(JitEnableRemoveEmptyTryCatchOrTryFault, "JitEnableRemoveEmptyTryCatchOrTryFault", 1) +CONFIG_INTEGER(JitEnableRemoveUnreachableTry, "JitEnableRemoveUnreachableTry", 1) // Overall master enable for Guarded Devirtualization. RELEASE_CONFIG_INTEGER(JitEnableGuardedDevirtualization, "JitEnableGuardedDevirtualization", 1) From 73d608147146085dfd8cc9d23dfa064515746221 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Sat, 20 Jun 2026 08:12:58 -0700 Subject: [PATCH 2/2] JIT: fix fgRemoveUnreachableTry doc comment Update the Notes section to describe the actual PASS ordering: EH table entries are dropped first, then the DFS is rebuilt and blocks are removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/fgehopt.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 6ad382fe2fd878..8a952f64311919 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -1818,15 +1818,17 @@ void Compiler::fgDebugCheckTryFinallyExits() // // Notes: // Walks the EH table; for each entry whose try entry is not in the DFS -// reachable set, clears the artificial bbRefs and BBF_DONT_REMOVE on the -// try/filter/handler entry blocks so they become candidates for the normal -// dead-block removal pass (fgRemoveBlocksOutsideDfsTree). After that runs, -// drops the EH table entries whose blocks have been removed. +// reachable set: retargets any callfinally pairs that target the handler +// to its continuation, unprotects every block in the try/filter/handler, +// clears stale tryIndex/hndIndex on those blocks, and drops the EH table +// entry. After all dead entries are dropped, recomputes the DFS and runs +// fgRemoveBlocksOutsideDfsTree to delete the now-unreachable blocks. // -// The standard DFS is already EH-aware (AllSuccessorEnumerator visits -// EH successors), so try-reachability implies filter/handler/finally -// reachability and a simple m_dfsTree->Contains(ebdTryBeg) check is -// sufficient. +// The standard DFS is EH-aware (AllSuccessorEnumerator visits EH +// successors), so try-reachability normally implies handler/filter +// reachability; the callfinally retargeting handles the case where +// finally-cloning has left live callfinally pairs into an otherwise-dead +// handler. // PhaseStatus Compiler::fgRemoveUnreachableTry() {