diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 82a714b303da95..ad5d5e3e10751d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1970,11 +1970,15 @@ class FlowGraphDfsTree BasicBlock** m_postOrder; unsigned m_postOrderCount; + // Whether the DFS that produced the tree found any backedges. + bool m_hasCycle; + public: - FlowGraphDfsTree(Compiler* comp, BasicBlock** postOrder, unsigned postOrderCount) + FlowGraphDfsTree(Compiler* comp, BasicBlock** postOrder, unsigned postOrderCount, bool hasCycle) : m_comp(comp) , m_postOrder(postOrder) , m_postOrderCount(postOrderCount) + , m_hasCycle(hasCycle) { } @@ -2004,6 +2008,11 @@ class FlowGraphDfsTree return BitVecTraits(m_postOrderCount, m_comp); } + bool HasCycle() const + { + return m_hasCycle; + } + bool Contains(BasicBlock* block) const; bool IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) const; }; @@ -2246,10 +2255,6 @@ class FlowGraphNaturalLoops // Collection of loops that were found. jitstd::vector m_loops; - // Whether or not we saw any non-natural loop cycles, also known as - // irreducible loops. - unsigned m_improperLoopHeaders = 0; - FlowGraphNaturalLoops(const FlowGraphDfsTree* dfs); static bool FindNaturalLoopBlocks(FlowGraphNaturalLoop* loop, ArrayStack& worklist); @@ -2264,11 +2269,6 @@ class FlowGraphNaturalLoops return m_loops.size(); } - bool HaveNonNaturalLoopCycles() - { - return m_improperLoopHeaders > 0; - } - FlowGraphNaturalLoop* GetLoopByIndex(unsigned index); FlowGraphNaturalLoop* GetLoopByHeader(BasicBlock* header); @@ -6076,8 +6076,8 @@ class Compiler PhaseStatus fgSetBlockOrder(); bool fgHasCycleWithoutGCSafePoint(); - template - unsigned fgRunDfs(TFuncAssignPreorder assignPreorder, TFuncAssignPostorder assignPostorder); + template + unsigned fgRunDfs(VisitPreorder assignPreorder, VisitPostorder assignPostorder, VisitEdge visitEdge); FlowGraphDfsTree* fgComputeDfs(); void fgInvalidateDfsTree(); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 68b1894fe0537e..4acf9046caaa9e 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4913,16 +4913,19 @@ inline bool Compiler::compCanHavePatchpoints(const char** reason) // Type parameters: // VisitPreorder - Functor type that takes a BasicBlock* and its preorder number // VisitPostorder - Functor type that takes a BasicBlock* and its postorder number +// VisitEdge - Functor type that takes two BasicBlock*. // // Parameters: // visitPreorder - Functor to visit block in its preorder // visitPostorder - Functor to visit block in its postorder +// visitEdge - Functor to visit an edge. Called after visitPreorder (if +// this is the first time the successor is seen). // // Returns: // Number of blocks visited. // -template -unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPostorder) +template +unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPostorder, VisitEdge visitEdge) { BitVecTraits traits(fgBBNumMax + 1, this); BitVec visited(BitVecOps::MakeEmpty(&traits)); @@ -4950,6 +4953,8 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos blocks.Emplace(this, succ); visitPreorder(succ, preOrderIndex++); } + + visitEdge(block, succ); } else { diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 9f9e5bb4785e24..f8fc9d506e680e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -5210,7 +5210,8 @@ void Compiler::fgDebugCheckDfsTree() [=](BasicBlock* block, unsigned postorderNum) { assert(block->bbNewPostorderNum == postorderNum); assert(m_dfsTree->GetPostOrder(postorderNum) == block); - }); + }, + [](BasicBlock* block, BasicBlock* succ) {}); assert(m_dfsTree->GetPostOrderCount() == count); } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index aebae003262303..912f0b91cf22ad 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4079,15 +4079,30 @@ bool FlowGraphDfsTree::IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) FlowGraphDfsTree* Compiler::fgComputeDfs() { BasicBlock** postOrder = new (this, CMK_DepthFirstSearch) BasicBlock*[fgBBcount]; + bool hasCycle = false; - unsigned numBlocks = fgRunDfs([](BasicBlock* block, unsigned preorderNum) { block->bbPreorderNum = preorderNum; }, - [=](BasicBlock* block, unsigned postorderNum) { - block->bbNewPostorderNum = postorderNum; - assert(postorderNum < fgBBcount); - postOrder[postorderNum] = block; - }); + auto visitPreorder = [](BasicBlock* block, unsigned preorderNum) { + block->bbPreorderNum = preorderNum; + block->bbNewPostorderNum = UINT_MAX; + }; + + auto visitPostorder = [=](BasicBlock* block, unsigned postorderNum) { + block->bbNewPostorderNum = postorderNum; + assert(postorderNum < fgBBcount); + postOrder[postorderNum] = block; + }; - return new (this, CMK_DepthFirstSearch) FlowGraphDfsTree(this, postOrder, numBlocks); + auto visitEdge = [&hasCycle](BasicBlock* block, BasicBlock* succ) { + // Check if block -> succ is a backedge, in which case the flow + // graph has a cycle. + if ((succ->bbPreorderNum <= block->bbPreorderNum) && (succ->bbNewPostorderNum == UINT_MAX)) + { + hasCycle = true; + } + }; + + unsigned numBlocks = fgRunDfs(visitPreorder, visitPostorder, visitEdge); + return new (this, CMK_DepthFirstSearch) FlowGraphDfsTree(this, postOrder, numBlocks, hasCycle); } //------------------------------------------------------------------------ @@ -4388,13 +4403,20 @@ FlowGraphNaturalLoops* FlowGraphNaturalLoops::Find(const FlowGraphDfsTree* dfsTr { unsigned rpoNum = dfsTree->GetPostOrderCount() - i; BasicBlock* const block = dfsTree->GetPostOrder(i - 1); - JITDUMP("%02u -> " FMT_BB "[%u, %u]\n", rpoNum + 1, block->bbNum, block->bbPreorderNum + 1, - block->bbNewPostorderNum + 1); + JITDUMP("%02u -> " FMT_BB "[%u, %u]\n", rpoNum, block->bbNum, block->bbPreorderNum, block->bbNewPostorderNum); } + + unsigned improperLoopHeaders = 0; #endif FlowGraphNaturalLoops* loops = new (comp, CMK_Loops) FlowGraphNaturalLoops(dfsTree); + if (!dfsTree->HasCycle()) + { + JITDUMP("Flow graph has no cycles; skipping identification of natural loops\n"); + return loops; + } + ArrayStack worklist(comp->getAllocator(CMK_Loops)); for (unsigned i = dfsTree->GetPostOrderCount(); i != 0; i--) @@ -4439,7 +4461,7 @@ FlowGraphNaturalLoops* FlowGraphNaturalLoops::Find(const FlowGraphDfsTree* dfsTr if (!FindNaturalLoopBlocks(loop, worklist)) { - loops->m_improperLoopHeaders++; + INDEBUG(improperLoopHeaders++); continue; } @@ -4545,9 +4567,9 @@ FlowGraphNaturalLoops* FlowGraphNaturalLoops::Find(const FlowGraphDfsTree* dfsTr JITDUMP("\nFound %zu loops\n", loops->m_loops.size()); } - if (loops->m_improperLoopHeaders > 0) + if (improperLoopHeaders > 0) { - JITDUMP("Rejected %u loop headers\n", loops->m_improperLoopHeaders); + JITDUMP("Rejected %u loop headers\n", improperLoopHeaders); } #endif diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 30218fd0da0cda..46f926a93ec54f 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -5498,9 +5498,15 @@ void Compiler::optFindNewLoops() m_newToOldLoop = m_loops->NumLoops() == 0 ? nullptr : (new (this, CMK_Loops) LoopDsc*[m_loops->NumLoops()]{}); m_oldToNewLoop = new (this, CMK_Loops) FlowGraphNaturalLoop*[BasicBlock::MAX_LOOP_NUM]{}; - // Unnatural loops can quickly become natural if we manage to remove some - // edges, so be conservative here. - fgMightHaveNaturalLoops = (m_loops->NumLoops() > 0) || m_loops->HaveNonNaturalLoopCycles(); + // Leave a bread crumb for future phases like loop alignment about whether + // looking for loops makes sense. We generally do not expect phases to + // introduce new cycles/loops in the flow graph; if they do, they should + // set this to true themselves. + // We use more general cycles over "m_loops->NumLoops() > 0" here because + // future optimizations can easily cause general cycles to become natural + // loops by removing edges. + fgMightHaveNaturalLoops = m_dfsTree->HasCycle(); + assert(fgMightHaveNaturalLoops || (m_loops->NumLoops() == 0)); for (BasicBlock* block : Blocks()) {