From 1ff715fcc9616eb28b8d800840693376ce546e43 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 9 Nov 2023 21:47:07 -0800 Subject: [PATCH] JIT: stop computing dom start nodes for morph RPO (#94497) Remove the up-front computation of enter blocks and dom start nodes from the DFS computations used for RPO. Also lay the groundwork for removing unreachable blocks, by tracking the postorder number of the last reachable block. Contributes to #93246. --- src/coreclr/jit/compiler.h | 5 +- src/coreclr/jit/fgopt.cpp | 132 ++++++++++++------------- src/coreclr/jit/fgprofilesynthesis.cpp | 1 - src/coreclr/jit/morph.cpp | 7 +- 4 files changed, 68 insertions(+), 77 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 24aa12b2c0f42..25c8292097886 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5329,15 +5329,12 @@ class Compiler BasicBlock* fgIntersectDom(BasicBlock* a, BasicBlock* b); // Intersect two immediate dominator sets. - void fgDfsReversePostorder(); + unsigned fgDfsReversePostorder(); void fgDfsReversePostorderHelper(BasicBlock* block, BlockSet& visited, unsigned& preorderIndex, unsigned& reversePostorderIndex); - BlockSet_ValRet_T fgDomFindStartNodes(); // Computes which basic blocks don't have incoming edges in the flow graph. - // Returns this as a set. - INDEBUG(void fgDispDomTree(DomTreeNode* domTree);) // Helper that prints out the Dominator Tree in debug builds. DomTreeNode* fgBuildDomTree(); // Once we compute all the immediate dominator sets for each node in the flow graph diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 6e95bb7ce15b9..665aeeee217df 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -763,59 +763,87 @@ bool Compiler::fgRemoveDeadBlocks() //------------------------------------------------------------- // fgDfsReversePostorder: Depth first search to establish block -// preorder and reverse postorder numbers, plus a reverse postorder for blocks. +// preorder and reverse postorder numbers, plus a reverse postorder for blocks, +// using all entry blocks and EH handler blocks as start blocks. // // Notes: -// Assumes caller has computed the fgEnterBlks set. -// // Each block's `bbPreorderNum` and `bbPostorderNum` is set. -// // The `fgBBReversePostorder` array is filled in with the `BasicBlock*` in reverse post-order. -// // This algorithm only pays attention to the actual blocks. It ignores any imaginary entry block. // -void Compiler::fgDfsReversePostorder() +// Unreachable blocks will have higher pre and post order numbers than reachable blocks. +// Hence they will appear at lower indices in the fgBBReversePostorder array. +// +unsigned Compiler::fgDfsReversePostorder() { assert(fgBBcount == fgBBNumMax); assert(BasicBlockBitSetTraits::GetSize(this) == fgBBNumMax + 1); - - // Make sure fgEnterBlks are still there in startNodes, even if they participate in a loop (i.e., there is - // an incoming edge into the block). - assert(fgEnterBlksSetValid); - fgBBReversePostorder = new (this, CMK_DominatorMemory) BasicBlock*[fgBBNumMax + 1]{}; - - // visited : Once we run the DFS post order sort recursive algorithm, we mark the nodes we visited to avoid - // backtracking. BlockSet visited(BlockSetOps::MakeEmpty(this)); - // We begin by figuring out which basic blocks don't have incoming edges and mark them as - // start nodes. Later on we run the recursive algorithm for each node that we - // mark in this step. - BlockSet_ValRet_T startNodes = fgDomFindStartNodes(); - - BlockSetOps::UnionD(this, startNodes, fgEnterBlks); - assert(BlockSetOps::IsMember(this, startNodes, fgFirstBB->bbNum)); - - // Call the flowgraph DFS traversal helper. unsigned preorderIndex = 1; unsigned postorderIndex = 1; - for (BasicBlock* const block : Blocks()) + + // Walk from our primary root. + // + fgDfsReversePostorderHelper(fgFirstBB, visited, preorderIndex, postorderIndex); + + // If we didn't end up visiting everything, try the EH roots. + // + if (preorderIndex != fgBBcount + 1) { - // If the block has no predecessors, and we haven't already visited it (because it's in fgEnterBlks but also - // reachable from the first block), go ahead and traverse starting from this block. - if (BlockSetOps::IsMember(this, startNodes, block->bbNum) && - !BlockSetOps::IsMember(this, visited, block->bbNum)) + for (EHblkDsc* const HBtab : EHClauses(this)) { - fgDfsReversePostorderHelper(block, visited, preorderIndex, postorderIndex); + if (HBtab->HasFilter()) + { + BasicBlock* const filterBlock = HBtab->ebdFilter; + if (!BlockSetOps::IsMember(this, visited, filterBlock->bbNum)) + { + fgDfsReversePostorderHelper(filterBlock, visited, preorderIndex, postorderIndex); + } + } + + BasicBlock* const handlerBlock = HBtab->ebdHndBeg; + if (!BlockSetOps::IsMember(this, visited, handlerBlock->bbNum)) + { + fgDfsReversePostorderHelper(handlerBlock, visited, preorderIndex, postorderIndex); + } + +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + // For ARM code, prevent creating retless calls by visiting the call finally successors + // + if (HBtab->HasFinallyHandler()) + { + for (BasicBlock* const finallyPredBlock : handlerBlock->PredBlocks()) + { + // In some rare cases the finally is a loop entry. + // + if (!finallyPredBlock->KindIs(BBJ_CALLFINALLY)) + { + continue; + } + assert(finallyPredBlock->isBBCallAlwaysPair()); + BasicBlock* const pairTailBlock = finallyPredBlock->Next(); + + if (!BlockSetOps::IsMember(this, visited, pairTailBlock->bbNum)) + { + fgDfsReversePostorderHelper(pairTailBlock, visited, preorderIndex, postorderIndex); + } + } + } +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) } } - // If there are still unvisited blocks (say isolated cycles), visit them too. + // That's everything reachable from the roots. // - if (preorderIndex != fgBBcount + 1) + const unsigned highestReachablePostorderNumber = postorderIndex - 1; + + // If we still didn't end up visiting everything, visit what remains. + // + if (highestReachablePostorderNumber != fgBBcount) { - JITDUMP("DFS: flow graph has some isolated cycles, doing extra traversals\n"); + JITDUMP("DFS: there are %u unreachable blocks\n", fgBBcount - highestReachablePostorderNumber); for (BasicBlock* const block : Blocks()) { if (!BlockSetOps::IsMember(this, visited, block->bbNum)) @@ -841,48 +869,12 @@ void Compiler::fgDfsReversePostorder() printf("\n"); } #endif // DEBUG -} - -//------------------------------------------------------------- -// fgDomFindStartNodes: Helper for dominance computation to find the start nodes block set. -// -// The start nodes is a set that represents which basic blocks in the flow graph don't have incoming edges. -// We begin assuming everything is a start block and remove any block that is a successor of another. -// -// Returns: -// Block set of start nodes. -// -BlockSet_ValRet_T Compiler::fgDomFindStartNodes() -{ - BlockSet startNodes(BlockSetOps::MakeFull(this)); - - for (BasicBlock* const block : Blocks()) - { - for (BasicBlock* const succ : block->Succs(this)) - { - BlockSetOps::RemoveElemD(this, startNodes, succ->bbNum); - } - } - -#ifdef DEBUG - if (verbose) - { - printf("\nDominator computation start blocks (those blocks with no incoming edges):\n"); - BlockSetOps::Iter iter(this, startNodes); - unsigned bbNum = 0; - while (iter.NextElem(&bbNum)) - { - printf(FMT_BB " ", bbNum); - } - printf("\n"); - } -#endif // DEBUG - return startNodes; + return highestReachablePostorderNumber; } //------------------------------------------------------------------------ -// fgDfsReversePostorderHelper: Helper to assign post-order numbers to blocks. +// fgDfsReversePostorderHelper: Helper to assign post-order numbers to blocks // // Arguments: // block - The starting entry block diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index fbea28ed4107c..c9204c1e971e6 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -790,7 +790,6 @@ void ProfileSynthesis::RandomizeLikelihoods() void ProfileSynthesis::BuildReversePostorder() { m_comp->EnsureBasicBlockEpoch(); - m_comp->fgComputeEnterBlocksSet(); m_comp->fgDfsReversePostorder(); // Build map from bbNum to Block*. diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1c24bc8ef8455..52d13dbe7ff59 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13938,8 +13938,6 @@ PhaseStatus Compiler::fgMorphBlocks() // We are optimizing. Process in RPO. // fgRenumberBlocks(); - EnsureBasicBlockEpoch(); - fgComputeEnterBlocksSet(); fgDfsReversePostorder(); // Disallow general creation of new blocks or edges as it @@ -13963,7 +13961,12 @@ PhaseStatus Compiler::fgMorphBlocks() fgFirstBB->Next()->bbFlags |= BBF_CAN_ADD_PRED; } + // Remember this so we can sanity check that no new blocks will get created. + // unsigned const bbNumMax = fgBBNumMax; + + // Morph the blocks in RPO. + // for (unsigned i = 1; i <= bbNumMax; i++) { BasicBlock* const block = fgBBReversePostorder[i];