Skip to content

Commit

Permalink
JIT: Delete old DFS and dominator implementations (dotnet#96927)
Browse files Browse the repository at this point in the history
- Switch the block weighting logic to be based on the new dominator tree
- Remove the old DFS
- Remove the old dominator computation
- Remove the old `DomTreeVisitor` and rename `NewDomTreeVisitor` -> `DomTreeVisitor`
- Rename `BasicBlock::bbNewPostorderNum` -> `BasicBlock::bbPostorderNum`
  • Loading branch information
jakobbotsch authored Jan 17, 2024
1 parent c6966d9 commit 143a9bf
Show file tree
Hide file tree
Showing 18 changed files with 163 additions and 1,005 deletions.
5 changes: 2 additions & 3 deletions src/coreclr/jit/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1609,9 +1609,8 @@ BasicBlock* BasicBlock::New(Compiler* compiler)

block->bbNatLoopNum = BasicBlock::NOT_IN_LOOP;

block->bbPreorderNum = 0;
block->bbPostorderNum = 0;
block->bbNewPostorderNum = 0;
block->bbPreorderNum = 0;
block->bbPostorderNum = 0;

return block;
}
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -1302,9 +1302,8 @@ struct BasicBlock : private LIR::Range

void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts

unsigned bbPreorderNum; // the block's preorder number in the graph (1...fgMaxBBNum]
unsigned bbPostorderNum; // the block's postorder number in the graph (1...fgMaxBBNum]
unsigned bbNewPostorderNum; // the block's postorder number in the graph [0...postOrderCount)
unsigned bbPreorderNum; // the block's preorder number in the graph [0...postOrderCount)
unsigned bbPostorderNum; // the block's postorder number in the graph [0...postOrderCount)

IL_OFFSET bbCodeOffs; // IL offset of the beginning of the block
IL_OFFSET bbCodeOffsEnd; // IL offset past the end of the block. Thus, the [bbCodeOffs..bbCodeOffsEnd)
Expand Down
25 changes: 11 additions & 14 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,6 @@ Histogram bbCntTable(bbCntBuckets);
unsigned bbSizeBuckets[] = {1, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 0};
Histogram bbOneBBSizeTable(bbSizeBuckets);

unsigned domsChangedIterationBuckets[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0};
Histogram domsChangedIterationTable(domsChangedIterationBuckets);

unsigned computeReachabilitySetsIterationBuckets[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0};
Histogram computeReachabilitySetsIterationTable(computeReachabilitySetsIterationBuckets);

Expand Down Expand Up @@ -1562,12 +1559,6 @@ void Compiler::compShutdown()
bbOneBBSizeTable.dump(jitstdout());
jitprintf("--------------------------------------------------\n");

jitprintf("--------------------------------------------------\n");
jitprintf("fgComputeDoms `while (change)` iterations:\n");
jitprintf("--------------------------------------------------\n");
domsChangedIterationTable.dump(jitstdout());
jitprintf("--------------------------------------------------\n");

jitprintf("--------------------------------------------------\n");
jitprintf("fgComputeReachabilitySets `while (change)` iterations:\n");
jitprintf("--------------------------------------------------\n");
Expand Down Expand Up @@ -5836,7 +5827,7 @@ void Compiler::RecomputeFlowGraphAnnotations()
assert(JitConfig.JitOptRepeatCount() > 0);
// Recompute reachability sets, dominators, and loops.
optResetLoopInfo();
fgDomsComputed = false;

fgComputeReachability();
optSetBlockWeights();
optFindLoops();
Expand All @@ -5858,7 +5849,6 @@ void Compiler::RecomputeFlowGraphAnnotations()
optLoopTableValid = false;
optLoopTable = nullptr;
optLoopCount = 0;
fgDomsComputed = false;
}

/*****************************************************************************/
Expand Down Expand Up @@ -9332,8 +9322,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
* cVarsFinal, dVarsFinal : Display the local variable table (call lvaTableDump(FINAL_FRAME_LAYOUT)).
* cBlockPreds, dBlockPreds : Display a block's predecessors (call block->dspPreds()).
* cBlockSuccs, dBlockSuccs : Display a block's successors (call block->dspSuccs(compiler)).
* cReach, dReach : Display all block reachability (call fgDispReach()).
* cDoms, dDoms : Display all block dominators (call fgDispDoms()).
* cReach, dReach : Display all block reachability (call BlockReachabilitySets::Dump).
* cDoms, dDoms : Display all block dominators (call FlowGraphDominatorTree::Dump).
* cLiveness, dLiveness : Display per-block variable liveness (call fgDispBBLiveness()).
* cCVarSet, dCVarSet : Display a "converted" VARSET_TP: the varset is assumed to be tracked variable
* indices. These are converted to variable numbers and sorted. (Calls
Expand Down Expand Up @@ -9658,7 +9648,14 @@ JITDBGAPI void __cdecl cDoms(Compiler* comp)
{
static unsigned sequenceNumber = 0; // separate calls with a number to indicate this function has been called
printf("===================================================================== *Doms %u\n", sequenceNumber++);
comp->fgDispDoms();
if (comp->m_domTree != nullptr)
{
comp->m_domTree->Dump();
}
else
{
printf(" Not computed\n");
}
}

JITDBGAPI void __cdecl cLiveness(Compiler* comp)
Expand Down
156 changes: 9 additions & 147 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1568,22 +1568,6 @@ enum API_ICorJitInfo_Names
API_COUNT
};

enum class FlowGraphUpdates
{
COMPUTE_BASICS = 0, // renumber blocks, reachability, etc
COMPUTE_DOMS = 1 << 0, // recompute dominators
};

inline constexpr FlowGraphUpdates operator|(FlowGraphUpdates a, FlowGraphUpdates b)
{
return (FlowGraphUpdates)((unsigned int)a | (unsigned int)b);
}

inline constexpr FlowGraphUpdates operator&(FlowGraphUpdates a, FlowGraphUpdates b)
{
return (FlowGraphUpdates)((unsigned int)a & (unsigned int)b);
}

// Profile checking options
//
// clang-format off
Expand Down Expand Up @@ -2348,7 +2332,7 @@ class FlowGraphNaturalLoops
class FlowGraphDominatorTree
{
template<typename TVisitor>
friend class NewDomTreeVisitor;
friend class DomTreeVisitor;

const FlowGraphDfsTree* m_dfsTree;
const DomTreeNode* m_domTree;
Expand All @@ -2368,6 +2352,10 @@ class FlowGraphDominatorTree
BasicBlock* Intersect(BasicBlock* block, BasicBlock* block2);
bool Dominates(BasicBlock* dominator, BasicBlock* dominated);

#ifdef DEBUG
void Dump();
#endif

static FlowGraphDominatorTree* Build(const FlowGraphDfsTree* dfsTree);
};

Expand Down Expand Up @@ -5012,15 +5000,6 @@ class Compiler
FlowGraphDominatorTree* m_domTree;
BlockReachabilitySets* m_reachabilitySets;

// 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
// postorder number, respectively. The arrays are indexed by basic block number. (Note that blocks are numbered
// starting from one. Thus, we always waste element zero. This makes debugging easier and makes the code less likely
// to suffer from bugs stemming from forgetting to add or subtract one from the block number to form an array
// index). The arrays are of size fgBBNumMax + 1.
unsigned* fgDomTreePreOrder;
unsigned* fgDomTreePostOrder;

bool fgBBVarSetsInited;

// Track how many artificial ref counts we've added to fgEntryBB (for OSR)
Expand Down Expand Up @@ -5140,7 +5119,6 @@ class Compiler

bool fgModified; // True if the flow graph has been modified recently
bool fgPredsComputed; // Have we computed the bbPreds list
bool fgDomsComputed; // Have we computed the dominator sets?
bool fgReturnBlocksComputed; // Have we computed the return blocks list?
bool fgOptimizedFinally; // Did we optimize any try-finallys?
bool fgCanonicalizedFirstBB; // TODO-Quirk: did we end up canonicalizing first BB?
Expand Down Expand Up @@ -5788,18 +5766,9 @@ class Compiler
void vnPrint(ValueNum vn, unsigned level);
#endif

bool fgDominate(const BasicBlock* b1, const BasicBlock* b2); // Return true if b1 dominates b2

// Dominator computation member functions
// Not exposed outside Compiler
protected:
// Compute immediate dominators, the dominator tree and and its pre/post-order travsersal numbers.
void fgComputeDoms();

BlockSet_ValRet_T fgGetDominatorSet(BasicBlock* block); // Returns a set of blocks that dominate the given block.
// Note: this is relatively slow compared to calling fgDominate(),
// especially if dealing with a single block versus block check.

void fgComputeReturnBlocks(); // Initialize fgReturnBlocks to a list of BBJ_RETURN blocks.

// Remove blocks determined to be unreachable by the 'canRemoveBlock'.
Expand All @@ -5808,34 +5777,10 @@ class Compiler

PhaseStatus fgComputeReachability(); // Perform flow graph node reachability analysis.

PhaseStatus fgComputeDominators(); // Compute (new) dominators
PhaseStatus fgComputeDominators(); // Compute dominators

bool fgRemoveDeadBlocks(); // Identify and remove dead blocks.

BasicBlock* fgIntersectDom(BasicBlock* a, BasicBlock* b); // Intersect two immediate dominator sets.

unsigned fgDfsReversePostorder();
void fgDfsReversePostorderHelper(BasicBlock* block,
BlockSet& visited,
unsigned& preorderIndex,
unsigned& reversePostorderIndex);

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
// (performed by fgComputeDoms), this procedure builds the dominance tree represented
// adjacency lists.

// In order to speed up the queries of the form 'Does A dominates B', we can perform a DFS preorder and postorder
// traversal of the dominance tree and the dominance query will become A dominates B iif preOrder(A) <= preOrder(B)
// && postOrder(A) >= postOrder(B) making the computation O(1).
void fgNumberDomTree(DomTreeNode* domTree);

// When the flow graph changes, we need to update the block numbers, reachability sets,
// dominators, and possibly loops.
//
void fgUpdateChangedFlowGraph(FlowGraphUpdates updates);

public:
enum GCPollType
{
Expand Down Expand Up @@ -6124,7 +6069,6 @@ class Compiler

#ifdef DEBUG

void fgDispDoms();
void fgDispBBLiveness(BasicBlock* block);
void fgDispBBLiveness();
void fgTableDispBasicBlock(BasicBlock* block, int ibcColWidth = 0);
Expand Down Expand Up @@ -12009,94 +11953,13 @@ class GenericTreeWalker final
// A dominator tree visitor implemented using the curiously-recurring-template pattern, similar to GenTreeVisitor.
template <typename TVisitor>
class DomTreeVisitor
{
protected:
Compiler* const m_compiler;
DomTreeNode* const m_domTree;

DomTreeVisitor(Compiler* compiler, DomTreeNode* domTree) : m_compiler(compiler), m_domTree(domTree)
{
}

void Begin()
{
}

void PreOrderVisit(BasicBlock* block)
{
}

void PostOrderVisit(BasicBlock* block)
{
}

void End()
{
}

public:
//------------------------------------------------------------------------
// WalkTree: Walk the dominator tree, starting from fgFirstBB.
//
// Parameter:
// tree - Dominator tree nodes.
//
// Notes:
// This performs a non-recursive, non-allocating walk of the tree by using
// DomTreeNode's firstChild and nextSibling links to locate the children of
// a node and BasicBlock's bbIDom parent link to go back up the tree when
// no more children are left.
//
// Forests are also supported, provided that all the roots are chained via
// DomTreeNode::nextSibling to fgFirstBB.
//
void WalkTree()
{
static_cast<TVisitor*>(this)->Begin();

for (BasicBlock *next, *block = m_compiler->fgFirstBB; block != nullptr; block = next)
{
static_cast<TVisitor*>(this)->PreOrderVisit(block);

next = m_domTree[block->bbNum].firstChild;

if (next != nullptr)
{
assert(next->bbIDom == block);
continue;
}

do
{
static_cast<TVisitor*>(this)->PostOrderVisit(block);

next = m_domTree[block->bbNum].nextSibling;

if (next != nullptr)
{
assert(next->bbIDom == block->bbIDom);
break;
}

block = block->bbIDom;

} while (block != nullptr);
}

static_cast<TVisitor*>(this)->End();
}
};

// A dominator tree visitor implemented using the curiously-recurring-template pattern, similar to GenTreeVisitor.
template <typename TVisitor>
class NewDomTreeVisitor
{
friend class FlowGraphDominatorTree;

protected:
Compiler* m_compiler;

NewDomTreeVisitor(Compiler* compiler) : m_compiler(compiler)
DomTreeVisitor(Compiler* compiler) : m_compiler(compiler)
{
}

Expand Down Expand Up @@ -12125,7 +11988,7 @@ class NewDomTreeVisitor
{
static_cast<TVisitor*>(this)->PreOrderVisit(block);

next = tree[block->bbNewPostorderNum].firstChild;
next = tree[block->bbPostorderNum].firstChild;

if (next != nullptr)
{
Expand All @@ -12137,7 +12000,7 @@ class NewDomTreeVisitor
{
static_cast<TVisitor*>(this)->PostOrderVisit(block);

next = tree[block->bbNewPostorderNum].nextSibling;
next = tree[block->bbPostorderNum].nextSibling;

if (next != nullptr)
{
Expand Down Expand Up @@ -12304,7 +12167,6 @@ extern size_t gcPtrMapNSize;
#if COUNT_BASIC_BLOCKS
extern Histogram bbCntTable;
extern Histogram bbOneBBSizeTable;
extern Histogram domsChangedIterationTable;
extern Histogram computeReachabilitySetsIterationTable;
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5011,7 +5011,7 @@ BasicBlockVisit FlowGraphNaturalLoop::VisitLoopBlocksReversePostOrder(TFunc func
// loop block rpo index = head block rpoIndex + index
// loop block po index = PostOrderCount - 1 - loop block rpo index
// = headPreOrderIndex - index
unsigned poIndex = m_header->bbNewPostorderNum - index;
unsigned poIndex = m_header->bbPostorderNum - index;
assert(poIndex < m_dfsTree->GetPostOrderCount());
return func(m_dfsTree->GetPostOrder(poIndex)) == BasicBlockVisit::Continue;
});
Expand Down Expand Up @@ -5039,7 +5039,7 @@ BasicBlockVisit FlowGraphNaturalLoop::VisitLoopBlocksPostOrder(TFunc func)
{
BitVecTraits traits(m_blocksSize, m_dfsTree->GetCompiler());
bool result = BitVecOps::VisitBitsReverse(&traits, m_blocks, [=](unsigned index) {
unsigned poIndex = m_header->bbNewPostorderNum - index;
unsigned poIndex = m_header->bbPostorderNum - index;
assert(poIndex < m_dfsTree->GetPostOrderCount());
return func(m_dfsTree->GetPostOrder(poIndex)) == BasicBlockVisit::Continue;
});
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/copyprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -454,15 +454,15 @@ PhaseStatus Compiler::optVnCopyProp()

VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::MakeEmpty(this));

class CopyPropDomTreeVisitor : public NewDomTreeVisitor<CopyPropDomTreeVisitor>
class CopyPropDomTreeVisitor : public DomTreeVisitor<CopyPropDomTreeVisitor>
{
// The map from lclNum to its recently live definitions as a stack.
LclNumToLiveDefsMap m_curSsaName;
bool m_madeChanges = false;

public:
CopyPropDomTreeVisitor(Compiler* compiler)
: NewDomTreeVisitor(compiler), m_curSsaName(compiler->getAllocator(CMK_CopyProp)), m_madeChanges(false)
: DomTreeVisitor(compiler), m_curSsaName(compiler->getAllocator(CMK_CopyProp)), m_madeChanges(false)
{
}

Expand Down
Loading

0 comments on commit 143a9bf

Please sign in to comment.