Skip to content

Commit 84f4c2a

Browse files
JIT: Run 3-opt once across all regions (dotnet#111989)
Part of dotnet#107749. 3-opt layout already refuses to align branches that cross EH regions, so there doesn't seem to be much utility in reordering each EH region independently. Thus, remove 3-opt's per-region ordering constraints, and run 3-opt once.
1 parent aef8f42 commit 84f4c2a

File tree

2 files changed

+24
-61
lines changed

2 files changed

+24
-61
lines changed

src/coreclr/jit/compiler.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -6326,7 +6326,6 @@ class Compiler
63266326
BasicBlock** blockOrder;
63276327
BasicBlock** tempOrder;
63286328
unsigned numCandidateBlocks;
6329-
unsigned currEHRegion;
63306329

63316330
#ifdef DEBUG
63326331
weight_t GetLayoutCost(unsigned startPos, unsigned endPos);
@@ -6341,7 +6340,7 @@ class Compiler
63416340
void AddNonFallthroughPreds(unsigned blockPos);
63426341
bool RunGreedyThreeOptPass(unsigned startPos, unsigned endPos);
63436342

6344-
bool RunThreeOptPass(BasicBlock* startBlock, BasicBlock* endBlock);
6343+
bool RunThreeOptPass();
63456344

63466345
public:
63476346
ThreeOptLayout(Compiler* comp);

src/coreclr/jit/fgopt.cpp

+23-59
Original file line numberDiff line numberDiff line change
@@ -4931,7 +4931,6 @@ Compiler::ThreeOptLayout::ThreeOptLayout(Compiler* comp)
49314931
, blockOrder(nullptr)
49324932
, tempOrder(nullptr)
49334933
, numCandidateBlocks(0)
4934-
, currEHRegion(0)
49354934
{
49364935
}
49374936

@@ -5125,7 +5124,7 @@ void Compiler::ThreeOptLayout::ConsiderEdge(FlowEdge* edge)
51255124
BasicBlock* const dstBlk = edge->getDestinationBlock();
51265125

51275126
// Ignore cross-region branches
5128-
if ((srcBlk->bbTryIndex != currEHRegion) || (dstBlk->bbTryIndex != currEHRegion))
5127+
if (!BasicBlock::sameTryRegion(srcBlk, dstBlk))
51295128
{
51305129
return;
51315130
}
@@ -5224,8 +5223,7 @@ void Compiler::ThreeOptLayout::AddNonFallthroughPreds(unsigned blockPos)
52245223
}
52255224

52265225
//-----------------------------------------------------------------------------
5227-
// Compiler::ThreeOptLayout::Run: Runs 3-opt for each contiguous region of the block list
5228-
// we're interested in reordering.
5226+
// Compiler::ThreeOptLayout::Run: Runs 3-opt on the candidate span of hot blocks.
52295227
// We skip reordering handler regions for now, as these are assumed to be cold.
52305228
//
52315229
void Compiler::ThreeOptLayout::Run()
@@ -5271,41 +5269,9 @@ void Compiler::ThreeOptLayout::Run()
52715269

52725270
// Repurpose 'bbPostorderNum' for the block's ordinal
52735271
block->bbPostorderNum = numCandidateBlocks++;
5274-
5275-
// While walking the span of blocks to reorder,
5276-
// remember where each try region ends within this span.
5277-
// We'll use this information to run 3-opt per region.
5278-
EHblkDsc* const HBtab = compiler->ehGetBlockTryDsc(block);
5279-
if (HBtab != nullptr)
5280-
{
5281-
HBtab->ebdTryLast = block;
5282-
}
5283-
}
5284-
5285-
// Reorder try regions first
5286-
bool modified = false;
5287-
for (EHblkDsc* const HBtab : EHClauses(compiler))
5288-
{
5289-
// If multiple region indices map to the same region,
5290-
// make sure we reorder its blocks only once
5291-
BasicBlock* const tryBeg = HBtab->ebdTryBeg;
5292-
if (tryBeg->getTryIndex() != currEHRegion++)
5293-
{
5294-
continue;
5295-
}
5296-
5297-
// Only reorder try regions within the candidate span of blocks
5298-
if ((tryBeg->bbPostorderNum < numCandidateBlocks) && (blockOrder[tryBeg->bbPostorderNum] == tryBeg))
5299-
{
5300-
JITDUMP("Running 3-opt for try region #%d\n", (currEHRegion - 1));
5301-
modified |= RunThreeOptPass(tryBeg, HBtab->ebdTryLast);
5302-
}
53035272
}
53045273

5305-
// Finally, reorder the main method body
5306-
currEHRegion = 0;
5307-
JITDUMP("Running 3-opt for main method body\n");
5308-
modified |= RunThreeOptPass(compiler->fgFirstBB, blockOrder[numCandidateBlocks - 1]);
5274+
const bool modified = RunThreeOptPass();
53095275

53105276
if (modified)
53115277
{
@@ -5314,14 +5280,25 @@ void Compiler::ThreeOptLayout::Run()
53145280
BasicBlock* const block = blockOrder[i - 1];
53155281
BasicBlock* const next = blockOrder[i];
53165282

5283+
if (block->NextIs(next))
5284+
{
5285+
continue;
5286+
}
5287+
53175288
// Only reorder within EH regions to maintain contiguity.
5318-
// TODO: Allow moving blocks in different regions when 'next' is the region entry.
5319-
// This would allow us to move entire regions up/down because of the contiguity requirement.
5320-
if (!block->NextIs(next) && BasicBlock::sameEHRegion(block, next))
5289+
if (!BasicBlock::sameEHRegion(block, next))
5290+
{
5291+
continue;
5292+
}
5293+
5294+
// Don't move the entry of an EH region.
5295+
if (compiler->bbIsTryBeg(next) || compiler->bbIsHandlerBeg(next))
53215296
{
5322-
compiler->fgUnlinkBlock(next);
5323-
compiler->fgInsertBBafter(block, next);
5297+
continue;
53245298
}
5299+
5300+
compiler->fgUnlinkBlock(next);
5301+
compiler->fgInsertBBafter(block, next);
53255302
}
53265303
}
53275304
}
@@ -5466,12 +5443,6 @@ bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned
54665443
continue;
54675444
}
54685445

5469-
// Don't consider any cut points that would disturb other EH regions
5470-
if (!BasicBlock::sameEHRegion(s2Block, s3Block))
5471-
{
5472-
continue;
5473-
}
5474-
54755446
// Compute the cost delta of this partition
54765447
const weight_t currCost = currCostBase + GetCost(s3BlockPrev, s3Block);
54775448
const weight_t newCost =
@@ -5529,22 +5500,15 @@ bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned
55295500
}
55305501

55315502
//-----------------------------------------------------------------------------
5532-
// Compiler::ThreeOptLayout::RunThreeOptPass: Runs 3-opt for the given block range.
5533-
//
5534-
// Parameters:
5535-
// startBlock - The first block of the range to reorder
5536-
// endBlock - The last block (inclusive) of the range to reorder
5503+
// Compiler::ThreeOptLayout::RunThreeOptPass: Runs 3-opt on the candidate span of blocks.
55375504
//
55385505
// Returns:
55395506
// True if we reordered anything, false otherwise
55405507
//
5541-
bool Compiler::ThreeOptLayout::RunThreeOptPass(BasicBlock* startBlock, BasicBlock* endBlock)
5508+
bool Compiler::ThreeOptLayout::RunThreeOptPass()
55425509
{
5543-
assert(startBlock != nullptr);
5544-
assert(endBlock != nullptr);
5545-
5546-
const unsigned startPos = startBlock->bbPostorderNum;
5547-
const unsigned endPos = endBlock->bbPostorderNum;
5510+
const unsigned startPos = 0;
5511+
const unsigned endPos = numCandidateBlocks - 1;
55485512
const unsigned numBlocks = (endPos - startPos + 1);
55495513
assert(startPos <= endPos);
55505514

0 commit comments

Comments
 (0)