Skip to content

Commit 3cf22b7

Browse files
amanasifkhalidamanasifkhalid
authored
JIT: Template EH-specific checks in 3-opt layout (#111437)
3-opt performs various checks within hot loops to maintain EH invariants. Since we know beforehand if the method has EH regions or not, we can save a bit of throughput in the latter case by skipping these checks. Also, relax an invariant around considering edges out of call-finally pairs.
1 parent 3d27554 commit 3cf22b7

File tree

2 files changed

+99
-69
lines changed

2 files changed

+99
-69
lines changed

‎src/coreclr/jit/compiler.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -6181,6 +6181,7 @@ class Compiler
61816181
bool fgReorderBlocks(bool useProfile);
61826182
PhaseStatus fgSearchImprovedLayout();
61836183

6184+
template <bool hasEH>
61846185
class ThreeOptLayout
61856186
{
61866187
static bool EdgeCmp(const FlowEdge* left, const FlowEdge* right);
@@ -6192,10 +6193,9 @@ class Compiler
61926193
BasicBlock** tempOrder;
61936194
unsigned numCandidateBlocks;
61946195

6195-
#ifdef DEBUG
6196-
weight_t GetLayoutCost(unsigned startPos, unsigned endPos);
6197-
#endif // DEBUG
6196+
bool IsCandidateBlock(BasicBlock* block) const;
61986197

6198+
INDEBUG(weight_t GetLayoutCost(unsigned startPos, unsigned endPos);)
61996199
weight_t GetCost(BasicBlock* block, BasicBlock* next);
62006200
weight_t GetPartitionCostDelta(unsigned s2Start, unsigned s3Start, unsigned s3End, unsigned s4End);
62016201
void SwapPartitions(unsigned s1Start, unsigned s2Start, unsigned s3Start, unsigned s3End, unsigned s4End);

‎src/coreclr/jit/fgopt.cpp

+96-66
Original file line numberDiff line numberDiff line change
@@ -4459,7 +4459,8 @@ bool Compiler::fgReorderBlocks(bool useProfile)
44594459
// Returns:
44604460
// True if 'right' should be considered before 'left', and false otherwise
44614461
//
4462-
/* static */ bool Compiler::ThreeOptLayout::EdgeCmp(const FlowEdge* left, const FlowEdge* right)
4462+
template <bool hasEH>
4463+
/* static */ bool Compiler::ThreeOptLayout<hasEH>::EdgeCmp(const FlowEdge* left, const FlowEdge* right)
44634464
{
44644465
assert(left != right);
44654466
const weight_t leftWeight = left->getLikelyWeight();
@@ -4494,7 +4495,8 @@ bool Compiler::fgReorderBlocks(bool useProfile)
44944495
// To save an allocation, we will reuse the DFS tree's underlying array for 'tempOrder'.
44954496
// This means we will trash the DFS tree.
44964497
//
4497-
Compiler::ThreeOptLayout::ThreeOptLayout(Compiler* comp, BasicBlock** initialLayout, unsigned numHotBlocks)
4498+
template <bool hasEH>
4499+
Compiler::ThreeOptLayout<hasEH>::ThreeOptLayout(Compiler* comp, BasicBlock** initialLayout, unsigned numHotBlocks)
44984500
: compiler(comp)
44994501
, cutPoints(comp->getAllocator(CMK_FlowEdge), &ThreeOptLayout::EdgeCmp)
45004502
, blockOrder(initialLayout)
@@ -4503,6 +4505,23 @@ Compiler::ThreeOptLayout::ThreeOptLayout(Compiler* comp, BasicBlock** initialLay
45034505
{
45044506
}
45054507

4508+
//-----------------------------------------------------------------------------
4509+
// Compiler::ThreeOptLayout::IsCandidateBlock: Determines if a block is being considered for reordering
4510+
// by checking if it is in 'blockOrder'.
4511+
//
4512+
// Parameters:
4513+
// block - the block to check
4514+
//
4515+
// Returns:
4516+
// True if 'block' is in the set of candidate blocks, false otherwise
4517+
//
4518+
template <bool hasEH>
4519+
bool Compiler::ThreeOptLayout<hasEH>::IsCandidateBlock(BasicBlock* block) const
4520+
{
4521+
assert(block != nullptr);
4522+
return (block->bbPreorderNum < numCandidateBlocks) && (blockOrder[block->bbPreorderNum] == block);
4523+
}
4524+
45064525
#ifdef DEBUG
45074526
//-----------------------------------------------------------------------------
45084527
// Compiler::ThreeOptLayout::GetLayoutCost: Computes the cost of the layout for the region
@@ -4515,7 +4534,8 @@ Compiler::ThreeOptLayout::ThreeOptLayout(Compiler* comp, BasicBlock** initialLay
45154534
// Returns:
45164535
// The region's layout cost
45174536
//
4518-
weight_t Compiler::ThreeOptLayout::GetLayoutCost(unsigned startPos, unsigned endPos)
4537+
template <bool hasEH>
4538+
weight_t Compiler::ThreeOptLayout<hasEH>::GetLayoutCost(unsigned startPos, unsigned endPos)
45194539
{
45204540
assert(startPos <= endPos);
45214541
assert(endPos < numCandidateBlocks);
@@ -4542,7 +4562,8 @@ weight_t Compiler::ThreeOptLayout::GetLayoutCost(unsigned startPos, unsigned end
45424562
// Returns:
45434563
// The cost
45444564
//
4545-
weight_t Compiler::ThreeOptLayout::GetCost(BasicBlock* block, BasicBlock* next)
4565+
template <bool hasEH>
4566+
weight_t Compiler::ThreeOptLayout<hasEH>::GetCost(BasicBlock* block, BasicBlock* next)
45464567
{
45474568
assert(block != nullptr);
45484569
assert(next != nullptr);
@@ -4574,10 +4595,11 @@ weight_t Compiler::ThreeOptLayout::GetCost(BasicBlock* block, BasicBlock* next)
45744595
// The difference in cost between the current and proposed layouts.
45754596
// A negative delta indicates the proposed layout is an improvement.
45764597
//
4577-
weight_t Compiler::ThreeOptLayout::GetPartitionCostDelta(unsigned s2Start,
4578-
unsigned s3Start,
4579-
unsigned s3End,
4580-
unsigned s4End)
4598+
template <bool hasEH>
4599+
weight_t Compiler::ThreeOptLayout<hasEH>::GetPartitionCostDelta(unsigned s2Start,
4600+
unsigned s3Start,
4601+
unsigned s3End,
4602+
unsigned s4End)
45814603
{
45824604
BasicBlock* const s2Block = blockOrder[s2Start];
45834605
BasicBlock* const s2BlockPrev = blockOrder[s2Start - 1];
@@ -4632,7 +4654,8 @@ weight_t Compiler::ThreeOptLayout::GetPartitionCostDelta(unsigned s2Start,
46324654
//
46334655
// If 's3End' and 's4End' are the same, the fourth partition doesn't exist.
46344656
//
4635-
void Compiler::ThreeOptLayout::SwapPartitions(
4657+
template <bool hasEH>
4658+
void Compiler::ThreeOptLayout<hasEH>::SwapPartitions(
46364659
unsigned s1Start, unsigned s2Start, unsigned s3Start, unsigned s3End, unsigned s4End)
46374660
{
46384661
INDEBUG(const weight_t currLayoutCost = GetLayoutCost(s1Start, s4End));
@@ -4691,13 +4714,14 @@ void Compiler::ThreeOptLayout::SwapPartitions(
46914714
// Returns:
46924715
// True if 'edge' can be considered for aligning, false otherwise
46934716
//
4717+
template <bool hasEH>
46944718
template <bool addToQueue>
4695-
bool Compiler::ThreeOptLayout::ConsiderEdge(FlowEdge* edge)
4719+
bool Compiler::ThreeOptLayout<hasEH>::ConsiderEdge(FlowEdge* edge)
46964720
{
46974721
assert(edge != nullptr);
46984722

4699-
// Don't add an edge that we've already considered
4700-
// (For exceptionally branchy methods, we want to avoid exploding 'cutPoints' in size)
4723+
// Don't add an edge that we've already considered.
4724+
// For exceptionally branchy methods, we want to avoid exploding 'cutPoints' in size.
47014725
if (addToQueue && edge->visited())
47024726
{
47034727
return false;
@@ -4706,45 +4730,26 @@ bool Compiler::ThreeOptLayout::ConsiderEdge(FlowEdge* edge)
47064730
BasicBlock* const srcBlk = edge->getSourceBlock();
47074731
BasicBlock* const dstBlk = edge->getDestinationBlock();
47084732

4709-
// Ignore cross-region branches
4710-
if (!BasicBlock::sameTryRegion(srcBlk, dstBlk))
4733+
// Don't consider edges to or from outside the hot range.
4734+
if (!IsCandidateBlock(srcBlk) || !IsCandidateBlock(dstBlk))
47114735
{
47124736
return false;
47134737
}
47144738

4715-
// For backward jumps, we will consider partitioning before 'srcBlk'.
4716-
// If 'srcBlk' is a BBJ_CALLFINALLYRET, this partition will split up a call-finally pair.
4717-
// Thus, don't consider edges out of BBJ_CALLFINALLYRET blocks.
4718-
if (srcBlk->KindIs(BBJ_CALLFINALLYRET))
4739+
// Don't consider single-block loop backedges.
4740+
if (srcBlk == dstBlk)
47194741
{
47204742
return false;
47214743
}
47224744

4723-
const unsigned srcPos = srcBlk->bbPreorderNum;
4724-
const unsigned dstPos = dstBlk->bbPreorderNum;
4725-
assert(srcPos < compiler->m_dfsTree->GetPostOrderCount());
4726-
assert(dstPos < compiler->m_dfsTree->GetPostOrderCount());
4727-
4728-
// Don't consider edges to or from outside the hot range (i.e. ordinal doesn't match 'blockOrder' position).
4729-
if ((srcPos >= numCandidateBlocks) || (srcBlk != blockOrder[srcPos]))
4730-
{
4731-
return false;
4732-
}
4733-
4734-
if ((dstPos >= numCandidateBlocks) || (dstBlk != blockOrder[dstPos]))
4745+
// Don't move the method entry block.
4746+
if (dstBlk->IsFirst())
47354747
{
47364748
return false;
47374749
}
47384750

4739-
// Don't consider edges to blocks outside the hot range (i.e. ordinal number isn't set),
4740-
// or backedges to the first block in a region; we don't want to change the entry point.
4741-
if ((dstPos == 0) || compiler->bbIsTryBeg(dstBlk))
4742-
{
4743-
return false;
4744-
}
4745-
4746-
// Don't consider backedges for single-block loops
4747-
if (srcPos == dstPos)
4751+
// Ignore cross-region branches, and don't try to change the region's entry block.
4752+
if (hasEH && (!BasicBlock::sameTryRegion(srcBlk, dstBlk) || compiler->bbIsTryBeg(dstBlk)))
47484753
{
47494754
return false;
47504755
}
@@ -4765,7 +4770,8 @@ bool Compiler::ThreeOptLayout::ConsiderEdge(FlowEdge* edge)
47654770
// Parameters:
47664771
// blockPos - The index into 'blockOrder' of the source block
47674772
//
4768-
void Compiler::ThreeOptLayout::AddNonFallthroughSuccs(unsigned blockPos)
4773+
template <bool hasEH>
4774+
void Compiler::ThreeOptLayout<hasEH>::AddNonFallthroughSuccs(unsigned blockPos)
47694775
{
47704776
assert(blockPos < numCandidateBlocks);
47714777
BasicBlock* const block = blockOrder[blockPos];
@@ -4787,7 +4793,8 @@ void Compiler::ThreeOptLayout::AddNonFallthroughSuccs(unsigned blockPos)
47874793
// Parameters:
47884794
// blockPos - The index into 'blockOrder' of the target block
47894795
//
4790-
void Compiler::ThreeOptLayout::AddNonFallthroughPreds(unsigned blockPos)
4796+
template <bool hasEH>
4797+
void Compiler::ThreeOptLayout<hasEH>::AddNonFallthroughPreds(unsigned blockPos)
47914798
{
47924799
assert(blockPos < numCandidateBlocks);
47934800
BasicBlock* const block = blockOrder[blockPos];
@@ -4809,7 +4816,8 @@ void Compiler::ThreeOptLayout::AddNonFallthroughPreds(unsigned blockPos)
48094816
// Returns:
48104817
// True if any blocks were moved
48114818
//
4812-
bool Compiler::ThreeOptLayout::Run()
4819+
template <bool hasEH>
4820+
bool Compiler::ThreeOptLayout<hasEH>::Run()
48134821
{
48144822
assert(numCandidateBlocks > 0);
48154823
RunThreeOpt();
@@ -4834,7 +4842,8 @@ bool Compiler::ThreeOptLayout::Run()
48344842
// and try to create fallthrough on each edge via partition swaps, starting with the hottest edges.
48354843
// For each swap, repopulate the priority queue with edges along the modified cut points.
48364844
//
4837-
bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned endPos)
4845+
template <bool hasEH>
4846+
bool Compiler::ThreeOptLayout<hasEH>::RunGreedyThreeOptPass(unsigned startPos, unsigned endPos)
48384847
{
48394848
assert(cutPoints.Empty());
48404849
assert(startPos < endPos);
@@ -4952,7 +4961,7 @@ bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned
49524961
BasicBlock* const s3BlockPrev = blockOrder[position - 1];
49534962

49544963
// Don't consider any cut points that would break up call-finally pairs
4955-
if (s3Block->KindIs(BBJ_CALLFINALLYRET))
4964+
if (hasEH && s3Block->KindIs(BBJ_CALLFINALLYRET))
49564965
{
49574966
continue;
49584967
}
@@ -5012,7 +5021,8 @@ bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned
50125021
//-----------------------------------------------------------------------------
50135022
// Compiler::ThreeOptLayout::RunThreeOpt: Runs 3-opt on the candidate span of blocks.
50145023
//
5015-
void Compiler::ThreeOptLayout::RunThreeOpt()
5024+
template <bool hasEH>
5025+
void Compiler::ThreeOptLayout<hasEH>::RunThreeOpt()
50165026
{
50175027
// For methods with fewer than three candidate blocks, we cannot partition anything
50185028
if (numCandidateBlocks < 3)
@@ -5047,28 +5057,47 @@ void Compiler::ThreeOptLayout::RunThreeOpt()
50475057
// Returns:
50485058
// True if any blocks were moved
50495059
//
5050-
bool Compiler::ThreeOptLayout::ReorderBlockList()
5060+
template <bool hasEH>
5061+
bool Compiler::ThreeOptLayout<hasEH>::ReorderBlockList()
50515062
{
50525063
// As we reorder blocks, remember the last candidate block we found in each region.
50535064
// In case we cannot place two blocks next to each other because they are in different regions,
50545065
// we will instead place the latter block after the last one we saw in its region.
50555066
// This ensures cold blocks sink to the end of their respective regions.
50565067
// This will also push nested regions further down the method, but we will move them later, anyway.
5057-
BasicBlock** const lastHotBlocks = new (compiler, CMK_BasicBlock) BasicBlock* [compiler->compHndBBtabCount + 1] {};
5058-
lastHotBlocks[0] = compiler->fgFirstBB;
5068+
BasicBlock** lastHotBlocks = nullptr;
50595069

5060-
for (EHblkDsc* const HBtab : EHClauses(compiler))
5070+
if (hasEH)
50615071
{
5062-
lastHotBlocks[HBtab->ebdTryBeg->bbTryIndex] = HBtab->ebdTryBeg;
5072+
lastHotBlocks = new (compiler, CMK_BasicBlock) BasicBlock* [compiler->compHndBBtabCount + 1] {};
5073+
lastHotBlocks[0] = compiler->fgFirstBB;
5074+
5075+
for (EHblkDsc* const HBtab : EHClauses(compiler))
5076+
{
5077+
lastHotBlocks[HBtab->ebdTryBeg->bbTryIndex] = HBtab->ebdTryBeg;
5078+
}
50635079
}
50645080

50655081
// Reorder the block list.
50665082
JITDUMP("Reordering block list\n");
50675083
bool modified = false;
50685084
for (unsigned i = 1; i < numCandidateBlocks; i++)
50695085
{
5070-
BasicBlock* const block = blockOrder[i - 1];
5071-
BasicBlock* const blockToMove = blockOrder[i];
5086+
BasicBlock* const block = blockOrder[i - 1];
5087+
BasicBlock* const blockToMove = blockOrder[i];
5088+
5089+
if (!hasEH)
5090+
{
5091+
if (!block->NextIs(blockToMove))
5092+
{
5093+
compiler->fgUnlinkBlock(blockToMove);
5094+
compiler->fgInsertBBafter(block, blockToMove);
5095+
modified = true;
5096+
}
5097+
5098+
continue;
5099+
}
5100+
50725101
lastHotBlocks[block->bbTryIndex] = block;
50735102

50745103
// Don't move call-finally pair tails independently.
@@ -5121,7 +5150,7 @@ bool Compiler::ThreeOptLayout::ReorderBlockList()
51215150
}
51225151
}
51235152

5124-
if (compiler->compHndBBtabCount == 0)
5153+
if (!hasEH)
51255154
{
51265155
return modified;
51275156
}
@@ -5141,8 +5170,7 @@ bool Compiler::ThreeOptLayout::ReorderBlockList()
51415170
// If this try region isn't in the candidate span of blocks, don't consider it.
51425171
// Also, if this try region's entry is also the method entry, don't move it.
51435172
BasicBlock* const tryBeg = HBtab->ebdTryBeg;
5144-
if ((tryBeg->bbPreorderNum >= numCandidateBlocks) || (blockOrder[tryBeg->bbPreorderNum] != tryBeg) ||
5145-
tryBeg->IsFirst())
5173+
if (!IsCandidateBlock(tryBeg) || tryBeg->IsFirst())
51465174
{
51475175
continue;
51485176
}
@@ -5190,17 +5218,14 @@ bool Compiler::ThreeOptLayout::ReorderBlockList()
51905218
// Compiler::ThreeOptLayout::CompactHotJumps: Move blocks in the candidate span
51915219
// closer to their most-likely successors.
51925220
//
5193-
void Compiler::ThreeOptLayout::CompactHotJumps()
5221+
template <bool hasEH>
5222+
void Compiler::ThreeOptLayout<hasEH>::CompactHotJumps()
51945223
{
51955224
JITDUMP("Compacting hot jumps\n");
51965225

5197-
auto isCandidateBlock = [this](BasicBlock* block) {
5198-
return (block->bbPreorderNum < numCandidateBlocks) && (blockOrder[block->bbPreorderNum] == block);
5199-
};
5200-
52015226
auto isBackwardJump = [&](BasicBlock* block, BasicBlock* target) {
5202-
assert(isCandidateBlock(block));
5203-
assert(isCandidateBlock(target));
5227+
assert(IsCandidateBlock(block));
5228+
assert(IsCandidateBlock(target));
52045229
return block->bbPreorderNum >= target->bbPreorderNum;
52055230
};
52065231

@@ -5232,7 +5257,7 @@ void Compiler::ThreeOptLayout::CompactHotJumps()
52325257
// If we aren't sure which successor is hotter, and we already fall into one of them,
52335258
// do nothing.
52345259
BasicBlock* const unlikelyTarget = unlikelyEdge->getDestinationBlock();
5235-
if ((unlikelyEdge->getLikelihood() == 0.5) && isCandidateBlock(unlikelyTarget) &&
5260+
if ((unlikelyEdge->getLikelihood() == 0.5) && IsCandidateBlock(unlikelyTarget) &&
52365261
(unlikelyTarget->bbPreorderNum == (i + 1)))
52375262
{
52385263
continue;
@@ -5401,14 +5426,19 @@ PhaseStatus Compiler::fgSearchImprovedLayout()
54015426
}
54025427

54035428
bool modified = false;
5404-
if (numHotBlocks > 0)
5429+
if (numHotBlocks == 0)
54055430
{
5406-
ThreeOptLayout layoutRunner(this, initialLayout, numHotBlocks);
5431+
JITDUMP("No hot blocks found. Skipping reordering.\n");
5432+
}
5433+
else if (compHndBBtabCount == 0)
5434+
{
5435+
ThreeOptLayout</* hasEH */ false> layoutRunner(this, initialLayout, numHotBlocks);
54075436
modified = layoutRunner.Run();
54085437
}
54095438
else
54105439
{
5411-
JITDUMP("No hot blocks found. Skipping reordering.\n");
5440+
ThreeOptLayout</* hasEH */ true> layoutRunner(this, initialLayout, numHotBlocks);
5441+
modified = layoutRunner.Run();
54125442
}
54135443

54145444
// 3-opt will mess with post-order numbers regardless of whether it modifies anything,

0 commit comments

Comments
 (0)