Skip to content

Commit dacc035

Browse files
committed
JIT: stop computing dom start nodes for morph RPO
We don't need quite this broad of a start node set, as the DFS will find unreachable blocks on its own. Also lay the groundwork for removing unreachable blocks, by tracking the postorder number of the last reachable block. Contributes to dotnet#93246.
1 parent 76a995a commit dacc035

File tree

3 files changed

+63
-30
lines changed

3 files changed

+63
-30
lines changed

src/coreclr/jit/compiler.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -5330,13 +5330,14 @@ class Compiler
53305330
BasicBlock* fgIntersectDom(BasicBlock* a, BasicBlock* b); // Intersect two immediate dominator sets.
53315331

53325332
void fgDfsReversePostorder();
5333+
unsigned fgDfsReversePostorderCore(BlockSet_ValArg_T startBlocks);
53335334
void fgDfsReversePostorderHelper(BasicBlock* block,
53345335
BlockSet& visited,
53355336
unsigned& preorderIndex,
53365337
unsigned& reversePostorderIndex);
53375338

5338-
BlockSet_ValRet_T fgDomFindStartNodes(); // Computes which basic blocks don't have incoming edges in the flow graph.
5339-
// Returns this as a set.
5339+
BlockSet_ValRet_T fgDomFindStartBlocks(); // Computes which basic blocks don't have incoming edges in the flow graph.
5340+
// Returns this as a set.
53405341

53415342
INDEBUG(void fgDispDomTree(DomTreeNode* domTree);) // Helper that prints out the Dominator Tree in debug builds.
53425343

src/coreclr/jit/fgopt.cpp

+43-25
Original file line numberDiff line numberDiff line change
@@ -763,54 +763,70 @@ bool Compiler::fgRemoveDeadBlocks()
763763

764764
//-------------------------------------------------------------
765765
// fgDfsReversePostorder: Depth first search to establish block
766-
// preorder and reverse postorder numbers, plus a reverse postorder for blocks.
766+
// preorder and reverse postorder numbers, plus a reverse postorder for blocks,
767+
// using all entry blocks and blocks without preds as start lblocks.
767768
//
768769
// Notes:
769-
// Assumes caller has computed the fgEnterBlks set.
770-
//
771770
// Each block's `bbPreorderNum` and `bbPostorderNum` is set.
772-
//
773771
// The `fgBBReversePostorder` array is filled in with the `BasicBlock*` in reverse post-order.
774-
//
775772
// This algorithm only pays attention to the actual blocks. It ignores any imaginary entry block.
776773
//
777774
void Compiler::fgDfsReversePostorder()
778775
{
779-
assert(fgBBcount == fgBBNumMax);
780-
assert(BasicBlockBitSetTraits::GetSize(this) == fgBBNumMax + 1);
781-
782776
// Make sure fgEnterBlks are still there in startNodes, even if they participate in a loop (i.e., there is
783777
// an incoming edge into the block).
784778
assert(fgEnterBlksSetValid);
785779

786-
fgBBReversePostorder = new (this, CMK_DominatorMemory) BasicBlock*[fgBBNumMax + 1]{};
787-
788-
// visited : Once we run the DFS post order sort recursive algorithm, we mark the nodes we visited to avoid
789-
// backtracking.
790-
BlockSet visited(BlockSetOps::MakeEmpty(this));
791-
792780
// We begin by figuring out which basic blocks don't have incoming edges and mark them as
793-
// start nodes. Later on we run the recursive algorithm for each node that we
781+
// start blocks. Later on we run the recursive algorithm for each node that we
794782
// mark in this step.
795-
BlockSet_ValRet_T startNodes = fgDomFindStartNodes();
783+
BlockSet_ValRet_T startBlocks = fgDomFindStartBlocks();
784+
BlockSetOps::UnionD(this, startBlocks, fgEnterBlks);
785+
assert(BlockSetOps::IsMember(this, startBlocks, fgFirstBB->bbNum));
796786

797-
BlockSetOps::UnionD(this, startNodes, fgEnterBlks);
798-
assert(BlockSetOps::IsMember(this, startNodes, fgFirstBB->bbNum));
787+
fgDfsReversePostorderCore(startBlocks);
788+
}
789+
790+
//-------------------------------------------------------------
791+
// fgDfsReversePostorderCore: Depth first search to establish block
792+
// preorder and reverse postorder numbers, plus a reverse postorder for blocks,
793+
// using a specified set of start blocks.
794+
//
795+
// Arguments:
796+
// startBlocks: set of blocks to consider as DFS roots
797+
//
798+
// Returns:
799+
// postorder number of last block reachable from the root set. Any block
800+
// with a postorder number higher than this was not reachable from the root set.
801+
//
802+
// Notes:
803+
// Each block's `bbPreorderNum` and `bbPostorderNum` is set.
804+
// The `fgBBReversePostorder` array is filled in with the `BasicBlock*` in reverse post-order.
805+
// This algorithm only pays attention to the actual blocks. It ignores any imaginary entry block.
806+
//
807+
unsigned Compiler::fgDfsReversePostorderCore(BlockSet_ValArg_T startBlocks)
808+
{
809+
assert(fgBBcount == fgBBNumMax);
810+
assert(BasicBlockBitSetTraits::GetSize(this) == fgBBNumMax + 1);
811+
fgBBReversePostorder = new (this, CMK_DominatorMemory) BasicBlock*[fgBBNumMax + 1]{};
812+
BlockSet visited(BlockSetOps::MakeEmpty(this));
799813

800814
// Call the flowgraph DFS traversal helper.
801815
unsigned preorderIndex = 1;
802816
unsigned postorderIndex = 1;
803817
for (BasicBlock* const block : Blocks())
804818
{
805-
// If the block has no predecessors, and we haven't already visited it (because it's in fgEnterBlks but also
806-
// reachable from the first block), go ahead and traverse starting from this block.
807-
if (BlockSetOps::IsMember(this, startNodes, block->bbNum) &&
819+
// Spawn a DFS from each unvisited start block.
820+
//
821+
if (BlockSetOps::IsMember(this, startBlocks, block->bbNum) &&
808822
!BlockSetOps::IsMember(this, visited, block->bbNum))
809823
{
810824
fgDfsReversePostorderHelper(block, visited, preorderIndex, postorderIndex);
811825
}
812826
}
813827

828+
const unsigned lastReachablePostorderNumber = postorderIndex;
829+
814830
// If there are still unvisited blocks (say isolated cycles), visit them too.
815831
//
816832
if (preorderIndex != fgBBcount + 1)
@@ -841,18 +857,20 @@ void Compiler::fgDfsReversePostorder()
841857
printf("\n");
842858
}
843859
#endif // DEBUG
860+
861+
return lastReachablePostorderNumber;
844862
}
845863

846864
//-------------------------------------------------------------
847-
// fgDomFindStartNodes: Helper for dominance computation to find the start nodes block set.
865+
// fgDomFindStartBlocks: Helper for dominance computation to find the start block set.
848866
//
849-
// The start nodes is a set that represents which basic blocks in the flow graph don't have incoming edges.
867+
// The start block set represents basic blocks in the flow graph that do not have incoming edges.
850868
// We begin assuming everything is a start block and remove any block that is a successor of another.
851869
//
852870
// Returns:
853-
// Block set of start nodes.
871+
// Block set describing the start blocks.
854872
//
855-
BlockSet_ValRet_T Compiler::fgDomFindStartNodes()
873+
BlockSet_ValRet_T Compiler::fgDomFindStartBlocks()
856874
{
857875
BlockSet startNodes(BlockSetOps::MakeFull(this));
858876

src/coreclr/jit/morph.cpp

+17-3
Original file line numberDiff line numberDiff line change
@@ -13871,7 +13871,7 @@ PhaseStatus Compiler::fgMorphBlocks()
1387113871
fgRenumberBlocks();
1387213872
EnsureBasicBlockEpoch();
1387313873
fgComputeEnterBlocksSet();
13874-
fgDfsReversePostorder();
13874+
const unsigned lastReachablePostorderNumber = fgDfsReversePostorderCore(fgEnterBlks);
1387513875

1387613876
// Disallow general creation of new blocks or edges as it
1387713877
// would invalidate RPO.
@@ -13894,8 +13894,22 @@ PhaseStatus Compiler::fgMorphBlocks()
1389413894
fgFirstBB->Next()->bbFlags |= BBF_CAN_ADD_PRED;
1389513895
}
1389613896

13897-
unsigned const bbNumMax = fgBBNumMax;
13898-
for (unsigned i = 1; i <= bbNumMax; i++)
13897+
// Remember this so we can sanity check that no new blocks will get created.
13898+
//
13899+
INDEBUG(unsigned const bbNumMax = fgBBNumMax;);
13900+
13901+
// Todo: verify enter block set is sufficient to allow actual removal here.
13902+
// Likely need to add genReturnBB, fgEntryBB (for OSR), and perhaps more.
13903+
//
13904+
if (lastReachablePostorderNumber < fgBBNumMax)
13905+
{
13906+
JITDUMP("Method has %u unreachable blocks, consider removing them\n",
13907+
fgBBNumMax - lastReachablePostorderNumber);
13908+
}
13909+
13910+
// Morph the blocks in RPO.
13911+
//
13912+
for (unsigned i = 1; i <= fgBBNumMax; i++)
1389913913
{
1390013914
BasicBlock* const block = fgBBReversePostorder[i];
1390113915
fgMorphBlock(block);

0 commit comments

Comments
 (0)