diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index d24e360eb82c47..acfebd81b21c01 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -613,8 +613,28 @@ void BasicBlock::dspJumpKind() switch (bbJumpKind) { case BBJ_EHFINALLYRET: + { + printf(" ->"); + + // Early in compilation, we display the jump kind before the BBJ_EHFINALLYRET successors have been set. + if (bbJumpEhf == nullptr) + { + printf(" ????"); + } + else + { + const unsigned jumpCnt = bbJumpEhf->bbeCount; + BasicBlock** const jumpTab = bbJumpEhf->bbeSuccs; + + for (unsigned i = 0; i < jumpCnt; i++) + { + printf("%c" FMT_BB, (i == 0) ? ' ' : ',', jumpTab[i]->bbNum); + } + } + printf(" (finret)"); break; + } case BBJ_EHFAULTRET: printf(" (falret)"); @@ -1126,6 +1146,7 @@ BasicBlock* BasicBlock::GetSucc(unsigned i) const else { assert(i == 1); + assert(bbNext != bbJumpDest); return bbJumpDest; } @@ -1164,7 +1185,15 @@ unsigned BasicBlock::NumSucc(Compiler* comp) { return 0; } - return comp->fgNSuccsOfFinallyRet(this); + + // We may call this before we've computed the BBJ_EHFINALLYRET successors in the importer. Tolerate. + // + if (bbJumpEhf == nullptr) + { + return 0; + } + + return bbJumpEhf->bbeCount; case BBJ_CALLFINALLY: case BBJ_ALWAYS: @@ -1213,15 +1242,14 @@ BasicBlock* BasicBlock::GetSucc(unsigned i, Compiler* comp) switch (bbJumpKind) { case BBJ_EHFILTERRET: - { // Handler is the (sole) normal successor of the filter. assert(comp->fgFirstBlockOfHandler(this) == bbJumpDest); return bbJumpDest; - } case BBJ_EHFINALLYRET: - // Note: the following call is expensive. - return comp->fgSuccOfFinallyRet(this, i); + assert(bbJumpEhf != nullptr); + assert(i < bbJumpEhf->bbeCount); + return bbJumpEhf->bbeSuccs[i]; case BBJ_CALLFINALLY: case BBJ_ALWAYS: @@ -1240,6 +1268,7 @@ BasicBlock* BasicBlock::GetSucc(unsigned i, Compiler* comp) else { assert(i == 1); + assert(bbNext != bbJumpDest); return bbJumpDest; } @@ -1645,6 +1674,24 @@ BBswtDesc::BBswtDesc(Compiler* comp, const BBswtDesc* other) } } +//------------------------------------------------------------------------ +// BBehfDesc copy ctor: copy a EHFINALLYRET descriptor +// +// Arguments: +// comp - compiler instance +// other - existing descriptor to copy +// +BBehfDesc::BBehfDesc(Compiler* comp, const BBehfDesc* other) : bbeCount(other->bbeCount) +{ + // Allocate and fill in a new dst tab + // + bbeSuccs = new (comp, CMK_BasicBlock) BasicBlock*[bbeCount]; + for (unsigned i = 0; i < bbeCount; i++) + { + bbeSuccs[i] = other->bbeSuccs[i]; + } +} + //------------------------------------------------------------------------ // unmarkLoopAlign: Unmarks the LOOP_ALIGN flag from the block and reduce the // loop alignment count. diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index fb48792ca856c6..4d9da601e08236 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -104,6 +104,7 @@ struct BasicBlockList; struct FlowEdge; struct EHblkDsc; struct BBswtDesc; +struct BBehfDesc; struct StackEntry { @@ -355,6 +356,20 @@ class BBSwitchTargetList BBArrayIterator end() const; }; +// BBEhfSuccList: adapter class for forward iteration of BBJ_EHFINALLYRET blocks, using range-based `for`, +// normally used via BasicBlock::EHFinallyRetSuccs(), e.g.: +// for (BasicBlock* const succ : block->EHFinallyRetSuccs()) ... +// +class BBEhfSuccList +{ + BBehfDesc* m_bbeDesc; + +public: + BBEhfSuccList(BBehfDesc* bbeDesc); + BBArrayIterator begin() const; + BBArrayIterator end() const; +}; + //------------------------------------------------------------------------ // BasicBlockFlags: a bitmask of flags for BasicBlock // @@ -519,6 +534,7 @@ struct BasicBlock : private LIR::Range unsigned bbJumpOffs; // PC offset (temporary only) BasicBlock* bbJumpDest; // basic block BBswtDesc* bbJumpSwt; // switch descriptor + BBehfDesc* bbJumpEhf; // BBJ_EHFINALLYRET descriptor }; public: @@ -646,6 +662,26 @@ struct BasicBlock : private LIR::Range bbJumpSwt = jumpSwt; } + BBehfDesc* GetJumpEhf() const + { + assert(KindIs(BBJ_EHFINALLYRET)); + return bbJumpEhf; + } + + void SetJumpEhf(BBehfDesc* jumpEhf) + { + assert(KindIs(BBJ_EHFINALLYRET)); + bbJumpEhf = jumpEhf; + } + + void SetJumpKindAndTarget(BBjumpKinds jumpKind, BBehfDesc* jumpEhf) + { + assert(jumpKind == BBJ_EHFINALLYRET); + assert(jumpEhf != nullptr); + bbJumpKind = jumpKind; + bbJumpEhf = jumpEhf; + } + BasicBlockFlags bbFlags; static_assert_no_msg((BBF_SPLIT_NONEXIST & BBF_SPLIT_LOST) == 0); @@ -856,12 +892,7 @@ struct BasicBlock : private LIR::Range // (3) BBJ_SWITCH // // For BBJ_EHFINALLYRET, if no Compiler* is passed, then the block is considered to have no - // successor. If Compiler* is passed, we figure out the actual successors. Some cases will want one behavior, - // other cases the other. For example, IL verification requires that these blocks end in an empty operand - // stack, and since the dataflow analysis of IL verification is concerned only with the contents of the - // operand stack, we can consider the finally block to have no successors. But a more general dataflow - // analysis that is tracking the contents of local variables might want to consider *all* successors, - // and would pass the current Compiler object. + // successor. If Compiler* is passed, we use the actual successors. // // Similarly, BBJ_EHFILTERRET blocks are assumed to have no successors if Compiler* is not passed; if // Compiler* is passed, NumSucc/GetSucc yields the first block of the try block's handler. @@ -869,9 +900,9 @@ struct BasicBlock : private LIR::Range // For BBJ_SWITCH, if Compiler* is not passed, then all switch successors are returned. If Compiler* // is passed, then only unique switch successors are returned; the duplicate successors are omitted. // - // Note that for BBJ_COND, which has two successors (fall through and condition true branch target), - // only the unique targets are returned. Thus, if both targets are the same, NumSucc() will only return 1 - // instead of 2. + // Note that for BBJ_COND, which has two successors (fall through (condition false), and condition true + // branch target), only the unique targets are returned. Thus, if both targets are the same, NumSucc() + // will only return 1 instead of 2. // // NumSucc: Returns the number of successors of "this". unsigned NumSucc() const; @@ -881,7 +912,7 @@ struct BasicBlock : private LIR::Range BasicBlock* GetSucc(unsigned i) const; BasicBlock* GetSucc(unsigned i, Compiler* comp); - // SwitchTargets: convenience methods for enabling range-based `for` iteration over a switch block's targets, e.g.: + // SwitchTargets: convenience method for enabling range-based `for` iteration over a switch block's targets, e.g.: // for (BasicBlock* const bTarget : block->SwitchTargets()) ... // BBSwitchTargetList SwitchTargets() const @@ -890,6 +921,16 @@ struct BasicBlock : private LIR::Range return BBSwitchTargetList(bbJumpSwt); } + // EHFinallyRetSuccs: convenience method for enabling range-based `for` iteration over BBJ_EHFINALLYRET block + // successors, e.g.: + // for (BasicBlock* const succ : block->EHFinallyRetSuccs()) ... + // + BBEhfSuccList EHFinallyRetSuccs() const + { + assert(bbJumpKind == BBJ_EHFINALLYRET); + return BBEhfSuccList(bbJumpEhf); + } + BasicBlock* GetUniquePred(Compiler* comp) const; BasicBlock* GetUniqueSucc() const; @@ -1668,6 +1709,39 @@ inline BBArrayIterator BBSwitchTargetList::end() const return BBArrayIterator(m_bbsDesc->bbsDstTab + m_bbsDesc->bbsCount); } +// BBehfDesc -- descriptor for a BBJ_EHFINALLYRET block +// +struct BBehfDesc +{ + BasicBlock** bbeSuccs; // array of `BasicBlock*` pointing to BBJ_EHFINALLYRET block successors + unsigned bbeCount; // size of `bbeSuccs` array + + BBehfDesc() : bbeSuccs(nullptr), bbeCount(0) + { + } + + BBehfDesc(Compiler* comp, const BBehfDesc* other); +}; + +// BBEhfSuccList out-of-class-declaration implementations (here due to C++ ordering requirements). +// + +inline BBEhfSuccList::BBEhfSuccList(BBehfDesc* bbeDesc) : m_bbeDesc(bbeDesc) +{ + assert(m_bbeDesc != nullptr); + assert((m_bbeDesc->bbeSuccs != nullptr) || (m_bbeDesc->bbeCount == 0)); +} + +inline BBArrayIterator BBEhfSuccList::begin() const +{ + return BBArrayIterator(m_bbeDesc->bbeSuccs); +} + +inline BBArrayIterator BBEhfSuccList::end() const +{ + return BBArrayIterator(m_bbeDesc->bbeSuccs + m_bbeDesc->bbeCount); +} + // BBSuccList out-of-class-declaration implementations // inline BasicBlock::BBSuccList::BBSuccList(const BasicBlock* block) diff --git a/src/coreclr/jit/clrjit.natvis b/src/coreclr/jit/clrjit.natvis index 81e1503de22cf7..fb0f2c42dafce5 100644 --- a/src/coreclr/jit/clrjit.natvis +++ b/src/coreclr/jit/clrjit.natvis @@ -23,6 +23,7 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u BB{bbNum,d}->BB{bbJumpDest->bbNum,d}; {bbJumpKind,en} BB{bbNum,d}; {bbJumpKind,en}; {bbJumpSwt->bbsCount} cases + BB{bbNum,d}; {bbJumpKind,en}; {bbJumpEhf->bbeCount} succs BB{bbNum,d}; {bbJumpKind,en} diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index a8a15587e19434..a7d7f616452d28 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2322,11 +2322,11 @@ class Compiler // lives in a filter.) unsigned ehGetCallFinallyRegionIndex(unsigned finallyIndex, bool* inTryRegion); - // Find the range of basic blocks in which all BBJ_CALLFINALLY will be found that target the 'finallyIndex' region's - // handler. Set begBlk to the first block, and endBlk to the block after the last block of the range - // (nullptr if the last block is the last block in the program). + // Find the range of basic blocks in which all BBJ_CALLFINALLY will be found that target the 'finallyIndex' + // region's handler. Set `firstBlock` to the first block, and `lastBlock` to the last block of the range + // (the range is inclusive of `firstBlock` and `lastBlock`). Thus, the range is [firstBlock .. lastBlock]. // Precondition: 'finallyIndex' is the EH region of a try/finally clause. - void ehGetCallFinallyBlockRange(unsigned finallyIndex, BasicBlock** begBlk, BasicBlock** endBlk); + void ehGetCallFinallyBlockRange(unsigned finallyIndex, BasicBlock** firstBlock, BasicBlock** lastBlock); #ifdef DEBUG // Given a BBJ_CALLFINALLY block and the EH region index of the finally it is calling, return @@ -5382,21 +5382,6 @@ class Compiler PhaseStatus fgInsertGCPolls(); BasicBlock* fgCreateGCPoll(GCPollType pollType, BasicBlock* block); - // Requires that "block" is a block that returns from - // a finally. Returns the number of successors (jump targets of - // of blocks in the covered "try" that did a "LEAVE".) - unsigned fgNSuccsOfFinallyRet(BasicBlock* block); - - // Requires that "block" is a block that returns (in the sense of BBJ_EHFINALLYRET) from - // a finally. Returns its "i"th successor (jump targets of - // of blocks in the covered "try" that did a "LEAVE".) - // Requires that "i" < fgNSuccsOfFinallyRet(block). - BasicBlock* fgSuccOfFinallyRet(BasicBlock* block, unsigned i); - -private: - // Factor out common portions of the impls of the methods above. - void fgSuccOfFinallyRetWork(BasicBlock* block, unsigned i, BasicBlock** bres, unsigned* nres); - public: // For many purposes, it is desirable to be able to enumerate the *distinct* targets of a switch statement, // skipping duplicate targets. (E.g., in flow analyses that are only interested in the set of possible targets.) @@ -5472,6 +5457,12 @@ class Compiler void fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* newTarget, BasicBlock* oldTarget); + void fgChangeEhfBlock(BasicBlock* oldBlock, BasicBlock* newBlock); + + void fgReplaceEhfSuccessor(BasicBlock* block, BasicBlock* oldSucc, BasicBlock* newSucc); + + void fgRemoveEhfSuccessor(BasicBlock* block, BasicBlock* succ); + void fgReplaceJumpTarget(BasicBlock* block, BasicBlock* newTarget, BasicBlock* oldTarget); void fgReplacePred(BasicBlock* block, BasicBlock* oldPred, BasicBlock* newPred); @@ -5610,8 +5601,7 @@ class Compiler void fgRemoveReturnBlock(BasicBlock* block); - /* Helper code that has been factored out */ - inline void fgConvertBBToThrowBB(BasicBlock* block); + void fgConvertBBToThrowBB(BasicBlock* block); bool fgCastNeeded(GenTree* tree, var_types toType); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index a3bed975f9dfaf..2bfc18ba2da9e2 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -623,43 +623,19 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func) break; case BBJ_EHFINALLYRET: - { - EHblkDsc* ehDsc = comp->ehGetDsc(getHndIndex()); - assert(ehDsc->HasFinallyHandler()); - - BasicBlock* begBlk; - BasicBlock* endBlk; - comp->ehGetCallFinallyBlockRange(getHndIndex(), &begBlk, &endBlk); - - BasicBlock* finBeg = ehDsc->ebdHndBeg; - - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) + for (unsigned i = 0; i < bbJumpEhf->bbeCount; i++) { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) - { - continue; - } - - assert(bcall->isBBCallAlwaysPair()); - - RETURN_ON_ABORT(func(bcall->Next())); + RETURN_ON_ABORT(func(bbJumpEhf->bbeSuccs[i])); } RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) + for (unsigned i = 0; i < bbJumpEhf->bbeCount; i++) { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) - { - continue; - } - - assert(bcall->isBBCallAlwaysPair()); - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bcall->Next(), func)); + RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbJumpEhf->bbeSuccs[i], func)); } break; - } case BBJ_CALLFINALLY: case BBJ_EHCATCHRET: @@ -757,30 +733,11 @@ BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func) break; case BBJ_EHFINALLYRET: - { - EHblkDsc* ehDsc = comp->ehGetDsc(getHndIndex()); - assert(ehDsc->HasFinallyHandler()); - - BasicBlock* begBlk; - BasicBlock* endBlk; - comp->ehGetCallFinallyBlockRange(getHndIndex(), &begBlk, &endBlk); - - BasicBlock* finBeg = ehDsc->ebdHndBeg; - - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) + for (unsigned i = 0; i < bbJumpEhf->bbeCount; i++) { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) - { - continue; - } - - assert(bcall->isBBCallAlwaysPair()); - - RETURN_ON_ABORT(func(bcall->Next())); + RETURN_ON_ABORT(func(bbJumpEhf->bbeSuccs[i])); } - break; - } case BBJ_CALLFINALLY: case BBJ_EHCATCHRET: @@ -3207,48 +3164,6 @@ inline unsigned Compiler::fgThrowHlpBlkStkLevel(BasicBlock* block) #endif // !FEATURE_FIXED_OUT_ARGS -/* - Small inline function to change a given block to a throw block. - -*/ -inline void Compiler::fgConvertBBToThrowBB(BasicBlock* block) -{ - JITDUMP("Converting " FMT_BB " to BBJ_THROW\n", block->bbNum); - assert(fgPredsComputed); - - // Ordering of the following operations matters. - // First, note if we are looking at the first block of a call always pair. - const bool isCallAlwaysPair = block->isBBCallAlwaysPair(); - - // Scrub this block from the pred lists of any successors - fgRemoveBlockAsPred(block); - - // Update jump kind after the scrub. - block->SetJumpKind(BBJ_THROW DEBUG_ARG(this)); - - // Any block with a throw is rare - block->bbSetRunRarely(); - - // If we've converted a BBJ_CALLFINALLY block to a BBJ_THROW block, - // then mark the subsequent BBJ_ALWAYS block as unreferenced. - // - // Must do this after we update bbJumpKind of block. - if (isCallAlwaysPair) - { - BasicBlock* leaveBlk = block->Next(); - noway_assert(leaveBlk->KindIs(BBJ_ALWAYS)); - - // leaveBlk is now unreachable, so scrub the pred lists. - leaveBlk->bbFlags &= ~BBF_DONT_REMOVE; - leaveBlk->bbRefs = 0; - leaveBlk->bbPreds = nullptr; - -#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) - fgClearFinallyTargetBit(leaveBlk->GetJumpDest()); -#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) - } -} - /***************************************************************************** * * Return true if we've added any new basic blocks. diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 75078fc57f8d6f..0390715aa1dc45 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -382,6 +382,55 @@ void Compiler::fgRemoveReturnBlock(BasicBlock* block) } } +//------------------------------------------------------------------------ +// fgConvertBBToThrowBB: Change a given block to a throw block. +// +// Arguments: +// block - block in question +// +void Compiler::fgConvertBBToThrowBB(BasicBlock* block) +{ + JITDUMP("Converting " FMT_BB " to BBJ_THROW\n", block->bbNum); + assert(fgPredsComputed); + + // Ordering of the following operations matters. + // First, note if we are looking at the first block of a call always pair. + const bool isCallAlwaysPair = block->isBBCallAlwaysPair(); + + // Scrub this block from the pred lists of any successors + fgRemoveBlockAsPred(block); + + // Update jump kind after the scrub. + block->SetJumpKind(BBJ_THROW DEBUG_ARG(this)); + + // Any block with a throw is rare + block->bbSetRunRarely(); + + // If we've converted a BBJ_CALLFINALLY block to a BBJ_THROW block, + // then mark the subsequent BBJ_ALWAYS block as unreferenced. + // + // Must do this after we update bbJumpKind of block. + if (isCallAlwaysPair) + { + BasicBlock* leaveBlk = block->Next(); + noway_assert(leaveBlk->KindIs(BBJ_ALWAYS)); + + leaveBlk->bbFlags &= ~BBF_DONT_REMOVE; + + // leaveBlk is now unreachable, so scrub the pred lists. + for (BasicBlock* const leavePredBlock : leaveBlk->PredBlocks()) + { + fgRemoveEhfSuccessor(leavePredBlock, leaveBlk); + } + assert(leaveBlk->bbRefs == 0); + assert(leaveBlk->bbPreds == nullptr); + +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + fgClearFinallyTargetBit(leaveBlk->GetJumpDest()); +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + } +} + /***************************************************************************** * fgChangeSwitchBlock: * @@ -439,7 +488,7 @@ void Compiler::fgChangeSwitchBlock(BasicBlock* oldSwitchBlock, BasicBlock* newSw } //------------------------------------------------------------------------ -// fgReplaceSwitchJumpTarget: update BBJ_SWITCH block so that all control +// fgReplaceSwitchJumpTarget: update BBJ_SWITCH block so that all control // that previously flowed to oldTarget now flows to newTarget. // // Arguments: @@ -449,8 +498,6 @@ void Compiler::fgChangeSwitchBlock(BasicBlock* oldSwitchBlock, BasicBlock* newSw // // Notes: // Updates the jump table and the cached unique target set (if any). -// Can be called before or after pred lists are built. -// If pred lists are built, updates pred lists. // void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* newTarget, BasicBlock* oldTarget) { @@ -462,7 +509,6 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne // For the jump targets values that match oldTarget of our BBJ_SWITCH // replace predecessor 'blockSwitch' with 'newTarget' - // unsigned jumpCnt = blockSwitch->GetJumpSwt()->bbsCount; BasicBlock** jumpTab = blockSwitch->GetJumpSwt()->bbsDstTab; @@ -516,6 +562,151 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne noway_assert(!"Did not find oldTarget in jumpTab[]"); } +//------------------------------------------------------------------------ +// fgChangeEhfBlock: We have a BBJ_EHFINALLYRET block at 'oldBlock' and we want to move this +// to 'newBlock'. All of the 'oldBlock' successors need to have their predecessor lists updated +// by removing edges to 'oldBlock' and adding edges to 'newBlock'. +// +// Arguments: +// oldBlock - previous BBJ_EHFINALLYRET block +// newBlock - block that is replacing 'oldBlock' +// +void Compiler::fgChangeEhfBlock(BasicBlock* oldBlock, BasicBlock* newBlock) +{ + assert(oldBlock != nullptr); + assert(newBlock != nullptr); + assert(oldBlock->KindIs(BBJ_EHFINALLYRET)); + assert(fgPredsComputed); + + for (BasicBlock* const succ : oldBlock->EHFinallyRetSuccs()) + { + assert(succ != nullptr); + + // Remove the old edge [oldBlock => succ] + // + assert(succ->countOfInEdges() > 0); + fgRemoveRefPred(succ, oldBlock); + + // Create the new edge [newBlock => succ] + // + fgAddRefPred(succ, newBlock); + } +} + +//------------------------------------------------------------------------ +// fgReplaceEhfSuccessor: update BBJ_EHFINALLYRET block so that all control +// that previously flowed to oldSucc now flows to newSucc. It is assumed +// that oldSucc is currently a successor of `block`. +// +// Arguments: +// block - BBJ_EHFINALLYRET block +// oldSucc - old successor +// newSucc - new successor +// +void Compiler::fgReplaceEhfSuccessor(BasicBlock* block, BasicBlock* oldSucc, BasicBlock* newSucc) +{ + assert(block != nullptr); + assert(oldSucc != nullptr); + assert(newSucc != nullptr); + assert(block->KindIs(BBJ_EHFINALLYRET)); + assert(fgPredsComputed); + + BBehfDesc* const ehfDesc = block->GetJumpEhf(); + const unsigned succCount = ehfDesc->bbeCount; + BasicBlock** const succTab = ehfDesc->bbeSuccs; + + // Walk the successor table looking for blocks to update the preds for + bool found = false; + FlowEdge* newEdge = nullptr; + for (unsigned i = 0; i < succCount; i++) + { + if (succTab[i] != oldSucc) + { + continue; + } + + found = true; + + // Change the succTab entry to branch to the new location + // + succTab[i] = newSucc; + + if (newEdge == nullptr) + { + // Remove the old edge [block => oldSucc] + // + fgRemoveAllRefPreds(oldSucc, block); + + // Create the new edge [block => newSucc] + // + newEdge = fgAddRefPred(newSucc, block); + } + else + { + newSucc->bbRefs++; + newEdge->incrementDupCount(); + } + } + noway_assert(found && "Did not find oldSucc in succTab[]"); +} + +//------------------------------------------------------------------------ +// fgRemoveEhfSuccessor: update BBJ_EHFINALLYRET block to remove `succ` as a successor. +// Updates the predecessor list of `succ`. +// +// Arguments: +// block - BBJ_EHFINALLYRET block +// succ - successor +// +void Compiler::fgRemoveEhfSuccessor(BasicBlock* block, BasicBlock* succ) +{ + assert(block != nullptr); + assert(succ != nullptr); + assert(fgPredsComputed); + assert(block->KindIs(BBJ_EHFINALLYRET)); + + // Don't `assert(succ->isBBCallAlwaysPairTail())`; we've already unlinked the CALLFINALLY + assert(succ->KindIs(BBJ_ALWAYS)); + + fgRemoveRefPred(succ, block); + + BBehfDesc* const ehfDesc = block->GetJumpEhf(); + unsigned succCount = ehfDesc->bbeCount; + BasicBlock** succTab = ehfDesc->bbeSuccs; + bool found = false; + + // Walk the successor table looking for the specified successor block. + for (unsigned i = 0; i < succCount; i++) + { + if (succTab[i] == succ) + { + // If it's not the last one, move everything after in the table down one slot. + if (i + 1 < succCount) + { + memmove_s(&succTab[i], (succCount - i) * sizeof(BasicBlock*), &succTab[i + 1], + (succCount - i - 1) * sizeof(BasicBlock*)); + } + + --succCount; + + found = true; + +#ifdef DEBUG + // We only expect to see a successor once in the table. + for (; i < succCount; i++) + { + assert(succTab[i] != succ); + } +#endif // DEBUG + + break; + } + } + assert(found); + + ehfDesc->bbeCount = succCount; +} + //------------------------------------------------------------------------ // Compiler::fgReplaceJumpTarget: For a given block, replace the target 'oldTarget' with 'newTarget'. // @@ -529,7 +720,7 @@ void Compiler::fgReplaceSwitchJumpTarget(BasicBlock* blockSwitch, BasicBlock* ne // We assert for other jump kinds. // 2. All branch targets found are updated. If there are multiple ways for a block // to reach 'oldTarget' (e.g., multiple arms of a switch), all of them are changed. -// 3. The predecessor lists are updated, if they've been built. +// 3. The predecessor lists are updated. // 4. If any switch table entry was updated, the switch table "unique successor" cache is invalidated. // void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* newTarget, BasicBlock* oldTarget) @@ -3979,7 +4170,7 @@ void Compiler::fgCheckForLoopsInHandlers() { if (blk->bbFlags & BBF_BACKWARD_JUMP_TARGET) { - JITDUMP("\nHandler block " FMT_BB "is backward jump target; can't have patchpoints in this method\n", + JITDUMP("\nHandler block " FMT_BB " is backward jump target; can't have patchpoints in this method\n", blk->bbNum); compHasBackwardJumpInHandler = true; break; @@ -5090,8 +5281,18 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) noway_assert(leaveBlk->KindIs(BBJ_ALWAYS)); leaveBlk->bbFlags &= ~BBF_DONT_REMOVE; - leaveBlk->bbRefs = 0; - leaveBlk->bbPreds = nullptr; + + // The BBJ_ALWAYS normally has a reference count of 1 and a single predecessor. However, + // it might not have a predecessor on ARM, where we don't create BBF_RETLESS_CALL BBJ_CALLFINALLY. + // And on other platforms, we might not have marked it as BBF_RETLESS_CALL even though it is. + // (Some early flow optimization should probably aggressively mark these as BBF_RETLESS_CALL + // and not depend on fgRemoveBlock() to do that.) + for (BasicBlock* const leavePredBlock : leaveBlk->PredBlocks()) + { + fgRemoveEhfSuccessor(leavePredBlock, leaveBlk); + } + assert(leaveBlk->bbRefs == 0); + assert(leaveBlk->bbPreds == nullptr); fgRemoveBlock(leaveBlk, /* unreachable */ true); @@ -5286,13 +5487,11 @@ void Compiler::fgRemoveBlock(BasicBlock* block, bool unreachable) predBlock->SetJumpDest(succBlock); break; + case BBJ_EHFINALLYRET: + fgReplaceEhfSuccessor(predBlock, block, succBlock); + break; + case BBJ_SWITCH: - // Change any jumps from 'predBlock' (a BBJ_SWITCH) to 'block' to jump to 'succBlock' - // - // For the jump targets of 'predBlock' (a BBJ_SWITCH) that jump to 'block' - // remove the old predecessor at 'block' from 'predBlock' and - // add the new predecessor at 'succBlock' from 'predBlock' - // fgReplaceSwitchJumpTarget(predBlock, succBlock, block); break; } @@ -5374,7 +5573,6 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst) { switch (bSrc->GetJumpKind()) { - case BBJ_NONE: bSrc->SetJumpKindAndTarget(BBJ_ALWAYS, bDst); JITDUMP("Block " FMT_BB " ended with a BBJ_NONE, Changed to an unconditional jump to " FMT_BB "\n", diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 60aad57559d3e6..6a568a817bc156 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -2035,8 +2035,40 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 * break; case BBJ_EHFINALLYRET: - printf("%*s (finret)", maxBlockNumWidth - 2, ""); + { + printf("->"); + + int ehfWidth = 0; + const BBehfDesc* const ehfDesc = block->GetJumpEhf(); + if (ehfDesc == nullptr) + { + printf(" ????"); + ehfWidth = 5; + } + else + { + // Very early in compilation, we won't have fixed up the BBJ_EHFINALLYRET successors yet. + + const unsigned jumpCnt = ehfDesc->bbeCount; + BasicBlock** const jumpTab = ehfDesc->bbeSuccs; + + for (unsigned i = 0; i < jumpCnt; i++) + { + printf("%c" FMT_BB, (i == 0) ? ' ' : ',', jumpTab[i]->bbNum); + ehfWidth += 1 /* space/comma */ + 2 /* BB */ + max(CountDigits(jumpTab[i]->bbNum), 2); + } + } + + int singleBlockWidth = 1 /* space */ + 2 /* BB */ + maxBlockNumWidth; + if (ehfWidth < singleBlockWidth) + { + // only if we didn't display anything or we displayeda small numbered block + printf("%*s", singleBlockWidth - ehfWidth, ""); + } + + printf(" (finret)"); break; + } case BBJ_EHFAULTRET: printf("%*s (falret)", maxBlockNumWidth - 2, ""); @@ -2067,9 +2099,9 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 * { printf("->"); - const BBswtDesc* const bbJumpSwt = block->GetJumpSwt(); - const unsigned jumpCnt = bbJumpSwt->bbsCount; - BasicBlock** const jumpTab = bbJumpSwt->bbsDstTab; + const BBswtDesc* const jumpSwt = block->GetJumpSwt(); + const unsigned jumpCnt = jumpSwt->bbsCount; + BasicBlock** const jumpTab = jumpSwt->bbsDstTab; int switchWidth = 0; for (unsigned i = 0; i < jumpCnt; i++) @@ -2077,17 +2109,17 @@ void Compiler::fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth /* = 0 * printf("%c" FMT_BB, (i == 0) ? ' ' : ',', jumpTab[i]->bbNum); switchWidth += 1 /* space/comma */ + 2 /* BB */ + max(CountDigits(jumpTab[i]->bbNum), 2); - const bool isDefault = bbJumpSwt->bbsHasDefault && (i == jumpCnt - 1); + const bool isDefault = jumpSwt->bbsHasDefault && (i == jumpCnt - 1); if (isDefault) { printf("[def]"); switchWidth += 5; } - const bool isDominant = bbJumpSwt->bbsHasDominantCase && (i == bbJumpSwt->bbsDominantCase); + const bool isDominant = jumpSwt->bbsHasDominantCase && (i == jumpSwt->bbsDominantCase); if (isDominant) { - printf("[dom(" FMT_WT ")]", bbJumpSwt->bbsDominantFraction); + printf("[dom(" FMT_WT ")]", jumpSwt->bbsDominantFraction); switchWidth += 10; } } @@ -2717,36 +2749,41 @@ bool BBPredsChecker::CheckEHFinallyRet(BasicBlock* blockPred, BasicBlock* block) { // If the current block is a successor to a BBJ_EHFINALLYRET (return from finally), // then the lexically previous block should be a call to the same finally. + // Also, `block` should be in the explicit successors list of `blockPred`. // Verify all of that. + bool found = false; + for (BasicBlock* const succ : blockPred->EHFinallyRetSuccs()) + { + if (block == succ) + { + assert(!found); // we should only find it once + found = true; + } + } + assert(found && "BBJ_EHFINALLYRET successor not found"); + unsigned hndIndex = blockPred->getHndIndex(); EHblkDsc* ehDsc = comp->ehGetDsc(hndIndex); BasicBlock* finBeg = ehDsc->ebdHndBeg; - // Because there is no bbPrev, we have to search for the lexically previous - // block. We can shorten the search by only looking in places where it is legal - // to have a call to the finally. + BasicBlock* firstBlock; + BasicBlock* lastBlock; + comp->ehGetCallFinallyBlockRange(hndIndex, &firstBlock, &lastBlock); - BasicBlock* begBlk; - BasicBlock* endBlk; - comp->ehGetCallFinallyBlockRange(hndIndex, &begBlk, &endBlk); - - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) + found = false; + for (BasicBlock* const bcall : comp->Blocks(firstBlock, lastBlock)) { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) + if (bcall->KindIs(BBJ_CALLFINALLY) && bcall->HasJumpTo(finBeg) && bcall->NextIs(block)) { - continue; - } - - if (bcall->NextIs(block)) - { - return true; + found = true; + break; } } #if defined(FEATURE_EH_FUNCLETS) - if (comp->fgFuncletsCreated) + if (!found && comp->fgFuncletsCreated) { // There is no easy way to search just the funclets that were pulled out of // the corresponding try body, so instead we search all the funclets, and if @@ -2755,27 +2792,19 @@ bool BBPredsChecker::CheckEHFinallyRet(BasicBlock* blockPred, BasicBlock* block) for (BasicBlock* const bcall : comp->Blocks(comp->fgFirstFuncletBB)) { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) + if (bcall->KindIs(BBJ_CALLFINALLY) && bcall->HasJumpTo(finBeg) && bcall->NextIs(block) && + comp->ehCallFinallyInCorrectRegion(bcall, hndIndex)) { - continue; - } - - if (!bcall->NextIs(block)) - { - continue; - } - - if (comp->ehCallFinallyInCorrectRegion(bcall, hndIndex)) - { - return true; + found = true; + break; } } } #endif // FEATURE_EH_FUNCLETS - assert(!"BBJ_EHFINALLYRET predecessor of block that doesn't follow a BBJ_CALLFINALLY!"); - return false; + assert(found && "BBJ_EHFINALLYRET predecessor of block that doesn't follow a BBJ_CALLFINALLY!"); + return found; } //------------------------------------------------------------------------------ @@ -2868,6 +2897,40 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef assert(block->IsLast() || (block->bbNum + 1 == block->Next()->bbNum)); } + // Check that all the successors have the current traversal stamp. Use the 'Compiler*' version of the + // iterator, but not for BBJ_SWITCH: we don't want to end up calling GetDescriptorForSwitch(), which will + // dynamically create the unique switch list. + if (block->KindIs(BBJ_SWITCH)) + { + for (BasicBlock* const succBlock : block->Succs()) + { + assert(succBlock->bbTraversalStamp == curTraversalStamp); + } + + // Also check the unique successor set, if it exists. Make sure to NOT allocate it if it doesn't exist! + BlockToSwitchDescMap* switchMap = GetSwitchDescMap(/* createIfNull */ false); + if (switchMap != nullptr) + { + SwitchUniqueSuccSet sd; + if (switchMap->Lookup(block, &sd)) + { + for (unsigned i = 0; i < sd.numDistinctSuccs; i++) + { + const BasicBlock* const nonDuplicateSucc = sd.nonDuplicates[i]; + assert(nonDuplicateSucc != nullptr); + assert(nonDuplicateSucc->bbTraversalStamp == curTraversalStamp); + } + } + } + } + else + { + for (BasicBlock* const succBlock : block->Succs(this)) + { + assert(succBlock->bbTraversalStamp == curTraversalStamp); + } + } + // If the block is a BBJ_COND, a BBJ_SWITCH or a // lowered GT_SWITCH_TABLE node then make sure it // ends with a conditional jump or a GT_SWITCH diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 8fd44c5e6f89c0..91c30074fd8b99 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -128,15 +128,18 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() continue; } + assert(lastBlock->KindIs(BBJ_EHFINALLYRET)); + JITDUMP("EH#%u has empty finally, removing the region.\n", XTnum); // Find all the call finallys that invoke this finally, // and modify them to jump to the return point. BasicBlock* firstCallFinallyRangeBlock = nullptr; - BasicBlock* endCallFinallyRangeBlock = nullptr; - ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &endCallFinallyRangeBlock); + BasicBlock* lastCallFinallyRangeBlock = nullptr; + ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &lastCallFinallyRangeBlock); - BasicBlock* currentBlock = firstCallFinallyRangeBlock; + BasicBlock* currentBlock = firstCallFinallyRangeBlock; + BasicBlock* const endCallFinallyRangeBlock = lastCallFinallyRangeBlock->Next(); while (currentBlock != endCallFinallyRangeBlock) { @@ -170,9 +173,11 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() // Delete the leave block, which should be marked as // keep always and have the sole finally block as a pred. + // Remove `leaveBlock` as a successor of the finally `lastBlock`, which also removes the predecessor + // edge in `leaveBlock` from the finally return. assert((leaveBlock->bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0); nextBlock = leaveBlock->Next(); - fgRemoveRefPred(leaveBlock, firstBlock); + fgRemoveEhfSuccessor(lastBlock, leaveBlock); leaveBlock->bbFlags &= ~BBF_KEEP_BBJ_ALWAYS; fgRemoveBlock(leaveBlock, /* unreachable */ true); @@ -204,7 +209,10 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() BasicBlock* const lastTryBlock = HBtab->ebdTryLast; assert(firstTryBlock->getTryIndex() == XTnum); - for (BasicBlock* const block : Blocks(firstTryBlock)) + assert((firstTryBlock->bbFlags & BBF_TRY_BEG) != 0); + firstTryBlock->bbFlags &= ~BBF_TRY_BEG; + + for (BasicBlock* const block : Blocks(firstTryBlock, lastTryBlock)) { // Look for blocks directly contained in this try, and // update the try region appropriately. @@ -223,17 +231,6 @@ PhaseStatus Compiler::fgRemoveEmptyFinally() block->clearTryIndex(); } } - - if (block == firstTryBlock) - { - assert((block->bbFlags & BBF_TRY_BEG) != 0); - block->bbFlags &= ~BBF_TRY_BEG; - } - - if (block == lastTryBlock) - { - break; - } } // Remove the try-finally EH region. This will compact the EH table @@ -430,11 +427,11 @@ PhaseStatus Compiler::fgRemoveEmptyTry() // There should be just one callfinally that invokes this // finally, the one we found above. Verify this. BasicBlock* firstCallFinallyRangeBlock = nullptr; - BasicBlock* endCallFinallyRangeBlock = nullptr; + BasicBlock* lastCallFinallyRangeBlock = nullptr; bool verifiedSingleCallfinally = true; - ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &endCallFinallyRangeBlock); + ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &lastCallFinallyRangeBlock); - for (BasicBlock* block = firstCallFinallyRangeBlock; block != endCallFinallyRangeBlock; block = block->Next()) + for (BasicBlock* const block : Blocks(firstCallFinallyRangeBlock, lastCallFinallyRangeBlock)) { if (block->KindIs(BBJ_CALLFINALLY) && block->HasJumpTo(firstHandlerBlock)) { @@ -446,8 +443,6 @@ PhaseStatus Compiler::fgRemoveEmptyTry() verifiedSingleCallfinally = false; break; } - - block = block->Next(); } } @@ -605,7 +600,7 @@ PhaseStatus Compiler::fgRemoveEmptyTry() // // If all callfinallys for a given finally are converted to jump to // the clone, the try-finally is modified into a try-fault, -// distingushable from organic try-faults by handler type +// distinguishable from organic try-faults by handler type // EH_HANDLER_FAULT_WAS_FINALLY vs the organic EH_HANDLER_FAULT. // // Does not yet handle thread abort. The open issues here are how @@ -734,7 +729,7 @@ PhaseStatus Compiler::fgCloneFinally() bool isAllRare = true; bool hasSwitch = false; - for (const BasicBlock* block = firstBlock; block != nextBlock; block = block->Next()) + for (BasicBlock* const block : Blocks(firstBlock, lastBlock)) { if (block->KindIs(BBJ_SWITCH)) { @@ -751,7 +746,7 @@ PhaseStatus Compiler::fgCloneFinally() regionStmtCount++; } - hasFinallyRet = hasFinallyRet || (block->KindIs(BBJ_EHFINALLYRET)); + hasFinallyRet = hasFinallyRet || block->KindIs(BBJ_EHFINALLYRET); isAllRare = isAllRare && block->isRunRarely(); } @@ -788,9 +783,8 @@ PhaseStatus Compiler::fgCloneFinally() continue; } - JITDUMP("EH#%u is a candidate for finally cloning:" - " %u blocks, %u statements\n", - XTnum, regionBBCount, regionStmtCount); + JITDUMP("EH#%u is a candidate for finally cloning: %u blocks, %u statements\n", XTnum, regionBBCount, + regionStmtCount); // Walk the try region backwards looking for blocks // that transfer control to a callfinally. @@ -832,6 +826,9 @@ PhaseStatus Compiler::fgCloneFinally() { continue; } +#else + BasicBlock* const jumpDest = block; +#endif // FEATURE_EH_CALLFINALLY_THUNKS // The jumpDest must be a callfinally that in turn invokes the // finally of interest. @@ -839,15 +836,6 @@ PhaseStatus Compiler::fgCloneFinally() { continue; } -#else - // Look for call finally pair directly within the try - if (!block->isBBCallAlwaysPair() || !block->HasJumpTo(firstBlock)) - { - continue; - } - - BasicBlock* const jumpDest = block; -#endif // FEATURE_EH_CALLFINALLY_THUNKS // Found a block that invokes the finally. // @@ -957,23 +945,19 @@ PhaseStatus Compiler::fgCloneFinally() // callfinally to come first after the try, so we can fall out of the try // into the clone. BasicBlock* firstCallFinallyRangeBlock = nullptr; - BasicBlock* endCallFinallyRangeBlock = nullptr; - ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &endCallFinallyRangeBlock); + BasicBlock* lastCallFinallyRangeBlock = nullptr; + ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &lastCallFinallyRangeBlock); if (tryToRelocateCallFinally) { BasicBlock* firstCallFinallyBlock = nullptr; - for (BasicBlock* block = firstCallFinallyRangeBlock; block != endCallFinallyRangeBlock; - block = block->Next()) + for (BasicBlock* const block : Blocks(firstCallFinallyRangeBlock, lastCallFinallyRangeBlock)) { - if (block->isBBCallAlwaysPair()) + if (block->isBBCallAlwaysPair() && block->HasJumpTo(firstBlock)) { - if (block->HasJumpTo(firstBlock)) - { - firstCallFinallyBlock = block; - break; - } + firstCallFinallyBlock = block; + break; } } @@ -1047,7 +1031,7 @@ PhaseStatus Compiler::fgCloneFinally() // Avoid asserts when `fgNewBBinRegion` verifies the handler table, by mapping any cloned finally // return blocks to BBJ_ALWAYS (which we would do below if we didn't do it here). - BBjumpKinds bbNewJumpKind = (block->KindIs(BBJ_EHFINALLYRET)) ? BBJ_ALWAYS : block->GetJumpKind(); + BBjumpKinds bbNewJumpKind = block->KindIs(BBJ_EHFINALLYRET) ? BBJ_ALWAYS : block->GetJumpKind(); if (block == firstBlock) { @@ -1152,72 +1136,70 @@ PhaseStatus Compiler::fgCloneFinally() // retargeted (since they want to return to other places). BasicBlock* const firstCloneBlock = blockMap[firstBlock]; bool retargetedAllCalls = true; - BasicBlock* currentBlock = firstCallFinallyRangeBlock; weight_t retargetedWeight = BB_ZERO_WEIGHT; + BasicBlock* currentBlock = firstCallFinallyRangeBlock; + BasicBlock* const endCallFinallyRangeBlock = lastCallFinallyRangeBlock->Next(); while (currentBlock != endCallFinallyRangeBlock) { BasicBlock* nextBlockToScan = currentBlock->Next(); - if (currentBlock->isBBCallAlwaysPair()) + if (currentBlock->isBBCallAlwaysPair() && currentBlock->HasJumpTo(firstBlock)) { - if (currentBlock->HasJumpTo(firstBlock)) + BasicBlock* const leaveBlock = currentBlock->Next(); + BasicBlock* const postTryFinallyBlock = leaveBlock->GetJumpDest(); + + // Note we must retarget all callfinallies that have this + // continuation, or we can't clean up the continuation + // block properly below, since it will be reachable both + // by the cloned finally and by the called finally. + if (postTryFinallyBlock == normalCallFinallyReturn) { - BasicBlock* const leaveBlock = currentBlock->Next(); - BasicBlock* const postTryFinallyBlock = leaveBlock->GetJumpDest(); - - // Note we must retarget all callfinallies that have this - // continuation, or we can't clean up the continuation - // block properly below, since it will be reachable both - // by the cloned finally and by the called finally. - if (postTryFinallyBlock == normalCallFinallyReturn) - { - JITDUMP("Retargeting callfinally " FMT_BB " to clone entry " FMT_BB "\n", currentBlock->bbNum, - firstCloneBlock->bbNum); + JITDUMP("Retargeting callfinally " FMT_BB " to clone entry " FMT_BB "\n", currentBlock->bbNum, + firstCloneBlock->bbNum); - // This call returns to the expected spot, so - // retarget it to branch to the clone. - currentBlock->SetJumpKindAndTarget(BBJ_ALWAYS, firstCloneBlock); + // This call returns to the expected spot, so + // retarget it to branch to the clone. + currentBlock->SetJumpKindAndTarget(BBJ_ALWAYS, firstCloneBlock); - // Ref count updates. - fgAddRefPred(firstCloneBlock, currentBlock); - fgRemoveRefPred(firstBlock, currentBlock); + // Ref count updates. + fgAddRefPred(firstCloneBlock, currentBlock); + fgRemoveRefPred(firstBlock, currentBlock); - // Delete the leave block, which should be marked as - // keep always. - assert((leaveBlock->bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0); - nextBlock = leaveBlock->Next(); + // Delete the leave block, which should be marked as + // keep always. + assert((leaveBlock->bbFlags & BBF_KEEP_BBJ_ALWAYS) != 0); + nextBlock = leaveBlock->Next(); - // All preds should be BBJ_EHFINALLYRETs from the finally. - for (BasicBlock* const leavePred : leaveBlock->PredBlocks()) - { - assert(leavePred->KindIs(BBJ_EHFINALLYRET)); - assert(leavePred->getHndIndex() == XTnum); - } + // All preds should be BBJ_EHFINALLYRETs from the finally. + for (BasicBlock* const leavePred : leaveBlock->PredBlocks()) + { + assert(leavePred->getHndIndex() == XTnum); + fgRemoveEhfSuccessor(leavePred, leaveBlock); + } - leaveBlock->bbRefs = 0; - leaveBlock->bbPreds = nullptr; - leaveBlock->bbFlags &= ~BBF_KEEP_BBJ_ALWAYS; - fgRemoveBlock(leaveBlock, /* unreachable */ true); + assert(leaveBlock->bbRefs == 0); + assert(leaveBlock->bbPreds == nullptr); + leaveBlock->bbFlags &= ~BBF_KEEP_BBJ_ALWAYS; + fgRemoveBlock(leaveBlock, /* unreachable */ true); - // Make sure iteration isn't going off the deep end. - assert(leaveBlock != endCallFinallyRangeBlock); + // Make sure iteration isn't going off the deep end. + assert(leaveBlock != endCallFinallyRangeBlock); - if (currentBlock->hasProfileWeight()) - { - retargetedWeight += currentBlock->bbWeight; - } - } - else + if (currentBlock->hasProfileWeight()) { - // We can't retarget this call since it - // returns somewhere else. - JITDUMP("Can't retarget callfinally in " FMT_BB " as it jumps to " FMT_BB ", not " FMT_BB "\n", - currentBlock->bbNum, postTryFinallyBlock->bbNum, normalCallFinallyReturn->bbNum); - - retargetedAllCalls = false; + retargetedWeight += currentBlock->bbWeight; } } + else + { + // We can't retarget this call since it + // returns somewhere else. + JITDUMP("Can't retarget callfinally in " FMT_BB " as it jumps to " FMT_BB ", not " FMT_BB "\n", + currentBlock->bbNum, postTryFinallyBlock->bbNum, normalCallFinallyReturn->bbNum); + + retargetedAllCalls = false; + } } currentBlock = nextBlockToScan; @@ -1233,12 +1215,11 @@ PhaseStatus Compiler::fgCloneFinally() firstBlock->bbCatchTyp = BBCT_FAULT; // Change all BBJ_EHFINALLYRET to BBJ_EHFAULTRET in the now-fault region. - BasicBlock* const hndBegIter = HBtab->ebdHndBeg; - BasicBlock* const hndEndIter = HBtab->ebdHndLast->Next(); - for (BasicBlock* block = hndBegIter; block != hndEndIter; block = block->Next()) + for (BasicBlock* const block : Blocks(HBtab->ebdHndBeg, HBtab->ebdHndLast)) { if (block->KindIs(BBJ_EHFINALLYRET)) { + assert(block->GetJumpEhf()->bbeCount == 0); block->SetJumpKind(BBJ_EHFAULTRET DEBUG_ARG(this)); } } @@ -1351,7 +1332,7 @@ PhaseStatus Compiler::fgCloneFinally() // Depending on the model for invoking finallys, the callfinallies may // lie within the try region (callfinally thunks) or in the enclosing // region. - +// void Compiler::fgDebugCheckTryFinallyExits() { unsigned XTnum = 0; @@ -1530,7 +1511,7 @@ void Compiler::fgDebugCheckTryFinallyExits() // BBF_FINALLY_TARGET bbFlag is left unchanged by this method // since it cannot be incrementally updated. Proper updates happen // when fgUpdateFinallyTargetFlags runs after all finally optimizations. - +// void Compiler::fgCleanupContinuation(BasicBlock* continuation) { // The continuation may be a finalStep block. @@ -1774,8 +1755,8 @@ PhaseStatus Compiler::fgMergeFinallyChains() // Find all the callfinallys that invoke this finally. BasicBlock* firstCallFinallyRangeBlock = nullptr; - BasicBlock* endCallFinallyRangeBlock = nullptr; - ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &endCallFinallyRangeBlock); + BasicBlock* lastCallFinallyRangeBlock = nullptr; + ehGetCallFinallyBlockRange(XTnum, &firstCallFinallyRangeBlock, &lastCallFinallyRangeBlock); // Clear out any stale entries in the continuation map continuationMap.RemoveAll(); @@ -1785,8 +1766,7 @@ PhaseStatus Compiler::fgMergeFinallyChains() unsigned callFinallyCount = 0; BasicBlock* const beginHandlerBlock = HBtab->ebdHndBeg; - for (BasicBlock* currentBlock = firstCallFinallyRangeBlock; currentBlock != endCallFinallyRangeBlock; - currentBlock = currentBlock->Next()) + for (BasicBlock* const currentBlock : Blocks(firstCallFinallyRangeBlock, lastCallFinallyRangeBlock)) { // Ignore "retless" callfinallys (where the finally doesn't return). if (currentBlock->isBBCallAlwaysPair() && currentBlock->HasJumpTo(beginHandlerBlock)) @@ -1832,8 +1812,7 @@ PhaseStatus Compiler::fgMergeFinallyChains() // to a callfinally that invokes this try's finally, and make // sure they all jump to the appropriate canonical // callfinally. - for (BasicBlock* currentBlock = firstCallFinallyRangeBlock; currentBlock != endCallFinallyRangeBlock; - currentBlock = currentBlock->Next()) + for (BasicBlock* const currentBlock : Blocks(firstCallFinallyRangeBlock, lastCallFinallyRangeBlock)) { bool merged = fgRetargetBranchesToCanonicalCallFinally(currentBlock, beginHandlerBlock, continuationMap); didMerge = didMerge || merged; diff --git a/src/coreclr/jit/fgflow.cpp b/src/coreclr/jit/fgflow.cpp index d051d8374a7d50..f5a35017371a59 100644 --- a/src/coreclr/jit/fgflow.cpp +++ b/src/coreclr/jit/fgflow.cpp @@ -341,29 +341,9 @@ void Compiler::fgRemoveBlockAsPred(BasicBlock* block) { PREFIX_ASSUME(block != nullptr); - BasicBlock* bNext; - switch (block->GetJumpKind()) { case BBJ_CALLFINALLY: - if (!(block->bbFlags & BBF_RETLESS_CALL)) - { - assert(block->isBBCallAlwaysPair()); - - /* The block after the BBJ_CALLFINALLY block is not reachable */ - bNext = block->Next(); - - /* bNext is an unreachable BBJ_ALWAYS block */ - noway_assert(bNext->KindIs(BBJ_ALWAYS)); - - while (bNext->countOfInEdges() > 0) - { - fgRemoveRefPred(bNext, bNext->bbPreds->getSourceBlock()); - } - } - fgRemoveRefPred(block->GetJumpDest(), block); - break; - case BBJ_ALWAYS: case BBJ_EHCATCHRET: fgRemoveRefPred(block->GetJumpDest(), block); @@ -379,41 +359,16 @@ void Compiler::fgRemoveBlockAsPred(BasicBlock* block) break; case BBJ_EHFILTERRET: - block->GetJumpDest()->bbRefs++; // To compensate the bbRefs-- inside fgRemoveRefPred fgRemoveRefPred(block->GetJumpDest(), block); break; case BBJ_EHFINALLYRET: - { - /* Remove block as the predecessor of the bbNext of all - BBJ_CALLFINALLY blocks calling this finally. No need - to look for BBJ_CALLFINALLY for fault handlers. */ - - unsigned hndIndex = block->getHndIndex(); - EHblkDsc* ehDsc = ehGetDsc(hndIndex); - - if (ehDsc->HasFinallyHandler()) + for (BasicBlock* const succ : block->EHFinallyRetSuccs()) { - BasicBlock* begBlk; - BasicBlock* endBlk; - ehGetCallFinallyBlockRange(hndIndex, &begBlk, &endBlk); - - BasicBlock* finBeg = ehDsc->ebdHndBeg; - - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) - { - if ((bcall->bbFlags & BBF_REMOVED) || !bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) - { - continue; - } - - assert(bcall->isBBCallAlwaysPair()); - fgRemoveRefPred(bcall->Next(), block); - } + fgRemoveRefPred(succ, block); } - } - break; + break; case BBJ_EHFAULTRET: case BBJ_THROW: @@ -433,61 +388,6 @@ void Compiler::fgRemoveBlockAsPred(BasicBlock* block) } } -unsigned Compiler::fgNSuccsOfFinallyRet(BasicBlock* block) -{ - BasicBlock* bb; - unsigned res; - fgSuccOfFinallyRetWork(block, ~0, &bb, &res); - return res; -} - -BasicBlock* Compiler::fgSuccOfFinallyRet(BasicBlock* block, unsigned i) -{ - BasicBlock* bb; - unsigned res; - fgSuccOfFinallyRetWork(block, i, &bb, &res); - return bb; -} - -void Compiler::fgSuccOfFinallyRetWork(BasicBlock* block, unsigned i, BasicBlock** bres, unsigned* nres) -{ - assert(block->hasHndIndex()); // Otherwise, endfinally outside a finally block? - - unsigned hndIndex = block->getHndIndex(); - EHblkDsc* ehDsc = ehGetDsc(hndIndex); - - assert(ehDsc->HasFinallyHandler()); // Otherwise, endfinally outside a finally block. - - *bres = nullptr; - unsigned succNum = 0; - - BasicBlock* begBlk; - BasicBlock* endBlk; - ehGetCallFinallyBlockRange(hndIndex, &begBlk, &endBlk); - - BasicBlock* finBeg = ehDsc->ebdHndBeg; - - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) - { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) - { - continue; - } - - assert(bcall->isBBCallAlwaysPair()); - - if (succNum == i) - { - *bres = bcall->Next(); - return; - } - succNum++; - } - - assert(i == ~0u); - *nres = succNum; -} - Compiler::SwitchUniqueSuccSet Compiler::GetDescriptorForSwitch(BasicBlock* switchBlk) { assert(switchBlk->KindIs(BBJ_SWITCH)); diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index bb9cb076a801ba..8a11b096685572 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -437,12 +437,9 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock) // to properly set the info.compProfilerCallback flag. continue; } - else + else if (!canRemoveBlock(block)) { - if (!canRemoveBlock(block)) - { - continue; - } + continue; } // Remove all the code for the block @@ -469,15 +466,29 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock) block->SetJumpKind(BBJ_THROW DEBUG_ARG(this)); block->bbSetRunRarely(); -#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) - // If this is a pair, we have to clear BBF_FINALLY_TARGET flag on - // the target node (of BBJ_ALWAYS) since BBJ_CALLFINALLY node is getting converted to a BBJ_THROW. + // If this is a pair, we just converted it to a BBJ_THROW. + // Get rid of the BBJ_ALWAYS block which is now dead. if (bIsBBCallAlwaysPair) { - noway_assert(block->Next()->KindIs(BBJ_ALWAYS)); - fgClearFinallyTargetBit(block->Next()->GetJumpDest()); - } + BasicBlock* leaveBlk = block->Next(); + noway_assert(leaveBlk->KindIs(BBJ_ALWAYS)); + + leaveBlk->bbFlags &= ~BBF_DONT_REMOVE; + + for (BasicBlock* const leavePredBlock : leaveBlk->PredBlocks()) + { + fgRemoveEhfSuccessor(leavePredBlock, leaveBlk); + } + assert(leaveBlk->bbRefs == 0); + assert(leaveBlk->bbPreds == nullptr); + + fgRemoveBlock(leaveBlk, /* unreachable */ true); + +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + // We have to clear BBF_FINALLY_TARGET flag on the target node (of BBJ_ALWAYS). + fgClearFinallyTargetBit(leaveBlk->GetJumpDest()); #endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + } } else { @@ -2025,14 +2036,13 @@ bool Compiler::fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext) void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) { noway_assert(block != nullptr); + noway_assert(bNext != nullptr); noway_assert((block->bbFlags & BBF_REMOVED) == 0); + noway_assert((bNext->bbFlags & BBF_REMOVED) == 0); noway_assert(block->KindIs(BBJ_NONE)); - noway_assert(block->NextIs(bNext)); - noway_assert(bNext != nullptr); - noway_assert((bNext->bbFlags & BBF_REMOVED) == 0); noway_assert(bNext->countOfInEdges() == 1 || block->isEmpty()); - noway_assert(bNext->bbPreds); + noway_assert(bNext->bbPreds != nullptr); #if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) noway_assert((bNext->bbFlags & BBF_FINALLY_TARGET) == 0); @@ -2267,7 +2277,6 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) /* set the right links */ - block->SetJumpKind(bNext->GetJumpKind() DEBUG_ARG(this)); VarSetOps::AssignAllowUninitRhs(this, block->bbLiveOut, bNext->bbLiveOut); // Update the beginning and ending IL offsets (bbCodeOffs and bbCodeOffsEnd). @@ -2319,7 +2328,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) /* Unlink bNext and update all the marker pointers if necessary */ - fgUnlinkRange(block->Next(), bNext); + fgUnlinkRange(bNext, bNext); // If bNext was the last block of a try or handler, update the EH table. @@ -2338,7 +2347,8 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) case BBJ_COND: case BBJ_ALWAYS: case BBJ_EHCATCHRET: - block->SetJumpDest(bNext->GetJumpDest()); + case BBJ_EHFILTERRET: + block->SetJumpKindAndTarget(bNext->GetJumpKind(), bNext->GetJumpDest()); /* Update the predecessor list for 'bNext->bbJumpDest' */ fgReplacePred(bNext->GetJumpDest(), bNext, block); @@ -2351,49 +2361,25 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) break; case BBJ_NONE: + block->SetJumpKind(bNext->GetJumpKind() DEBUG_ARG(this)); /* Update the predecessor list for 'bNext->bbNext' */ fgReplacePred(bNext->Next(), bNext, block); break; - case BBJ_EHFILTERRET: - fgReplacePred(bNext->GetJumpDest(), bNext, block); - break; - case BBJ_EHFINALLYRET: - { - unsigned hndIndex = block->getHndIndex(); - EHblkDsc* ehDsc = ehGetDsc(hndIndex); - - if (ehDsc->HasFinallyHandler()) // No need to do this for fault handlers - { - BasicBlock* begBlk; - BasicBlock* endBlk; - ehGetCallFinallyBlockRange(hndIndex, &begBlk, &endBlk); - - BasicBlock* finBeg = ehDsc->ebdHndBeg; - - for (BasicBlock* bcall = begBlk; bcall != endBlk; bcall = bcall->Next()) - { - if (!bcall->KindIs(BBJ_CALLFINALLY) || !bcall->HasJumpTo(finBeg)) - { - continue; - } - - noway_assert(bcall->isBBCallAlwaysPair()); - fgReplacePred(bcall->Next(), bNext, block); - } - } - } - break; + block->SetJumpKindAndTarget(bNext->GetJumpKind(), bNext->GetJumpEhf()); + fgChangeEhfBlock(bNext, block); + break; case BBJ_EHFAULTRET: case BBJ_THROW: case BBJ_RETURN: /* no jumps or fall through blocks to set here */ + block->SetJumpKind(bNext->GetJumpKind() DEBUG_ARG(this)); break; case BBJ_SWITCH: - block->SetJumpSwt(bNext->GetJumpSwt()); + block->SetJumpKindAndTarget(bNext->GetJumpKind(), bNext->GetJumpSwt()); // We are moving the switch jump from bNext to block. Examine the jump targets // of the BBJ_SWITCH at bNext and replace the predecessor to 'bNext' with ones to 'block' fgChangeSwitchBlock(bNext, block); @@ -2404,7 +2390,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) break; } - if (!bNext->HasJumpTo(nullptr) && bNext->GetJumpDest()->isLoopAlign()) + if (bNext->KindIs(BBJ_COND, BBJ_ALWAYS) && bNext->GetJumpDest()->isLoopAlign()) { // `bNext` has a backward target to some block which mean bNext is part of a loop. // `block` into which `bNext` is compacted should be updated with its loop number diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 3f7a622716024a..2c284730e48077 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2988,29 +2988,6 @@ bool Compiler::fgSimpleLowerCastOfSmpOp(LIR::Range& range, GenTreeCast* cast) return false; } -/***************************************************************************************************** - * - * Function to return the last basic block in the main part of the function. With funclets, it is - * the block immediately before the first funclet. - * An inclusive end of the main method. - */ - -BasicBlock* Compiler::fgLastBBInMainFunction() -{ -#if defined(FEATURE_EH_FUNCLETS) - - if (fgFirstFuncletBB != nullptr) - { - return fgFirstFuncletBB->Prev(); - } - -#endif // FEATURE_EH_FUNCLETS - - assert(fgLastBB->IsLast()); - - return fgLastBB; -} - //------------------------------------------------------------------------------ // fgGetDomSpeculatively: Try determine a more accurate dominator than cached bbIDom // @@ -3056,14 +3033,30 @@ BasicBlock* Compiler::fgGetDomSpeculatively(const BasicBlock* block) return lastReachablePred == nullptr ? block->bbIDom : lastReachablePred; } -/***************************************************************************************************** - * - * Function to return the first basic block after the main part of the function. With funclets, it is - * the block of the first funclet. Otherwise it is NULL if there are no funclets (fgLastBB->Next()). - * This is equivalent to fgLastBBInMainFunction()->bbNext - * An exclusive end of the main method. - */ +//------------------------------------------------------------------------------ +// fgLastBBInMainFunction: Return the last basic block in the main part of the function. +// With funclets, it is the block immediately before the first funclet. +// +BasicBlock* Compiler::fgLastBBInMainFunction() +{ +#if defined(FEATURE_EH_FUNCLETS) + + if (fgFirstFuncletBB != nullptr) + { + return fgFirstFuncletBB->Prev(); + } + +#endif // FEATURE_EH_FUNCLETS + + assert(fgLastBB->IsLast()); + return fgLastBB; +} +//------------------------------------------------------------------------------ +// fgEndBBAfterMainFunction: Return the first basic block after the main part of the function. +// With funclets, it is the block of the first funclet. Otherwise it is nullptr if there are no +// funclets. This is equivalent to fgLastBBInMainFunction()->Next(). +// BasicBlock* Compiler::fgEndBBAfterMainFunction() { #if defined(FEATURE_EH_FUNCLETS) @@ -3076,7 +3069,6 @@ BasicBlock* Compiler::fgEndBBAfterMainFunction() #endif // FEATURE_EH_FUNCLETS assert(fgLastBB->IsLast()); - return nullptr; } diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 8d735c3e99f83a..dda4792a7e098f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -12226,6 +12226,7 @@ void Compiler::impFixPredLists() { BasicBlock* const finallyBegBlock = HBtab->ebdHndBeg; BasicBlock* const finallyLastBlock = HBtab->ebdHndLast; + unsigned predCount = (unsigned)-1; for (BasicBlock* const finallyBlock : BasicBlockRangeList(finallyBegBlock, finallyLastBlock)) { @@ -12241,24 +12242,60 @@ void Compiler::impFixPredLists() continue; } - for (BasicBlock* const predBlock : finallyBegBlock->PredBlocks()) + // Count the number of predecessors. Then we can allocate the bbJumpEhf table and fill it in. + // We only need to count once, since it's invariant with the finally block. + if (predCount == (unsigned)-1) { - // We only care about preds that are callfinallies. - // - if (!predBlock->isBBCallAlwaysPair()) + predCount = 0; + for (BasicBlock* const predBlock : finallyBegBlock->PredBlocks()) { - continue; + // We only care about preds that are callfinallies. + // + if (!predBlock->isBBCallAlwaysPair()) + { + continue; + } + ++predCount; } + } + + BBehfDesc* jumpEhf = new (this, CMK_BasicBlock) BBehfDesc; - BasicBlock* const continuation = predBlock->Next(); - fgAddRefPred(continuation, finallyBlock); + // It's possible for the `finally` to have no CALLFINALLY predecessors if the `try` block + // has an unconditional `throw` (the finally will still be invoked in the exceptional + // case via the runtime). In that case, jumpEhf->bbeCount remains the default, zero, + // and jumpEhf->bbeSuccs remains the default, nullptr. + if (predCount > 0) + { + jumpEhf->bbeCount = predCount; + jumpEhf->bbeSuccs = new (this, CMK_BasicBlock) BasicBlock*[predCount]; - if (!added) + unsigned predNum = 0; + for (BasicBlock* const predBlock : finallyBegBlock->PredBlocks()) { - JITDUMP("\nAdding pred edges from BBJ_EHFINALLYRET blocks\n"); - added = true; + // We only care about preds that are callfinallies. + // + if (!predBlock->isBBCallAlwaysPair()) + { + continue; + } + + BasicBlock* const continuation = predBlock->Next(); + fgAddRefPred(continuation, finallyBlock); + + jumpEhf->bbeSuccs[predNum] = continuation; + ++predNum; + + if (!added) + { + JITDUMP("\nAdding pred edges from BBJ_EHFINALLYRET blocks\n"); + added = true; + } } + assert(predNum == predCount); } + + finallyBlock->SetJumpEhf(jumpEhf); } } } diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 66169d8eb380b1..322af836220385 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -918,12 +918,12 @@ unsigned Compiler::ehGetCallFinallyRegionIndex(unsigned finallyIndex, bool* inTr #endif } -void Compiler::ehGetCallFinallyBlockRange(unsigned finallyIndex, BasicBlock** begBlk, BasicBlock** endBlk) +void Compiler::ehGetCallFinallyBlockRange(unsigned finallyIndex, BasicBlock** startBlock, BasicBlock** lastBlock) { assert(finallyIndex != EHblkDsc::NO_ENCLOSING_INDEX); assert(ehGetDsc(finallyIndex)->HasFinallyHandler()); - assert(begBlk != nullptr); - assert(endBlk != nullptr); + assert(startBlock != nullptr); + assert(lastBlock != nullptr); #if FEATURE_EH_CALLFINALLY_THUNKS bool inTryRegion; @@ -931,8 +931,8 @@ void Compiler::ehGetCallFinallyBlockRange(unsigned finallyIndex, BasicBlock** be if (callFinallyRegionIndex == EHblkDsc::NO_ENCLOSING_INDEX) { - *begBlk = fgFirstBB; - *endBlk = fgEndBBAfterMainFunction(); + *startBlock = fgFirstBB; + *lastBlock = fgLastBBInMainFunction(); } else { @@ -940,19 +940,19 @@ void Compiler::ehGetCallFinallyBlockRange(unsigned finallyIndex, BasicBlock** be if (inTryRegion) { - *begBlk = ehDsc->ebdTryBeg; - *endBlk = ehDsc->ebdTryLast->Next(); + *startBlock = ehDsc->ebdTryBeg; + *lastBlock = ehDsc->ebdTryLast; } else { - *begBlk = ehDsc->ebdHndBeg; - *endBlk = ehDsc->ebdHndLast->Next(); + *startBlock = ehDsc->ebdHndBeg; + *lastBlock = ehDsc->ebdHndLast; } } #else // !FEATURE_EH_CALLFINALLY_THUNKS EHblkDsc* ehDsc = ehGetDsc(finallyIndex); - *begBlk = ehDsc->ebdTryBeg; - *endBlk = ehDsc->ebdTryLast->Next(); + *startBlock = ehDsc->ebdTryBeg; + *lastBlock = ehDsc->ebdTryLast; #endif // !FEATURE_EH_CALLFINALLY_THUNKS } diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index a7125734a718d8..f36a8360daeb3d 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2745,7 +2745,6 @@ void Compiler::optRedirectBlock(BasicBlock* blk, BlockToBlockMap* redirectMap, R case BBJ_RETURN: case BBJ_EHFILTERRET: case BBJ_EHFAULTRET: - case BBJ_EHFINALLYRET: case BBJ_EHCATCHRET: // These have no jump destination to update. break; @@ -2773,12 +2772,39 @@ void Compiler::optRedirectBlock(BasicBlock* blk, BlockToBlockMap* redirectMap, R } break; + case BBJ_EHFINALLYRET: + { + BBehfDesc* ehfDesc = blk->GetJumpEhf(); + BasicBlock* newSucc = nullptr; + for (unsigned i = 0; i < ehfDesc->bbeCount; i++) + { + BasicBlock* const succ = ehfDesc->bbeSuccs[i]; + if (redirectMap->Lookup(succ, &newSucc)) + { + if (updatePreds) + { + fgRemoveRefPred(succ, blk); + } + if (updatePreds || addPreds) + { + fgAddRefPred(newSucc, blk); + } + ehfDesc->bbeSuccs[i] = newSucc; + } + else if (addPreds) + { + fgAddRefPred(succ, blk); + } + } + } + break; + case BBJ_SWITCH: { bool redirected = false; for (unsigned i = 0; i < blk->GetJumpSwt()->bbsCount; i++) { - BasicBlock* switchDest = blk->GetJumpSwt()->bbsDstTab[i]; + BasicBlock* const switchDest = blk->GetJumpSwt()->bbsDstTab[i]; if (redirectMap->Lookup(switchDest, &newJumpDest)) { if (updatePreds) @@ -2831,6 +2857,10 @@ void Compiler::optCopyBlkDest(BasicBlock* from, BasicBlock* to) to->SetJumpDest(from->GetJumpDest()); break; + case BBJ_EHFINALLYRET: + to->SetJumpEhf(new (this, CMK_BasicBlock) BBehfDesc(this, from->GetJumpEhf())); + break; + case BBJ_SWITCH: to->SetJumpSwt(new (this, CMK_BasicBlock) BBswtDesc(this, from->GetJumpSwt())); break;