diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index efa5be945ec7a8..f2dc21a958ef66 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -105,6 +105,12 @@ AllSuccessorEnumerator::AllSuccessorEnumerator(Compiler* comp, BasicBlock* block FlowEdge* Compiler::BlockPredsWithEH(BasicBlock* blk) { + unsigned tryIndex; + if (!bbIsExFlowBlock(blk, &tryIndex)) + { + return blk->bbPreds; + } + BlockToFlowEdgeMap* ehPreds = GetBlockToEHPreds(); FlowEdge* res; if (ehPreds->Lookup(blk, &res)) @@ -113,76 +119,129 @@ FlowEdge* Compiler::BlockPredsWithEH(BasicBlock* blk) } res = blk->bbPreds; - unsigned tryIndex; - if (bbIsExFlowBlock(blk, &tryIndex)) - { - // Find the first block of the try. - EHblkDsc* ehblk = ehGetDsc(tryIndex); - BasicBlock* tryStart = ehblk->ebdTryBeg; - for (BasicBlock* const tryStartPredBlock : tryStart->PredBlocks()) + // Add all blocks handled by this handler (except for second blocks of BBJ_CALLFINALLY/BBJ_ALWAYS pairs; + // these cannot cause transfer to the handler...) + // TODO-Throughput: It would be nice if we could iterate just over the blocks in the try, via + // something like: + // for (BasicBlock* bb = ehblk->ebdTryBeg; bb != ehblk->ebdTryLast->Next(); bb = bb->Next()) + // (plus adding in any filter blocks outside the try whose exceptions are handled here). + // That doesn't work, however: funclets have caused us to sometimes split the body of a try into + // more than one sequence of contiguous blocks. We need to find a better way to do this. + for (BasicBlock* const bb : Blocks()) + { + if (bbInExnFlowRegions(tryIndex, bb) && !bb->isBBCallAlwaysPairTail()) { - res = new (this, CMK_FlowEdge) FlowEdge(tryStartPredBlock, res); + res = new (this, CMK_FlowEdge) FlowEdge(bb, res); #if MEASURE_BLOCK_SIZE genFlowNodeCnt += 1; genFlowNodeSize += sizeof(FlowEdge); #endif // MEASURE_BLOCK_SIZE } + } - // Now add all blocks handled by this handler (except for second blocks of BBJ_CALLFINALLY/BBJ_ALWAYS pairs; - // these cannot cause transfer to the handler...) - // TODO-Throughput: It would be nice if we could iterate just over the blocks in the try, via - // something like: - // for (BasicBlock* bb = ehblk->ebdTryBeg; bb != ehblk->ebdTryLast->Next(); bb = bb->Next()) - // (plus adding in any filter blocks outside the try whose exceptions are handled here). - // That doesn't work, however: funclets have caused us to sometimes split the body of a try into - // more than one sequence of contiguous blocks. We need to find a better way to do this. - for (BasicBlock* const bb : Blocks()) + EHblkDsc* ehblk = ehGetDsc(tryIndex); + if (ehblk->HasFinallyOrFaultHandler() && (ehblk->ebdHndBeg == blk)) + { + // block is a finally or fault handler; all enclosing filters are predecessors + unsigned enclosing = ehblk->ebdEnclosingTryIndex; + while (enclosing != EHblkDsc::NO_ENCLOSING_INDEX) { - if (bbInExnFlowRegions(tryIndex, bb) && !bb->isBBCallAlwaysPairTail()) + EHblkDsc* enclosingDsc = ehGetDsc(enclosing); + if (enclosingDsc->HasFilter()) { - res = new (this, CMK_FlowEdge) FlowEdge(bb, res); + for (BasicBlock* filterBlk = enclosingDsc->ebdFilter; filterBlk != enclosingDsc->ebdHndBeg; + filterBlk = filterBlk->Next()) + { + res = new (this, CMK_FlowEdge) FlowEdge(filterBlk, res); -#if MEASURE_BLOCK_SIZE - genFlowNodeCnt += 1; - genFlowNodeSize += sizeof(FlowEdge); -#endif // MEASURE_BLOCK_SIZE + assert(filterBlk->VisitEHSecondPassSuccs(this, [blk](BasicBlock* succ) { + return succ == blk ? BasicBlockVisit::Abort : BasicBlockVisit::Continue; + }) == BasicBlockVisit::Abort); + } } + + enclosing = enclosingDsc->ebdEnclosingTryIndex; } + } - if (ehblk->HasFinallyOrFaultHandler() && (ehblk->ebdHndBeg == blk)) +#ifdef DEBUG + unsigned hash = SsaStressHashHelper(); + if (hash != 0) + { + res = ShuffleHelper(hash, res); + } +#endif // DEBUG + ehPreds->Set(blk, res); + return res; +} + +//------------------------------------------------------------------------ +// BlockDominancePreds: +// Return list of dominance predecessors. This is the set that we know for +// sure contains a block that was fully executed before control reached +// 'blk'. +// +// Arguments: +// blk - Block to get dominance predecessors for. +// +// Returns: +// List of edges. +// +// Remarks: +// Differs from BlockPredsWithEH only in the treatment of handler blocks; +// enclosed blocks are never dominance preds, while all predecessors of +// blocks in the 'try' are (currently only the first try block expected). +// +FlowEdge* Compiler::BlockDominancePreds(BasicBlock* blk) +{ + unsigned tryIndex; + if (!bbIsExFlowBlock(blk, &tryIndex)) + { + return blk->bbPreds; + } + + EHblkDsc* ehblk = ehGetDsc(tryIndex); + if (!ehblk->HasFinallyOrFaultHandler() || (ehblk->ebdHndBeg != blk)) + { + return ehblk->ebdTryBeg->bbPreds; + } + + // Finally/fault handlers can be preceded by enclosing filters due to 2 + // pass EH, so add those and keep them cached. + BlockToFlowEdgeMap* domPreds = GetDominancePreds(); + FlowEdge* res; + if (domPreds->Lookup(blk, &res)) + { + return res; + } + + res = ehblk->ebdTryBeg->bbPreds; + if (ehblk->HasFinallyOrFaultHandler() && (ehblk->ebdHndBeg == blk)) + { + // block is a finally or fault handler; all enclosing filters are predecessors + unsigned enclosing = ehblk->ebdEnclosingTryIndex; + while (enclosing != EHblkDsc::NO_ENCLOSING_INDEX) { - // block is a finally or fault handler; all enclosing filters are predecessors - unsigned enclosing = ehblk->ebdEnclosingTryIndex; - while (enclosing != EHblkDsc::NO_ENCLOSING_INDEX) + EHblkDsc* enclosingDsc = ehGetDsc(enclosing); + if (enclosingDsc->HasFilter()) { - EHblkDsc* enclosingDsc = ehGetDsc(enclosing); - if (enclosingDsc->HasFilter()) + for (BasicBlock* filterBlk = enclosingDsc->ebdFilter; filterBlk != enclosingDsc->ebdHndBeg; + filterBlk = filterBlk->Next()) { - for (BasicBlock* filterBlk = enclosingDsc->ebdFilter; filterBlk != enclosingDsc->ebdHndBeg; - filterBlk = filterBlk->Next()) - { - res = new (this, CMK_FlowEdge) FlowEdge(filterBlk, res); - - assert(filterBlk->VisitEHSecondPassSuccs(this, [blk](BasicBlock* succ) { - return succ == blk ? BasicBlockVisit::Abort : BasicBlockVisit::Continue; - }) == BasicBlockVisit::Abort); - } - } + res = new (this, CMK_FlowEdge) FlowEdge(filterBlk, res); - enclosing = enclosingDsc->ebdEnclosingTryIndex; + assert(filterBlk->VisitEHSecondPassSuccs(this, [blk](BasicBlock* succ) { + return succ == blk ? BasicBlockVisit::Abort : BasicBlockVisit::Continue; + }) == BasicBlockVisit::Abort); + } } - } -#ifdef DEBUG - unsigned hash = SsaStressHashHelper(); - if (hash != 0) - { - res = ShuffleHelper(hash, res); + enclosing = enclosingDsc->ebdEnclosingTryIndex; } -#endif // DEBUG - ehPreds->Set(blk, res); } + + domPreds->Set(blk, res); return res; } diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 355403baa92684..bdc822bfef68d7 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -1945,6 +1945,7 @@ void Compiler::compInit(ArenaAllocator* pAlloc, #endif m_switchDescMap = nullptr; m_blockToEHPreds = nullptr; + m_dominancePreds = nullptr; m_fieldSeqStore = nullptr; m_refAnyClass = nullptr; for (MemoryKind memoryKind : allMemoryKinds()) @@ -5739,6 +5740,7 @@ void Compiler::ResetOptAnnotations() fgResetForSsa(); vnStore = nullptr; m_blockToEHPreds = nullptr; + m_dominancePreds = nullptr; fgSsaPassesCompleted = 0; fgVNPassesCompleted = 0; fgSsaChecksEnabled = false; diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b043f7f3f4b0cf..6980f6532b6fc8 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2372,12 +2372,8 @@ class Compiler } // Get the index to use as the cache key for sharing throw blocks #endif // !FEATURE_EH_FUNCLETS - // Returns a FlowEdge representing the "EH predecessors" of "blk". These are the normal predecessors of - // "blk", plus one special case: if "blk" is the first block of a handler, considers the predecessor(s) of the - // first block of the corresponding try region to be "EH predecessors". (If there is a single such predecessor, - // for example, we want to consider that the immediate dominator of the catch clause start block, so it's - // convenient to also consider it a predecessor.) FlowEdge* BlockPredsWithEH(BasicBlock* blk); + FlowEdge* BlockDominancePreds(BasicBlock* blk); // This table is useful for memoization of the method above. typedef JitHashTable, FlowEdge*> BlockToFlowEdgeMap; @@ -2391,6 +2387,16 @@ class Compiler return m_blockToEHPreds; } + BlockToFlowEdgeMap* m_dominancePreds; + BlockToFlowEdgeMap* GetDominancePreds() + { + if (m_dominancePreds == nullptr) + { + m_dominancePreds = new (getAllocator()) BlockToFlowEdgeMap(getAllocator()); + } + return m_dominancePreds; + } + void* ehEmitCookie(BasicBlock* block); UNATIVE_OFFSET ehCodeOffset(BasicBlock* block); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 47c9007ddee4fc..3a25901f658411 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -538,69 +538,6 @@ static BasicBlockVisit VisitEHSuccessors(Compiler* comp, BasicBlock* block, TFun return block->VisitEHSecondPassSuccs(comp, func); } -//------------------------------------------------------------------------------ -// VisitSuccessorEHSuccessors: Given a block and one of its regular successors, -// if that regular successor is the beginning of a try, then also visit its -// handlers. -// -// Arguments: -// comp - Compiler instance -// block - The block -// succ - A regular successor of block -// func - Callback -// -// Returns: -// Whether or not the visiting should proceed. -// -// Remarks: -// Because we make the conservative assumption that control flow can jump -// from a try block to its handler at any time, the immediate (regular -// control flow) predecessor(s) of the first block of a try block are also -// considered to have the first block of the handler as an EH successor. -// -// As an example: for liveness this makes variables that are "live-in" to the -// handler become "live-out" for these try-predecessor block, so that they -// become live-in to the try -- which we require. -// -// TODO-Cleanup: Is the above comment true today or is this code unnecessary? -// For a block T with an EH successor E liveness takes care to consider the -// live-in set E as "volatile" variables that are fully live at all points -// within the block T, including being a part of T's live-in set. That means -// that if T is the beginning of a try, then any predecessor of T will -// naturally also have E's live-in set as part of its live-out set. -// -template -static BasicBlockVisit VisitSuccessorEHSuccessors(Compiler* comp, BasicBlock* block, BasicBlock* succ, TFunc func) -{ - if (!comp->bbIsTryBeg(succ)) - { - return BasicBlockVisit::Continue; - } - - unsigned tryIndex = succ->getTryIndex(); - if (comp->bbInExnFlowRegions(tryIndex, block)) - { - // Already yielded as an EH successor of block itself - return BasicBlockVisit::Continue; - } - - EHblkDsc* eh = comp->ehGetDsc(tryIndex); - - do - { - RETURN_ON_ABORT(func(eh->ExFlowBlock())); - - if (eh->ebdEnclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) - { - break; - } - - eh = comp->ehGetDsc(eh->ebdEnclosingTryIndex); - } while (eh->ebdTryBeg == succ); - - return BasicBlockVisit::Continue; -} - //------------------------------------------------------------------------------ // VisitAllSuccs: Visit all successors (including EH successors) of this block. // @@ -622,23 +559,14 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func) RETURN_ON_ABORT(func(bbJumpEhf->bbeSuccs[i])); } - RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - - for (unsigned i = 0; i < bbJumpEhf->bbeCount; i++) - { - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbJumpEhf->bbeSuccs[i], func)); - } - - break; + return VisitEHSuccessors(comp, this, func); case BBJ_CALLFINALLY: case BBJ_EHCATCHRET: case BBJ_EHFILTERRET: case BBJ_LEAVE: RETURN_ON_ABORT(func(bbJumpDest)); - RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbJumpDest, func)); - break; + return VisitEHSuccessors(comp, this, func); case BBJ_ALWAYS: RETURN_ON_ABORT(func(bbJumpDest)); @@ -651,14 +579,12 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func) { RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); } - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbJumpDest, func)); - break; + + return BasicBlockVisit::Continue; case BBJ_NONE: RETURN_ON_ABORT(func(bbNext)); - RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbNext, func)); - break; + return VisitEHSuccessors(comp, this, func); case BBJ_COND: RETURN_ON_ABORT(func(bbNext)); @@ -668,14 +594,7 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func) RETURN_ON_ABORT(func(bbJumpDest)); } - RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbNext, func)); - - if (bbJumpDest != bbNext) - { - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, bbJumpDest, func)); - } - break; + return VisitEHSuccessors(comp, this, func); case BBJ_SWITCH: { @@ -685,27 +604,17 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func) RETURN_ON_ABORT(func(sd.nonDuplicates[i])); } - RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - - for (unsigned i = 0; i < sd.numDistinctSuccs; i++) - { - RETURN_ON_ABORT(VisitSuccessorEHSuccessors(comp, this, sd.nonDuplicates[i], func)); - } - - break; + return VisitEHSuccessors(comp, this, func); } case BBJ_THROW: case BBJ_RETURN: case BBJ_EHFAULTRET: - RETURN_ON_ABORT(VisitEHSuccessors(comp, this, func)); - break; + return VisitEHSuccessors(comp, this, func); default: unreached(); } - - return BasicBlockVisit::Continue; } //------------------------------------------------------------------------------ @@ -728,19 +637,18 @@ BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func) { RETURN_ON_ABORT(func(bbJumpEhf->bbeSuccs[i])); } - break; + + return BasicBlockVisit::Continue; case BBJ_CALLFINALLY: case BBJ_EHCATCHRET: case BBJ_EHFILTERRET: case BBJ_LEAVE: case BBJ_ALWAYS: - RETURN_ON_ABORT(func(bbJumpDest)); - break; + return func(bbJumpDest); case BBJ_NONE: - RETURN_ON_ABORT(func(bbNext)); - break; + return func(bbNext); case BBJ_COND: RETURN_ON_ABORT(func(bbNext)); @@ -750,7 +658,7 @@ BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func) RETURN_ON_ABORT(func(bbJumpDest)); } - break; + return BasicBlockVisit::Continue; case BBJ_SWITCH: { @@ -760,19 +668,17 @@ BasicBlockVisit BasicBlock::VisitRegularSuccs(Compiler* comp, TFunc func) RETURN_ON_ABORT(func(sd.nonDuplicates[i])); } - break; + return BasicBlockVisit::Continue; } case BBJ_THROW: case BBJ_RETURN: case BBJ_EHFAULTRET: - break; + return BasicBlockVisit::Continue; default: unreached(); } - - return BasicBlockVisit::Continue; } #undef RETURN_ON_ABORT diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index 2ad8108c9cd9aa..5601d58864ab17 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -231,12 +231,10 @@ void SsaBuilder::ComputeImmediateDom(BasicBlock** postOrder, int count) JITDUMP("[SsaBuilder::ComputeImmediateDom]\n"); // Add entry point to visited as its IDom is NULL. - BitVecOps::ClearD(&m_visitedTraits, m_visited); - BitVecOps::AddElemD(&m_visitedTraits, m_visited, m_pCompiler->fgFirstBB->bbNum); - assert(postOrder[count - 1] == m_pCompiler->fgFirstBB); - bool changed = true; + unsigned numIters = 0; + bool changed = true; while (changed) { changed = false; @@ -248,40 +246,28 @@ void SsaBuilder::ComputeImmediateDom(BasicBlock** postOrder, int count) DBG_SSA_JITDUMP("Visiting in reverse post order: " FMT_BB ".\n", block->bbNum); - // Find the first processed predecessor block. - BasicBlock* predBlock = nullptr; - for (FlowEdge* pred = m_pCompiler->BlockPredsWithEH(block); pred; pred = pred->getNextPredEdge()) + // Intersect DOM, if computed, for all predecessors. + BasicBlock* bbIDom = nullptr; + for (FlowEdge* pred = m_pCompiler->BlockDominancePreds(block); pred; pred = pred->getNextPredEdge()) { - if (BitVecOps::IsMember(&m_visitedTraits, m_visited, pred->getSourceBlock()->bbNum)) + BasicBlock* domPred = pred->getSourceBlock(); + if ((domPred->bbPostorderNum >= (unsigned)count) || (postOrder[domPred->bbPostorderNum] != domPred)) { - predBlock = pred->getSourceBlock(); - break; + continue; // Unreachable pred } - } - // There could just be a single basic block, so just check if there were any preds. - if (predBlock != nullptr) - { - DBG_SSA_JITDUMP("Pred block is " FMT_BB ".\n", predBlock->bbNum); - } + if ((numIters <= 0) && (domPred->bbPostorderNum <= (unsigned)i)) + { + continue; // Pred not yet visited + } - // Intersect DOM, if computed, for all predecessors. - BasicBlock* bbIDom = predBlock; - for (FlowEdge* pred = m_pCompiler->BlockPredsWithEH(block); pred; pred = pred->getNextPredEdge()) - { - if (predBlock != pred->getSourceBlock()) + if (bbIDom == nullptr) { - BasicBlock* domAncestor = IntersectDom(pred->getSourceBlock(), bbIDom); - // The result may be NULL if "block" and "pred->getBlock()" are part of a - // cycle -- neither is guaranteed ordered wrt the other in reverse postorder, - // so we may be computing the IDom of "block" before the IDom of "pred->getBlock()" has - // been computed. But that's OK -- if they're in a cycle, they share the same immediate - // dominator, so the contribution of "pred->getBlock()" is not necessary to compute - // the result. - if (domAncestor != nullptr) - { - bbIDom = domAncestor; - } + bbIDom = domPred; + } + else + { + bbIDom = IntersectDom(bbIDom, domPred); } } @@ -295,11 +281,10 @@ void SsaBuilder::ComputeImmediateDom(BasicBlock** postOrder, int count) block->bbIDom = bbIDom; } - // Mark the current block as visited. - BitVecOps::AddElemD(&m_visitedTraits, m_visited, block->bbNum); - DBG_SSA_JITDUMP("Marking block " FMT_BB " as processed.\n", block->bbNum); } + + numIters++; } } @@ -341,8 +326,10 @@ void SsaBuilder::ComputeDominanceFrontiers(BasicBlock** postOrder, int count, Bl FlowEdge* blockPreds = m_pCompiler->BlockPredsWithEH(block); - // If block has 0/1 predecessor, skip. - if ((blockPreds == nullptr) || (blockPreds->getNextPredEdge() == nullptr)) + // If block has 0/1 predecessor, skip, apart from handler entry blocks + // that are always in the dominance frontier of its enclosed blocks. + if (!m_pCompiler->bbIsHandlerBeg(block) && + ((blockPreds == nullptr) || (blockPreds->getNextPredEdge() == nullptr))) { DBG_SSA_JITDUMP(" Has %d preds; skipping.\n", blockPreds == nullptr ? 0 : 1); continue; @@ -1310,7 +1297,7 @@ void SsaBuilder::AddPhiArgsToSuccessors(BasicBlock* block) GenTreePhi* phi = tree->AsLclVar()->Data()->AsPhi(); unsigned ssaNum = m_renameStack.Top(lclNum); - AddPhiArg(handlerStart, stmt, phi, lclNum, ssaNum, block); + AddPhiArg(handlerStart, stmt, phi, lclNum, ssaNum, succ); } // Now handle memory.