Skip to content

Commit

Permalink
JIT: Implement greedy RPO-based block layout (#101473)
Browse files Browse the repository at this point in the history
Part of #93020. Compiler::fgDoReversePostOrderLayout reorders blocks based on a RPO of the flowgraph's successor edges. When reordering based on the RPO, we only reorder blocks within the same EH region to avoid breaking up their contiguousness. After establishing an RPO-based layout, we do another pass to move cold blocks to the ends of their regions in fgMoveColdBlocks.

The "greedy" part of this layout isn't all that greedy just yet. For now, we use edge likelihoods to make placement decisions only for BBJ_COND blocks' successors. I plan to extend this greediness to other multi-successor block kinds (BBJ_SWITCH, etc) in a follow-up so we can independently evaluate the value in doing so.

This new layout is disabled by default for now.
  • Loading branch information
amanasifkhalid authored May 2, 2024
1 parent 40c024f commit 28c39bc
Show file tree
Hide file tree
Showing 7 changed files with 576 additions and 23 deletions.
21 changes: 14 additions & 7 deletions src/coreclr/jit/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,33 +137,40 @@ void FlowEdge::addLikelihood(weight_t addedLikelihood)
// AllSuccessorEnumerator: Construct an instance of the enumerator.
//
// Arguments:
// comp - Compiler instance
// block - The block whose successors are to be iterated
// comp - Compiler instance
// block - The block whose successors are to be iterated
// useProfile - If true, determines the order of successors visited using profile data
//
AllSuccessorEnumerator::AllSuccessorEnumerator(Compiler* comp, BasicBlock* block)
AllSuccessorEnumerator::AllSuccessorEnumerator(Compiler* comp, BasicBlock* block, const bool useProfile /* = false */)
: m_block(block)
{
m_numSuccs = 0;
block->VisitAllSuccs(comp, [this](BasicBlock* succ) {
block->VisitAllSuccs(
comp,
[this](BasicBlock* succ) {
if (m_numSuccs < ArrLen(m_successors))
{
m_successors[m_numSuccs] = succ;
}

m_numSuccs++;
return BasicBlockVisit::Continue;
});
},
useProfile);

if (m_numSuccs > ArrLen(m_successors))
{
m_pSuccessors = new (comp, CMK_BasicBlock) BasicBlock*[m_numSuccs];

unsigned numSuccs = 0;
block->VisitAllSuccs(comp, [this, &numSuccs](BasicBlock* succ) {
block->VisitAllSuccs(
comp,
[this, &numSuccs](BasicBlock* succ) {
assert(numSuccs < m_numSuccs);
m_pSuccessors[numSuccs++] = succ;
return BasicBlockVisit::Continue;
});
},
useProfile);

assert(numSuccs == m_numSuccs);
}
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -1799,7 +1799,7 @@ struct BasicBlock : private LIR::Range
BasicBlockVisit VisitEHEnclosedHandlerSecondPassSuccs(Compiler* comp, TFunc func);

template <typename TFunc>
BasicBlockVisit VisitAllSuccs(Compiler* comp, TFunc func);
BasicBlockVisit VisitAllSuccs(Compiler* comp, TFunc func, const bool useProfile = false);

template <typename TFunc>
BasicBlockVisit VisitEHSuccs(Compiler* comp, TFunc func);
Expand Down Expand Up @@ -2497,7 +2497,7 @@ class AllSuccessorEnumerator

public:
// Constructs an enumerator of all `block`'s successors.
AllSuccessorEnumerator(Compiler* comp, BasicBlock* block);
AllSuccessorEnumerator(Compiler* comp, BasicBlock* block, const bool useProfile = false);

// Gets the block whose successors are enumerated.
BasicBlock* Block()
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -2793,6 +2793,9 @@ class Compiler
EHblkDsc* ehIsBlockHndLast(BasicBlock* block);
bool ehIsBlockEHLast(BasicBlock* block);

template <typename GetTryLast, typename SetTryLast>
void ehUpdateTryLasts(GetTryLast getTryLast, SetTryLast setTryLast);

bool ehBlockHasExnFlowDsc(BasicBlock* block);

// Return the region index of the most nested EH region this block is in.
Expand Down Expand Up @@ -6054,6 +6057,8 @@ class Compiler
bool fgComputeCalledCount(weight_t returnWeight);

bool fgReorderBlocks(bool useProfile);
void fgDoReversePostOrderLayout();
void fgMoveColdBlocks();

bool fgFuncletsAreCold();

Expand All @@ -6074,9 +6079,10 @@ class Compiler
PhaseStatus fgSetBlockOrder();
bool fgHasCycleWithoutGCSafePoint();

template<typename VisitPreorder, typename VisitPostorder, typename VisitEdge>
template <typename VisitPreorder, typename VisitPostorder, typename VisitEdge, const bool useProfile = false>
unsigned fgRunDfs(VisitPreorder assignPreorder, VisitPostorder assignPostorder, VisitEdge visitEdge);

template <const bool useProfile = false>
FlowGraphDfsTree* fgComputeDfs();
void fgInvalidateDfsTree();

Expand Down
32 changes: 23 additions & 9 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,12 +623,13 @@ BasicBlockVisit BasicBlock::VisitEHSuccs(Compiler* comp, TFunc func)
// Arguments:
// comp - Compiler instance
// func - Callback
// useProfile - If true, determines the order of successors visited using profile data
//
// Returns:
// Whether or not the visiting was aborted.
//
template <typename TFunc>
BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func, const bool useProfile /* = false */)
{
switch (bbKind)
{
Expand Down Expand Up @@ -662,10 +663,22 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
return VisitEHSuccs(comp, func);

case BBJ_COND:
RETURN_ON_ABORT(func(GetFalseTarget()));

if (!TrueEdgeIs(GetFalseEdge()))
if (TrueEdgeIs(GetFalseEdge()))
{
RETURN_ON_ABORT(func(GetFalseTarget()));
}
else if (useProfile && (GetTrueEdge()->getLikelihood() < GetFalseEdge()->getLikelihood()))
{
// When building an RPO-based block layout, we want to visit the unlikely successor first
// so that in the DFS computation, the likely successor will be processed right before this block,
// meaning the RPO-based layout will enable fall-through into the likely successor.
//
RETURN_ON_ABORT(func(GetTrueTarget()));
RETURN_ON_ABORT(func(GetFalseTarget()));
}
else
{
RETURN_ON_ABORT(func(GetFalseTarget()));
RETURN_ON_ABORT(func(GetTrueTarget()));
}

Expand Down Expand Up @@ -696,8 +709,8 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
// VisitRegularSuccs: Visit regular successors of this block.
//
// Arguments:
// comp - Compiler instance
// func - Callback
// comp - Compiler instance
// func - Callback
//
// Returns:
// Whether or not the visiting was aborted.
Expand Down Expand Up @@ -4745,6 +4758,7 @@ inline bool Compiler::compCanHavePatchpoints(const char** reason)
// 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*.
// useProfile - If true, determines order of successors visited using profile data
//
// Parameters:
// visitPreorder - Functor to visit block in its preorder
Expand All @@ -4755,7 +4769,7 @@ inline bool Compiler::compCanHavePatchpoints(const char** reason)
// Returns:
// Number of blocks visited.
//
template <typename VisitPreorder, typename VisitPostorder, typename VisitEdge>
template <typename VisitPreorder, typename VisitPostorder, typename VisitEdge, const bool useProfile /* = false */>
unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPostorder, VisitEdge visitEdge)
{
BitVecTraits traits(fgBBNumMax + 1, this);
Expand All @@ -4768,7 +4782,7 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos

auto dfsFrom = [&](BasicBlock* firstBB) {
BitVecOps::AddElemD(&traits, visited, firstBB->bbNum);
blocks.Emplace(this, firstBB);
blocks.Emplace(this, firstBB, useProfile);
visitPreorder(firstBB, preOrderIndex++);

while (!blocks.Empty())
Expand All @@ -4780,7 +4794,7 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos
{
if (BitVecOps::TryAddElemD(&traits, visited, succ->bbNum))
{
blocks.Emplace(this, succ);
blocks.Emplace(this, succ, useProfile);
visitPreorder(succ, preOrderIndex++);
}

Expand Down
Loading

0 comments on commit 28c39bc

Please sign in to comment.