From 7ccf49137b4ffdc15ce67a4267ffd5ba66fa7ea8 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 2 Dec 2024 07:41:04 -0800 Subject: [PATCH] JIT: enable removal of try/catch if the try can't throw. (#110273) If no tree in the try region of a try/catch can throw, then we can remove the try and delete the catch. If no tree in the try region of a try/finally can throw, we can remove the try and inline the finally. This slightly generalizes the empty-try/finally opt we have been doing for a long time. (We should do something similar for try/fault, but don't, yet). Since these optimization passes are cheap, and opportunities for them arise after other optimizations and unblock subsequent optimizations, run them early, middle, and late. Resolves #107191. I expect we'll see more of these cases in the future, say if we unblock cloning of loops with EH. --- src/coreclr/jit/compiler.cpp | 29 +- src/coreclr/jit/compiler.h | 5 +- src/coreclr/jit/compphases.h | 8 +- src/coreclr/jit/fgehopt.cpp | 540 +++++++++++++++++++++++++----- src/coreclr/jit/fgopt.cpp | 6 +- src/coreclr/jit/jitconfigvalues.h | 1 + src/coreclr/jit/valuenum.cpp | 6 +- 7 files changed, 501 insertions(+), 94 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 78888aef6a2e4..2ed11f5ff9e2c 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4637,10 +4637,14 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl DoPhase(this, PHASE_SWIFT_ERROR_RET, &Compiler::fgAddSwiftErrorReturns); #endif // SWIFT_SUPPORT - // Remove empty try regions + // Remove empty try regions (try/finally) // DoPhase(this, PHASE_EMPTY_TRY, &Compiler::fgRemoveEmptyTry); + // Remove empty try regions (try/catch) + // + DoPhase(this, PHASE_EMPTY_TRY_CATCH, &Compiler::fgRemoveEmptyTryCatch); + // Remove empty finally regions // DoPhase(this, PHASE_EMPTY_FINALLY, &Compiler::fgRemoveEmptyFinally); @@ -4810,6 +4814,18 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_UNROLL_LOOPS, &Compiler::optUnrollLoops); + // Try again to remove empty try finally/fault clauses + // + DoPhase(this, PHASE_EMPTY_FINALLY_2, &Compiler::fgRemoveEmptyFinally); + + // Remove empty try regions (try/finally) + // + DoPhase(this, PHASE_EMPTY_TRY_2, &Compiler::fgRemoveEmptyTry); + + // Remove empty try regions (try/catch) + // + DoPhase(this, PHASE_EMPTY_TRY_CATCH_2, &Compiler::fgRemoveEmptyTryCatch); + // Compute dominators and exceptional entry blocks // DoPhase(this, PHASE_COMPUTE_DOMINATORS, &Compiler::fgComputeDominators); @@ -5039,7 +5055,16 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl #endif // Try again to remove empty try finally/fault clauses - DoPhase(this, PHASE_EMPTY_FINALLY_2, &Compiler::fgRemoveEmptyFinally); + // + DoPhase(this, PHASE_EMPTY_FINALLY_3, &Compiler::fgRemoveEmptyFinally); + + // Remove empty try regions (try/finally) + // + DoPhase(this, PHASE_EMPTY_TRY_3, &Compiler::fgRemoveEmptyTry); + + // Remove empty try regions (try/catch) + // + DoPhase(this, PHASE_EMPTY_TRY_CATCH_3, &Compiler::fgRemoveEmptyTryCatch); if (UsesFunclets()) { diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 045e3eade03e4..6b7dc5ddfd808 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5270,7 +5270,6 @@ class Compiler bool fgModified = false; // True if the flow graph has been modified recently bool fgPredsComputed = false; // Have we computed the bbPreds list - bool fgOptimizedFinally = false; // Did we optimize any try-finallys? bool fgHasSwitch = false; // any BBJ_SWITCH jumps? @@ -5352,12 +5351,16 @@ class Compiler PhaseStatus fgRemoveEmptyTry(); + PhaseStatus fgRemoveEmptyTryCatch(); + PhaseStatus fgRemoveEmptyFinally(); PhaseStatus fgMergeFinallyChains(); PhaseStatus fgCloneFinally(); + void fgUpdateACDsBeforeEHTableEntryRemoval(unsigned XTnum); + void fgCleanupContinuation(BasicBlock* continuation); PhaseStatus fgTailMergeThrows(); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 75847d2242425..d5c703f66fc03 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -37,6 +37,7 @@ CompPhaseNameMacro(PHASE_MORPH_ADD_INTERNAL, "Morph - Add internal block CompPhaseNameMacro(PHASE_SWIFT_ERROR_RET, "Add Swift error returns", false, -1, true) CompPhaseNameMacro(PHASE_ALLOCATE_OBJECTS, "Allocate Objects", false, -1, false) CompPhaseNameMacro(PHASE_EMPTY_TRY, "Remove empty try", false, -1, false) +CompPhaseNameMacro(PHASE_EMPTY_TRY_CATCH, "Remove empty try/catch", false, -1, false) CompPhaseNameMacro(PHASE_EMPTY_FINALLY, "Remove empty finally", false, -1, false) CompPhaseNameMacro(PHASE_MERGE_FINALLY_CHAINS, "Merge callfinally chains", false, -1, false) CompPhaseNameMacro(PHASE_CLONE_FINALLY, "Clone finally", false, -1, false) @@ -74,6 +75,9 @@ CompPhaseNameMacro(PHASE_FIND_LOOPS, "Find loops", CompPhaseNameMacro(PHASE_CLONE_LOOPS, "Clone loops", false, -1, false) CompPhaseNameMacro(PHASE_UNROLL_LOOPS, "Unroll loops", false, -1, false) CompPhaseNameMacro(PHASE_MORPH_MDARR, "Morph array ops", false, -1, false) +CompPhaseNameMacro(PHASE_EMPTY_FINALLY_2, "Remove empty finally 2", false, -1, false) +CompPhaseNameMacro(PHASE_EMPTY_TRY_2, "Remove empty try 2", false, -1, false) +CompPhaseNameMacro(PHASE_EMPTY_TRY_CATCH_2, "Remove empty try-catch 2", false, -1, false) CompPhaseNameMacro(PHASE_HOIST_LOOP_CODE, "Hoist loop code", false, -1, false) CompPhaseNameMacro(PHASE_MARK_LOCAL_VARS, "Mark local vars", false, -1, false) CompPhaseNameMacro(PHASE_OPTIMIZE_BOOLS, "Optimize bools", false, -1, false) @@ -96,7 +100,9 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES, "Redundant branch opts", CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", false, -1, false) CompPhaseNameMacro(PHASE_IF_CONVERSION, "If conversion", false, -1, false) CompPhaseNameMacro(PHASE_VN_BASED_DEAD_STORE_REMOVAL,"VN-based dead store removal", false, -1, false) -CompPhaseNameMacro(PHASE_EMPTY_FINALLY_2, "Remove empty finally 2", false, -1, false) +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_3, "Remove empty try-catch 3", false, -1, false) CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH, "Update flow graph opt pass", false, -1, false) CompPhaseNameMacro(PHASE_STRESS_SPLIT_TREE, "Stress gtSplitTree", false, -1, false) CompPhaseNameMacro(PHASE_EXPAND_RTLOOKUPS, "Expand runtime lookups", false, -1, true) diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 4d1fe732234ea..bb59ddccf6ecc 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -225,87 +225,9 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() } } - // Likewise, look for ACDs within this try, and update their try regions. - // And look for ACDs within the now-removed handler, and remove them. + // Update any impacted ACDs. // - if (fgHasAddCodeDscMap()) - { - AddCodeDscMap* const map = fgGetAddCodeDscMap(); - for (AddCodeDsc* const add : AddCodeDscMap::ValueIteration(map)) - { - JITDUMP("Considering "); - JITDUMPEXEC(add->Dump()); - - // Remember the old lookup key - // - AddCodeDscKey oldKey(add); - - // If this ACD is in the handler we just removed, - // it must no longer be needed. - // - const bool inHnd = add->acdHndIndex > 0; - if (inHnd && ((unsigned)(add->acdHndIndex - 1) == XTnum)) - { - bool const removed = map->Remove(oldKey); - assert(removed); - JITDUMP("ACD%u was in EH#%u handler region: removing\n", add->acdNum, XTnum); - JITDUMPEXEC(add->Dump()); - continue; - } - - // If this ACD is NOT in the try we just removed, - // it is unaffected (though it may get updated - // when the EH region is removed). - // - bool inTry = add->acdTryIndex > 0; - - if (!inTry || ((unsigned)(add->acdTryIndex - 1) != XTnum)) - { - JITDUMP("ACD%u not affected\n", add->acdNum); - continue; - } - - // This ACD is in the try we just removed, - // update it to belong to the enclosing try region, - // which is the enclosing try of the associated handler. - // - // Then see if there is already an equivalent ACD in - // that region, and if so, "merge" this ACD into - // that one (by removing this ACD from the map). - // - // If there is no equivalent ACD, re-add this current ACD - // with an updated key. - // - add->acdTryIndex = firstBlock->bbTryIndex; - add->UpdateKeyDesignator(this); - - // Remove the ACD from the map via its old key - // - bool const removed = map->Remove(oldKey); - assert(removed); - - // Compute the new key an see if there's an existing - // ACD with that key. - // - AddCodeDscKey newKey(add); - AddCodeDsc* existing = nullptr; - if (map->Lookup(newKey, &existing)) - { - // If so, this ACD is now redundant - // - JITDUMP("ACD%u merged into ACD%u\n", add->acdNum, existing->acdNum); - JITDUMPEXEC(existing->Dump()); - } - else - { - // If not, re-enter this ACD in the map with the updated key - // - JITDUMP("ACD%u updated\n", add->acdNum); - map->Set(newKey, add); - JITDUMPEXEC(add->Dump()); - } - } - } + fgUpdateACDsBeforeEHTableEntryRemoval(XTnum); // Remove the try-finally EH region. This will compact the EH table // so XTnum now points at the next entry. @@ -321,7 +243,7 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() { JITDUMP("fgRemoveEmptyFinally() removed %u try-finally/fault clauses from %u finally/fault(s)\n", emptyCount, finallyCount); - fgOptimizedFinally = true; + fgInvalidateDfsTree(); #ifdef DEBUG if (verbose) @@ -339,7 +261,143 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() } //------------------------------------------------------------------------ -// fgRemoveEmptyTry: Optimize try/finallys where the try is empty +// fgUpdateACDsBeforeEHTableEntryRemoval: delete, move, or merge ACDs within +// an EH region we're about to remove. +// +// Arguments: +// XTNum -- eh region being removed +// +// Notes: +// XTnum must be the innermost mutual protect region, for a try-catch. +// +// We assume that the ACDs in the try/handler regions might still be needed +// (callers may "promote" these blocks to their enclosing regions). If the +// caller is actually removing the region code instead of merging it to the +// enclosing region, it is ok to have extra ACDs around. +// +// ACDs in filter regions are removed. +// +void Compiler::fgUpdateACDsBeforeEHTableEntryRemoval(unsigned XTnum) +{ + if (!fgHasAddCodeDscMap()) + { + // No ACDs to worry about at this time + // + return; + } + + EHblkDsc* const ebd = ehGetDsc(XTnum); + AddCodeDscMap* const map = fgGetAddCodeDscMap(); + for (AddCodeDsc* const add : AddCodeDscMap::ValueIteration(map)) + { + JITDUMP("Considering "); + JITDUMPEXEC(add->Dump()); + + // Remember the old lookup key + // + AddCodeDscKey oldKey(add); + + const bool inHnd = add->acdHndIndex > 0; + const bool inTry = add->acdTryIndex > 0; + const bool inThisHnd = + inHnd && ((unsigned)(add->acdHndIndex - 1) == XTnum) && (add->acdKeyDsg == AcdKeyDesignator::KD_HND); + const bool inThisFlt = + inHnd && ((unsigned)(add->acdHndIndex - 1) == XTnum) && (add->acdKeyDsg == AcdKeyDesignator::KD_FLT); + const bool inThisTry = + inTry && ((unsigned)(add->acdTryIndex - 1) == XTnum) && (add->acdKeyDsg == AcdKeyDesignator::KD_TRY); + + // If this ACD is in the filter of this region, it is no longer needed + // + if (inThisFlt) + { + bool const removed = map->Remove(oldKey); + assert(removed); + JITDUMP("ACD%u was in EH#%u filter region: removing\n", add->acdNum, XTnum); + JITDUMPEXEC(add->Dump()); + continue; + } + + // Note any ACDs in enclosed regions are updated when the region + // itself is removed. + // + if (!inThisTry && !inThisHnd) + { + JITDUMP("ACD%u not affected\n", add->acdNum); + continue; + } + + // If this ACD is in the handler of this region, update the + // enclosing handler index. + // + if (inThisHnd) + { + if (ebd->ebdEnclosingHndIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + add->acdHndIndex = 0; + } + else + { + add->acdHndIndex = ebd->ebdEnclosingHndIndex + 1; + } + } + + // If this ACD is in the try of this region, update the + // enclosing try index. + // + if (inThisTry) + { + if (ebd->ebdEnclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + add->acdTryIndex = 0; + } + else + { + add->acdTryIndex = ebd->ebdEnclosingTryIndex + 1; + } + } + + // Update the ACD key designator (note it may change). + // + // Then see if there is already an equivalent ACD in + // the new enclosing region, and if so, "merge" this ACD into + // that one (by removing this ACD from the map). + // + // If there is no equivalent ACD, re-add this current ACD + // with an updated key. + // + add->UpdateKeyDesignator(this); + + // Remove the ACD from the map via its old key + // + bool const removed = map->Remove(oldKey); + assert(removed); + + // Compute the new key an see if there's an existing + // ACD with that key. + // + AddCodeDscKey newKey(add); + AddCodeDsc* existing = nullptr; + if (map->Lookup(newKey, &existing)) + { + // If so, this ACD is now redundant + // + JITDUMP("ACD%u merged into ACD%u\n", add->acdNum, existing->acdNum); + JITDUMPEXEC(existing->Dump()); + } + else + { + // If not, re-enter this ACD in the map with the updated key + // + JITDUMP("ACD%u updated\n", add->acdNum); + map->Set(newKey, add); + JITDUMPEXEC(add->Dump()); + } + } +} + +//------------------------------------------------------------------------ +// fgRemoveEmptyTry: Optimize try/finallys where the try is empty, +// or cannot throw any exceptions // // Returns: // PhaseStatus indicating what, if anything, was changed. @@ -433,11 +491,34 @@ PhaseStatus Compiler::fgRemoveEmptyTry() assert(firstTryBlock->getTryIndex() == XTnum); + // Assume the try is empty + // + bool canThrow = false; + // Limit for now to trys that contain only a callfinally pair - // or branch to same. + // or branch to same (we check this later). So we only need + // check the first block. + // if (!firstTryBlock->isEmpty()) { - JITDUMP("EH#%u first try block " FMT_BB " not empty; skipping.\n", XTnum, firstTryBlock->bbNum); + // Walk statements to see if any can throw an exception. + // + for (Statement* const stmt : firstTryBlock->Statements()) + { + // Not clear when we can trust GTF_EXCEPT alone. + // GTF_CALL is too broad, but safe. + // + if ((stmt->GetRootNode()->gtFlags & (GTF_EXCEPT | GTF_CALL)) != 0) + { + canThrow = true; + break; + } + } + } + + if (canThrow) + { + JITDUMP("EH#%u first try block " FMT_BB " can throw exception; skipping.\n", XTnum, firstTryBlock->bbNum); XTnum++; continue; } @@ -637,15 +718,21 @@ PhaseStatus Compiler::fgRemoveEmptyTry() #endif // FEATURE_EH_WINDOWS_X86 } - // (6) Remove the try-finally EH region. This will compact the + // (6) Update any impacted ACDs. + // + fgUpdateACDsBeforeEHTableEntryRemoval(XTnum); + + // (7) Remove the try-finally EH region. This will compact the // EH table so XTnum now points at the next entry and will update // the EH region indices of any nested EH in the (former) handler. + // fgRemoveEHTableEntry(XTnum); - // (7) The handler entry has an artificial extra ref count. Remove it. + // (8) The handler entry has an artificial extra ref count. Remove it. // There also should be one normal ref, from the try, and the handler // may contain internal branches back to its start. So the ref count // should currently be at least 2. + // assert(firstHandlerBlock->bbRefs >= 2); firstHandlerBlock->bbRefs -= 1; @@ -659,7 +746,285 @@ PhaseStatus Compiler::fgRemoveEmptyTry() if (emptyCount > 0) { JITDUMP("fgRemoveEmptyTry() optimized %u empty-try try-finally clauses\n", emptyCount); - fgOptimizedFinally = true; + fgInvalidateDfsTree(); + return PhaseStatus::MODIFIED_EVERYTHING; + } + + return PhaseStatus::MODIFIED_NOTHING; +} + +//------------------------------------------------------------------------ +// fgRemoveEmptyTryCatch: Optimize try/catch where the try is empty, +// or cannot throw any exceptions +// +// Returns: +// PhaseStatus indicating what, if anything, was changed. +// +PhaseStatus Compiler::fgRemoveEmptyTryCatch() +{ + JITDUMP("\n*************** In fgRemoveEmptyTryCatch()\n"); + + // We need to do this transformation before funclets are created. + assert(!fgFuncletsCreated); + + bool enableRemoveEmptyTryCatch = true; + +#ifdef DEBUG + // Allow override to enable/disable. + enableRemoveEmptyTryCatch = (JitConfig.JitEnableRemoveEmptyTryCatch() == 1); +#endif // DEBUG + + if (!enableRemoveEmptyTryCatch) + { + JITDUMP("Empty try/catch removal disabled.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + + if (compHndBBtabCount == 0) + { + JITDUMP("No EH in this method, nothing to remove.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + + if (opts.MinOpts()) + { + JITDUMP("Method compiled with MinOpts, no removal.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + + if (opts.compDbgCode) + { + JITDUMP("Method compiled with debug codegen, no removal.\n"); + return PhaseStatus::MODIFIED_NOTHING; + } + +#ifdef DEBUG + if (verbose) + { + printf("\n*************** Before fgRemoveEmptyTryCatch()\n"); + fgDispBasicBlocks(); + fgDispHandlerTab(); + printf("\n"); + } +#endif // DEBUG + + // Look for try-catches where the try is empty. + unsigned emptyCount = 0; + unsigned XTnum = 0; + while (XTnum < compHndBBtabCount) + { + EHblkDsc* const HBtab = &compHndBBtab[XTnum]; + + // Check if this is a try/catch. + if (HBtab->HasFinallyOrFaultHandler()) + { + JITDUMP("EH#%u is not a try-catch; skipping.\n", XTnum); + XTnum++; + continue; + } + + // Examine the try region + // + BasicBlock* const firstTryBlock = HBtab->ebdTryBeg; + BasicBlock* const lastTryBlock = HBtab->ebdTryLast; + + // Assume the try is empty + // + bool canThrow = false; + + // Walk all blocks in the try. Since we are walking + // try regions inner/outer, if we find an enclosed + // try, we assume it must be able to throw. + // + for (BasicBlock* const tryBlock : Blocks(firstTryBlock, lastTryBlock)) + { + if (tryBlock->getTryIndex() != XTnum) + { + JITDUMP("EH#%u try block " FMT_BB " is nested try entry; skipping.\n", XTnum, tryBlock->bbNum); + canThrow = true; + break; + } + + // Walk statements to see if any can throw an exception. + // + for (Statement* const stmt : tryBlock->Statements()) + { + // Not clear when we can trust GTF_EXCEPT alone. + // GTF_CALL is perhaps too broad, but safe. + // + if ((stmt->GetRootNode()->gtFlags & (GTF_EXCEPT | GTF_CALL)) != 0) + { + JITDUMP("EH#%u " FMT_STMT " in " FMT_BB " can throw; skipping.\n", XTnum, stmt->GetID(), + tryBlock->bbNum); + canThrow = true; + break; + } + } + + if (canThrow) + { + break; + } + } + + if (canThrow) + { + // We could accelerate a bit by skipping to the first non-mutual protect region. + // + XTnum++; + continue; + } + + JITDUMP("EH#%u try has no statements that can throw\n", XTnum); + + // Since there are no tested trys, XTnum should be the try index of + // all blocks in the try region. + // + assert(firstTryBlock->getTryIndex() == XTnum); + assert(lastTryBlock->getTryIndex() == XTnum); + + // Examine the handler blocks. If we see an enclosed try, we bail out for now. + // We could handle this, with a bit more work. + // + BasicBlock* const firstHndBlock = HBtab->ebdHndBeg; + BasicBlock* const lastHndBlock = HBtab->ebdHndLast; + bool handlerEnclosesTry = false; + + for (BasicBlock* const handlerBlock : Blocks(firstHndBlock, lastHndBlock)) + { + if (bbIsTryBeg(handlerBlock)) + { + JITDUMP("EH#%u handler block " FMT_BB " is nested try entry; skipping.\n", XTnum, handlerBlock->bbNum); + handlerEnclosesTry = true; + break; + } + } + + if (handlerEnclosesTry) + { + // We could accelerate a bit by skipping to the first non-mutual protect region. + // + XTnum++; + continue; + } + + // Time to optimize. + // + unsigned const enclosingTryIndex = HBtab->ebdEnclosingTryIndex; + + // (1) Find enclosing try region for the try, if any, and + // update the try region for the blocks in the try. Note the + // handler region (if any) won't change. + // + for (BasicBlock* const tryBlock : Blocks(firstTryBlock, lastTryBlock)) + { + // Look for blocks directly contained in this try, and + // update the try region appropriately. + // + // The try region for blocks transitively contained (say in a + // child try) will get updated by the subsequent call to + // fgRemoveEHTableEntry. + // + if (tryBlock->getTryIndex() == XTnum) + { + if (enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + tryBlock->clearTryIndex(); + } + else + { + tryBlock->setTryIndex(enclosingTryIndex); + } + } + } + + // (2) Remove any filter blocks + // The first filter block has an artificial ref count + // + if (HBtab->HasFilter()) + { + BasicBlock* const firstFltBlock = HBtab->ebdFilter; + assert(firstFltBlock->bbRefs == 1); + firstFltBlock->bbRefs = 0; + BasicBlock* const afterLastFltBlock = HBtab->BBFilterLast()->Next(); + + // Must do this in two passes to handle loops or lexically + // backwards references. + // + for (BasicBlock* filterBlock = firstFltBlock; filterBlock != afterLastFltBlock; + filterBlock = filterBlock->Next()) + { + fgRemoveBlockAsPred(filterBlock); + filterBlock->SetKind(BBJ_THROW); + } + + for (BasicBlock* filterBlock = firstFltBlock; filterBlock != afterLastFltBlock; + filterBlock = filterBlock->Next()) + { + filterBlock->RemoveFlags(BBF_DONT_REMOVE); + fgRemoveBlock(filterBlock, /* unreachable */ true); + } + } + + // (3) Remove any handler blocks. + // The first handler block has an artificial ref count + // + assert(firstHndBlock->bbRefs == 1); + firstHndBlock->bbRefs = 0; + BasicBlock* const afterLastHndBlock = lastHndBlock->Next(); + + // Must do this in two passes to handle loops or lexically + // backwards references. + // + for (BasicBlock* handlerBlock = firstHndBlock; handlerBlock != afterLastHndBlock; + handlerBlock = handlerBlock->Next()) + { + assert(!bbIsTryBeg(handlerBlock)); + + // It's possible to see a callfinally pair in a catch, and if so + // there may be a pred edge into the pair tail from outside the catch. + // Handle this specially. + // + if (handlerBlock->isBBCallFinallyPair()) + { + BasicBlock* const tailBlock = handlerBlock->Next(); + fgPrepareCallFinallyRetForRemoval(tailBlock); + } + + fgRemoveBlockAsPred(handlerBlock); + handlerBlock->SetKind(BBJ_THROW); + } + + for (BasicBlock* handlerBlock = firstHndBlock; handlerBlock != afterLastHndBlock; + handlerBlock = handlerBlock->Next()) + { + assert(!bbIsTryBeg(handlerBlock)); + handlerBlock->RemoveFlags(BBF_DONT_REMOVE); + fgRemoveBlock(handlerBlock, /* unreachable */ true); + } + + // (4) Update any impacted ACDs. + // + fgUpdateACDsBeforeEHTableEntryRemoval(XTnum); + + // (5) Remove the try-catch EH region. This will compact the + // EH table so XTnum now points at the next entry and will update + // the EH region indices of any nested EH blocks. + // + fgRemoveEHTableEntry(XTnum); + + // (6) The old try entry no longer needs special protection. + // + firstTryBlock->RemoveFlags(BBF_DONT_REMOVE); + + // Another one bites the dust... + emptyCount++; + } + + if (emptyCount > 0) + { + JITDUMP("fgRemoveEmptyTryCatch() optimized %u empty-try catch clauses\n", emptyCount); + fgInvalidateDfsTree(); return PhaseStatus::MODIFIED_EVERYTHING; } @@ -1294,7 +1659,6 @@ PhaseStatus Compiler::fgCloneFinally() if (cloneCount > 0) { JITDUMP("fgCloneFinally() cloned %u finally handlers\n", cloneCount); - fgOptimizedFinally = true; #ifdef DEBUG if (verbose) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index a11b3c9f63ba4..8176daab56a5b 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -170,7 +170,11 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock) // PhaseStatus Compiler::fgComputeDominators() { - assert(m_dfsTree != nullptr); + if (m_dfsTree == nullptr) + { + m_dfsTree = fgComputeDfs(); + } + if (m_domTree == nullptr) { m_domTree = FlowGraphDominatorTree::Build(m_dfsTree); diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index faa9b0e7e45bf..7eb36d7be3340 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -665,6 +665,7 @@ RELEASE_CONFIG_INTEGER(JitEECallTimingInfo, "JitEECallTimingInfo", 0) CONFIG_INTEGER(JitEnableFinallyCloning, "JitEnableFinallyCloning", 1) CONFIG_INTEGER(JitEnableRemoveEmptyTry, "JitEnableRemoveEmptyTry", 1) +CONFIG_INTEGER(JitEnableRemoveEmptyTryCatch, "JitEnableRemoveEmptyTryCatch", 1) // Overall master enable for Guarded Devirtualization. RELEASE_CONFIG_INTEGER(JitEnableGuardedDevirtualization, "JitEnableGuardedDevirtualization", 1) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 01a177e6aa236..60ac86cf8e54e 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -10807,7 +10807,11 @@ PhaseStatus Compiler::fgValueNumber() } assert(m_dfsTree != nullptr); - assert(m_loops != nullptr); + + if (m_loops == nullptr) + { + m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + } m_blockToLoop = BlockToNaturalLoopMap::Build(m_loops); // Compute the side effects of loops.