diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index ffb524819dfab4..bdf334ca59bf1c 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -2126,7 +2126,7 @@ class AllSuccessorEnumerator } // Returns the next available successor or `nullptr` if there are no more successors. - BasicBlock* NextSuccessor(Compiler* comp) + BasicBlock* NextSuccessor() { m_curSucc++; if (m_curSucc >= m_numSuccs) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 0028ea1d542446..5fbe5eb98c14ce 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4696,6 +4696,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Run an early flow graph simplification pass // DoPhase(this, PHASE_EARLY_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase); + + // Build post-order + // + DoPhase(this, PHASE_DFS_BLOCKS, &Compiler::fgDfsBlocks); } // Promote struct locals @@ -4778,6 +4782,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl fgRenumberBlocks(); } + if (opts.OptimizationEnabled()) + { + // Build post-order + // + DoPhase(this, PHASE_DFS_BLOCKS, &Compiler::fgDfsBlocks); + } + // GS security checks for unsafe buffers // DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 0f2d6ec1c1a00e..26006d1dc8e65e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4493,8 +4493,8 @@ class Compiler unsigned fgBBNumMax; // The max bbNum that has been assigned to basic blocks unsigned fgDomBBcount; // # of BBs for which we have dominator and reachability information BasicBlock** fgBBReversePostorder; // Blocks in reverse postorder - BasicBlock** fgSSAPostOrder; // Blocks in postorder, computed during SSA - unsigned fgSSAPostOrderCount; // Number of blocks in fgSSAPostOrder + BasicBlock** fgPostOrder; // Blocks in post order + unsigned fgPostOrderCount; // Number of blocks in fgSSAPostOrder // After the dominance tree is computed, we cache a DFS preorder number and DFS postorder number to compute // dominance queries in O(1). fgDomTreePreOrder and fgDomTreePostOrder are arrays giving the block's preorder and @@ -5609,6 +5609,8 @@ class Compiler PhaseStatus fgSetBlockOrder(); + PhaseStatus fgDfsBlocks(); + void fgRemoveReturnBlock(BasicBlock* block); void fgConvertBBToThrowBB(BasicBlock* block); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 486a6de7ce5f83..c07301b59782f0 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -76,8 +76,8 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_BOOLS, "Optimize bools", CompPhaseNameMacro(PHASE_SWITCH_RECOGNITION, "Recognize Switch", false, -1, false) CompPhaseNameMacro(PHASE_FIND_OPER_ORDER, "Find oper order", false, -1, false) CompPhaseNameMacro(PHASE_SET_BLOCK_ORDER, "Set block order", false, -1, true) +CompPhaseNameMacro(PHASE_DFS_BLOCKS, "Set block order from DFS", false, -1, false) CompPhaseNameMacro(PHASE_BUILD_SSA, "Build SSA representation", true, -1, false) -CompPhaseNameMacro(PHASE_BUILD_SSA_TOPOSORT, "SSA: topological sort", false, PHASE_BUILD_SSA, false) CompPhaseNameMacro(PHASE_BUILD_SSA_DOMS, "SSA: Doms1", false, PHASE_BUILD_SSA, false) CompPhaseNameMacro(PHASE_BUILD_SSA_LIVENESS, "SSA: liveness", false, PHASE_BUILD_SSA, false) CompPhaseNameMacro(PHASE_BUILD_SSA_DF, "SSA: DF", false, PHASE_BUILD_SSA, false) diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index b5e51a247bb4c6..1f3cd326cefe6d 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -2976,6 +2976,7 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef bool allNodesLinked = (fgNodeThreading == NodeThreading::AllTrees) || (fgNodeThreading == NodeThreading::LIR); unsigned numBlocks = 0; + unsigned maxBBNum = 0; for (BasicBlock* const block : Blocks()) { @@ -2987,6 +2988,8 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef assert(block->IsLast() || (block->bbNum + 1 == block->Next()->bbNum)); } + maxBBNum = max(maxBBNum, block->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. @@ -3190,6 +3193,7 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef } assert(fgBBcount == numBlocks); + assert(fgBBNumMax >= maxBBNum); // Make sure the one return BB is not changed. if (genReturnBB != nullptr) diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index ca6394f101816e..c202f67161c7b1 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -544,6 +544,94 @@ bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock) return changed; } +PhaseStatus Compiler::fgDfsBlocks() +{ + fgPostOrder = new (this, CMK_BasicBlock) BasicBlock*[fgBBcount]; + BitVecTraits traits(fgBBNumMax + 1, this); + + BitVec visited(BitVecOps::MakeEmpty(&traits)); + + unsigned preOrderIndex = 0; + unsigned postOrderIndex = 0; + + ArrayStack blocks(getAllocator(CMK_BasicBlock)); + + auto dfsFrom = [&](BasicBlock* firstBB) { + + BitVecOps::AddElemD(&traits, visited, firstBB->bbNum); + blocks.Emplace(this, firstBB); + firstBB->bbPreorderNum = preOrderIndex++; + + while (!blocks.Empty()) + { + BasicBlock* block = blocks.TopRef().Block(); + BasicBlock* succ = blocks.TopRef().NextSuccessor(); + + if (succ != nullptr) + { + if (BitVecOps::TryAddElemD(&traits, visited, succ->bbNum)) + { + blocks.Emplace(this, succ); + succ->bbPreorderNum = preOrderIndex++; + } + } + else + { + blocks.Pop(); + fgPostOrder[postOrderIndex] = block; + block->bbPostorderNum = postOrderIndex++; + } + } + + }; + + dfsFrom(fgFirstBB); + + if ((fgEntryBB != nullptr) && !BitVecOps::IsMember(&traits, visited, fgEntryBB->bbNum)) + { + // OSR methods will early on create flow that looks like it goes to the + // patchpoint, but during morph we may transform to something that + // requires the original entry (fgEntryBB). + assert(opts.IsOSR()); + assert((fgEntryBB->bbRefs == 1) && (fgEntryBB->bbPreds == nullptr)); + dfsFrom(fgEntryBB); + } + + if ((genReturnBB != nullptr) && !BitVecOps::IsMember(&traits, visited, genReturnBB->bbNum) && !fgGlobalMorphDone) + { + // We introduce the merged return BB before morph and will redirect + // other returns to it as part of morph; keep it reachable. + dfsFrom(genReturnBB); + } + + PhaseStatus status = PhaseStatus::MODIFIED_NOTHING; + if (postOrderIndex != fgBBcount) + { +#ifdef DEBUG + if (verbose) + { + printf("%u/%u blocks are unreachable and will be removed\n", fgBBcount - postOrderIndex, fgBBcount); + for (BasicBlock* block : Blocks()) + { + if (!BitVecOps::IsMember(&traits, visited, block->bbNum)) + { + printf(" " FMT_BB "\n", block->bbNum); + } + } + } +#endif + + fgRemoveUnreachableBlocks( + [=, &traits](BasicBlock* block) { return !BitVecOps::IsMember(&traits, visited, block->bbNum); }); + + status = PhaseStatus::MODIFIED_EVERYTHING; + } + + fgPostOrderCount = postOrderIndex; + + return status; +} + //------------------------------------------------------------------------ // fgComputeReachability: Compute the dominator and reachable sets. // diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 391ca6758c55e7..ba5885c940c4e3 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14124,8 +14124,9 @@ PhaseStatus Compiler::fgMorphBlocks() // We are done with the global morphing phase // - fgGlobalMorph = false; - compCurBB = nullptr; + fgGlobalMorph = false; + fgGlobalMorphDone = true; + compCurBB = nullptr; #ifdef DEBUG if (optLocalAssertionProp) diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index a1012264e7c27b..5b8641de4d6d58 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -69,8 +69,6 @@ PhaseStatus Compiler::fgSsaBuild() JitTestCheckSSA(); #endif // DEBUG - fgSSAPostOrder = builder.GetPostOrder(&fgSSAPostOrderCount); - return PhaseStatus::MODIFIED_EVERYTHING; } @@ -136,88 +134,6 @@ SsaBuilder::SsaBuilder(Compiler* pCompiler) { } -//------------------------------------------------------------------------ -// TopologicalSort: Topologically sort the graph and return the number of nodes visited. -// -// Arguments: -// postOrder - The array in which the arranged basic blocks have to be returned. -// count - The size of the postOrder array. -// -// Return Value: -// The number of nodes visited while performing DFS on the graph. -// -unsigned SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count) -{ - Compiler* comp = m_pCompiler; - - // TopologicalSort is called first so m_visited should already be empty - assert(BitVecOps::IsEmpty(&m_visitedTraits, m_visited)); - - // Display basic blocks. - DBEXEC(VERBOSE, comp->fgDispBasicBlocks()); - DBEXEC(VERBOSE, comp->fgDispHandlerTab()); - - auto DumpBlockAndSuccessors = [](Compiler* comp, BasicBlock* block) { -#ifdef DEBUG - if (comp->verboseSsa) - { - printf("[SsaBuilder::TopologicalSort] Pushing " FMT_BB ": [", block->bbNum); - AllSuccessorEnumerator successors(comp, block); - unsigned index = 0; - while (true) - { - BasicBlock* succ = successors.NextSuccessor(comp); - - if (succ == nullptr) - { - break; - } - - printf("%s" FMT_BB, (index++ ? ", " : ""), succ->bbNum); - } - printf("]\n"); - } -#endif - }; - - // Compute order. - unsigned postIndex = 0; - BasicBlock* block = comp->fgFirstBB; - BitVecOps::AddElemD(&m_visitedTraits, m_visited, block->bbNum); - - ArrayStack blocks(m_allocator); - blocks.Emplace(comp, block); - DumpBlockAndSuccessors(comp, block); - - while (!blocks.Empty()) - { - BasicBlock* block = blocks.TopRef().Block(); - BasicBlock* succ = blocks.TopRef().NextSuccessor(comp); - - if (succ != nullptr) - { - // if the block on TOS still has unreached successors, visit them - if (BitVecOps::TryAddElemD(&m_visitedTraits, m_visited, succ->bbNum)) - { - blocks.Emplace(comp, succ); - DumpBlockAndSuccessors(comp, succ); - } - } - else - { - // all successors have been visited - blocks.Pop(); - - DBG_SSA_JITDUMP("[SsaBuilder::TopologicalSort] postOrder[%u] = " FMT_BB "\n", postIndex, block->bbNum); - postOrder[postIndex] = block; - block->bbPostorderNum = postIndex; - postIndex++; - } - } - - return postIndex; -} - /** * Computes the immediate dominator IDom for each block iteratively. * @@ -226,10 +142,13 @@ unsigned SsaBuilder::TopologicalSort(BasicBlock** postOrder, int count) * * @see "A simple, fast dominance algorithm." paper. */ -void SsaBuilder::ComputeImmediateDom(BasicBlock** postOrder, int count) +void SsaBuilder::ComputeImmediateDom() { JITDUMP("[SsaBuilder::ComputeImmediateDom]\n"); + BasicBlock** postOrder = m_pCompiler->fgPostOrder; + unsigned count = m_pCompiler->fgPostOrderCount; + // Add entry point to visited as its IDom is NULL. assert(postOrder[count - 1] == m_pCompiler->fgFirstBB); @@ -604,14 +523,14 @@ void SsaBuilder::AddPhiArg( * * To do so, the function computes liveness, dominance frontier and inserts a phi node, * if we have var v in def(b) and live-in(l) and l is in DF(b). - * - * @param postOrder The array of basic blocks arranged in postOrder. - * @param count The size of valid elements in the postOrder array. */ -void SsaBuilder::InsertPhiFunctions(BasicBlock** postOrder, int count) +void SsaBuilder::InsertPhiFunctions() { JITDUMP("*************** In SsaBuilder::InsertPhiFunctions()\n"); + BasicBlock** postOrder = m_pCompiler->fgPostOrder; + unsigned count = m_pCompiler->fgPostOrderCount; + // Compute dominance frontier. BlkToBlkVectorMap mapDF(m_allocator); ComputeDominanceFrontiers(postOrder, count, &mapDF); @@ -622,7 +541,7 @@ void SsaBuilder::InsertPhiFunctions(BasicBlock** postOrder, int count) JITDUMP("Inserting phi functions:\n"); - for (int i = 0; i < count; ++i) + for (unsigned i = 0; i < count; ++i) { BasicBlock* block = postOrder[i]; DBG_SSA_JITDUMP("Considering dominance frontier of block " FMT_BB ":\n", block->bbNum); @@ -1494,8 +1413,6 @@ void SsaBuilder::Build() // Allocate the postOrder array for the graph. - m_postOrder = new (m_allocator) BasicBlock*[blockCount]; - m_visitedTraits = BitVecTraits(blockCount, m_pCompiler); m_visited = BitVecOps::MakeEmpty(&m_visitedTraits); @@ -1511,13 +1428,10 @@ void SsaBuilder::Build() block->bbPostorderNum = 0; } - // Topologically sort the graph. - m_postOrderCount = TopologicalSort(m_postOrder, blockCount); - JITDUMP("[SsaBuilder] Topologically sorted the graph.\n"); - EndPhase(PHASE_BUILD_SSA_TOPOSORT); + m_pCompiler->fgDfsBlocks(); // Compute IDom(b). - ComputeImmediateDom(m_postOrder, m_postOrderCount); + ComputeImmediateDom(); m_pCompiler->fgSsaDomTree = m_pCompiler->fgBuildDomTree(); EndPhase(PHASE_BUILD_SSA_DOMS); @@ -1536,7 +1450,7 @@ void SsaBuilder::Build() } // Insert phi functions. - InsertPhiFunctions(m_postOrder, m_postOrderCount); + InsertPhiFunctions(); // Rename local variables and collect UD information for each ssa var. RenameVariables(); diff --git a/src/coreclr/jit/ssabuilder.h b/src/coreclr/jit/ssabuilder.h index 92faf0f21d05ef..9dd405d774cfc7 100644 --- a/src/coreclr/jit/ssabuilder.h +++ b/src/coreclr/jit/ssabuilder.h @@ -33,29 +33,15 @@ class SsaBuilder // variable are stored in the "per SSA data" on the local descriptor. void Build(); - BasicBlock** GetPostOrder(unsigned* count) - { - *count = m_postOrderCount; - return m_postOrder; - } - private: // Ensures that the basic block graph has a root for the dominator graph, by ensuring // that there is a first block that is not in a try region (adding an empty block for that purpose // if necessary). Eventually should move to Compiler. void SetupBBRoot(); - // Requires "postOrder" to be an array of size "count". Requires "count" to at least - // be the size of the flow graph. Sorts the current compiler's flow-graph and places - // the blocks in post order (i.e., a node's children first) in the array. Returns the - // number of nodes visited while sorting the graph. In other words, valid entries in - // the output array. - unsigned TopologicalSort(BasicBlock** postOrder, int count); - - // Requires "postOrder" to hold the blocks of the flowgraph in topologically sorted - // order. Requires count to be the valid entries in the "postOrder" array. Computes - // each block's immediate dominator and records it in the BasicBlock in bbIDom. - void ComputeImmediateDom(BasicBlock** postOrder, int count); + // Computes each block's immediate dominator and records it in the + // BasicBlock in bbIDom. + void ComputeImmediateDom(); // Compute flow graph dominance frontiers. void ComputeDominanceFrontiers(BasicBlock** postOrder, int count, BlkToBlkVectorMap* mapDF); @@ -73,7 +59,7 @@ class SsaBuilder // Requires "postOrder" to hold the blocks of the flowgraph in topologically sorted order. Requires // count to be the valid entries in the "postOrder" array. Inserts GT_PHI nodes at the beginning // of basic blocks that require them. - void InsertPhiFunctions(BasicBlock** postOrder, int count); + void InsertPhiFunctions(); // Rename all definitions and uses within the compiled method. void RenameVariables(); @@ -110,6 +96,4 @@ class SsaBuilder BitVec m_visited; SsaRenameState m_renameStack; - BasicBlock** m_postOrder = nullptr; - unsigned m_postOrderCount = 0; }; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 774ddd877c0bed..79ecfef0232165 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9668,8 +9668,7 @@ class ValueNumberState // bool IsReachable(BasicBlock* bb) { - return (bb->bbPostorderNum < m_comp->fgSSAPostOrderCount) && - (m_comp->fgSSAPostOrder[bb->bbPostorderNum] == bb) && + return (bb->bbPostorderNum < m_comp->fgPostOrderCount) && (m_comp->fgPostOrder[bb->bbPostorderNum] == bb) && !BitVecOps::IsMember(&m_blockTraits, m_provenUnreachableBlocks, bb->bbNum); } @@ -9850,9 +9849,9 @@ PhaseStatus Compiler::fgValueNumber() // SSA has already computed a post-order taking EH successors into account. // Visiting that in reverse will ensure we visit a block's predecessors // before itself whenever possible. - for (unsigned i = fgSSAPostOrderCount; i != 0; i--) + for (unsigned i = fgPostOrderCount; i != 0; i--) { - BasicBlock* block = fgSSAPostOrder[i - 1]; + BasicBlock* block = fgPostOrder[i - 1]; JITDUMP("Visiting " FMT_BB "\n", block->bbNum); if (block != fgFirstBB)