Skip to content

Commit 4c09966

Browse files
amanasifkhalidmichaelgsharp
authored andcommitted
JIT: Implement greedy RPO-based block layout (dotnet#101473)
Part of dotnet#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.
1 parent 28b9880 commit 4c09966

File tree

7 files changed

+576
-23
lines changed

7 files changed

+576
-23
lines changed

src/coreclr/jit/block.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,33 +137,40 @@ void FlowEdge::addLikelihood(weight_t addedLikelihood)
137137
// AllSuccessorEnumerator: Construct an instance of the enumerator.
138138
//
139139
// Arguments:
140-
// comp - Compiler instance
141-
// block - The block whose successors are to be iterated
140+
// comp - Compiler instance
141+
// block - The block whose successors are to be iterated
142+
// useProfile - If true, determines the order of successors visited using profile data
142143
//
143-
AllSuccessorEnumerator::AllSuccessorEnumerator(Compiler* comp, BasicBlock* block)
144+
AllSuccessorEnumerator::AllSuccessorEnumerator(Compiler* comp, BasicBlock* block, const bool useProfile /* = false */)
144145
: m_block(block)
145146
{
146147
m_numSuccs = 0;
147-
block->VisitAllSuccs(comp, [this](BasicBlock* succ) {
148+
block->VisitAllSuccs(
149+
comp,
150+
[this](BasicBlock* succ) {
148151
if (m_numSuccs < ArrLen(m_successors))
149152
{
150153
m_successors[m_numSuccs] = succ;
151154
}
152155

153156
m_numSuccs++;
154157
return BasicBlockVisit::Continue;
155-
});
158+
},
159+
useProfile);
156160

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

161165
unsigned numSuccs = 0;
162-
block->VisitAllSuccs(comp, [this, &numSuccs](BasicBlock* succ) {
166+
block->VisitAllSuccs(
167+
comp,
168+
[this, &numSuccs](BasicBlock* succ) {
163169
assert(numSuccs < m_numSuccs);
164170
m_pSuccessors[numSuccs++] = succ;
165171
return BasicBlockVisit::Continue;
166-
});
172+
},
173+
useProfile);
167174

168175
assert(numSuccs == m_numSuccs);
169176
}

src/coreclr/jit/block.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,7 +1799,7 @@ struct BasicBlock : private LIR::Range
17991799
BasicBlockVisit VisitEHEnclosedHandlerSecondPassSuccs(Compiler* comp, TFunc func);
18001800

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

18041804
template <typename TFunc>
18051805
BasicBlockVisit VisitEHSuccs(Compiler* comp, TFunc func);
@@ -2497,7 +2497,7 @@ class AllSuccessorEnumerator
24972497

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

25022502
// Gets the block whose successors are enumerated.
25032503
BasicBlock* Block()

src/coreclr/jit/compiler.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2793,6 +2793,9 @@ class Compiler
27932793
EHblkDsc* ehIsBlockHndLast(BasicBlock* block);
27942794
bool ehIsBlockEHLast(BasicBlock* block);
27952795

2796+
template <typename GetTryLast, typename SetTryLast>
2797+
void ehUpdateTryLasts(GetTryLast getTryLast, SetTryLast setTryLast);
2798+
27962799
bool ehBlockHasExnFlowDsc(BasicBlock* block);
27972800

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

60566059
bool fgReorderBlocks(bool useProfile);
6060+
void fgDoReversePostOrderLayout();
6061+
void fgMoveColdBlocks();
60576062

60586063
bool fgFuncletsAreCold();
60596064

@@ -6074,9 +6079,10 @@ class Compiler
60746079
PhaseStatus fgSetBlockOrder();
60756080
bool fgHasCycleWithoutGCSafePoint();
60766081

6077-
template<typename VisitPreorder, typename VisitPostorder, typename VisitEdge>
6082+
template <typename VisitPreorder, typename VisitPostorder, typename VisitEdge, const bool useProfile = false>
60786083
unsigned fgRunDfs(VisitPreorder assignPreorder, VisitPostorder assignPostorder, VisitEdge visitEdge);
60796084

6085+
template <const bool useProfile = false>
60806086
FlowGraphDfsTree* fgComputeDfs();
60816087
void fgInvalidateDfsTree();
60826088

src/coreclr/jit/compiler.hpp

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -623,12 +623,13 @@ BasicBlockVisit BasicBlock::VisitEHSuccs(Compiler* comp, TFunc func)
623623
// Arguments:
624624
// comp - Compiler instance
625625
// func - Callback
626+
// useProfile - If true, determines the order of successors visited using profile data
626627
//
627628
// Returns:
628629
// Whether or not the visiting was aborted.
629630
//
630631
template <typename TFunc>
631-
BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
632+
BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func, const bool useProfile /* = false */)
632633
{
633634
switch (bbKind)
634635
{
@@ -662,10 +663,22 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
662663
return VisitEHSuccs(comp, func);
663664

664665
case BBJ_COND:
665-
RETURN_ON_ABORT(func(GetFalseTarget()));
666-
667-
if (!TrueEdgeIs(GetFalseEdge()))
666+
if (TrueEdgeIs(GetFalseEdge()))
668667
{
668+
RETURN_ON_ABORT(func(GetFalseTarget()));
669+
}
670+
else if (useProfile && (GetTrueEdge()->getLikelihood() < GetFalseEdge()->getLikelihood()))
671+
{
672+
// When building an RPO-based block layout, we want to visit the unlikely successor first
673+
// so that in the DFS computation, the likely successor will be processed right before this block,
674+
// meaning the RPO-based layout will enable fall-through into the likely successor.
675+
//
676+
RETURN_ON_ABORT(func(GetTrueTarget()));
677+
RETURN_ON_ABORT(func(GetFalseTarget()));
678+
}
679+
else
680+
{
681+
RETURN_ON_ABORT(func(GetFalseTarget()));
669682
RETURN_ON_ABORT(func(GetTrueTarget()));
670683
}
671684

@@ -696,8 +709,8 @@ BasicBlockVisit BasicBlock::VisitAllSuccs(Compiler* comp, TFunc func)
696709
// VisitRegularSuccs: Visit regular successors of this block.
697710
//
698711
// Arguments:
699-
// comp - Compiler instance
700-
// func - Callback
712+
// comp - Compiler instance
713+
// func - Callback
701714
//
702715
// Returns:
703716
// Whether or not the visiting was aborted.
@@ -4745,6 +4758,7 @@ inline bool Compiler::compCanHavePatchpoints(const char** reason)
47454758
// VisitPreorder - Functor type that takes a BasicBlock* and its preorder number
47464759
// VisitPostorder - Functor type that takes a BasicBlock* and its postorder number
47474760
// VisitEdge - Functor type that takes two BasicBlock*.
4761+
// useProfile - If true, determines order of successors visited using profile data
47484762
//
47494763
// Parameters:
47504764
// visitPreorder - Functor to visit block in its preorder
@@ -4755,7 +4769,7 @@ inline bool Compiler::compCanHavePatchpoints(const char** reason)
47554769
// Returns:
47564770
// Number of blocks visited.
47574771
//
4758-
template <typename VisitPreorder, typename VisitPostorder, typename VisitEdge>
4772+
template <typename VisitPreorder, typename VisitPostorder, typename VisitEdge, const bool useProfile /* = false */>
47594773
unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPostorder, VisitEdge visitEdge)
47604774
{
47614775
BitVecTraits traits(fgBBNumMax + 1, this);
@@ -4768,7 +4782,7 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos
47684782

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

47744788
while (!blocks.Empty())
@@ -4780,7 +4794,7 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos
47804794
{
47814795
if (BitVecOps::TryAddElemD(&traits, visited, succ->bbNum))
47824796
{
4783-
blocks.Emplace(this, succ);
4797+
blocks.Emplace(this, succ, useProfile);
47844798
visitPreorder(succ, preOrderIndex++);
47854799
}
47864800

0 commit comments

Comments
 (0)