Skip to content

Commit

Permalink
JIT: Run 3-opt once across all regions (#111989)
Browse files Browse the repository at this point in the history
Part of #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.
  • Loading branch information
amanasifkhalid authored Jan 30, 2025
1 parent aef8f42 commit 84f4c2a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 61 deletions.
3 changes: 1 addition & 2 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6326,7 +6326,6 @@ class Compiler
BasicBlock** blockOrder;
BasicBlock** tempOrder;
unsigned numCandidateBlocks;
unsigned currEHRegion;

#ifdef DEBUG
weight_t GetLayoutCost(unsigned startPos, unsigned endPos);
Expand All @@ -6341,7 +6340,7 @@ class Compiler
void AddNonFallthroughPreds(unsigned blockPos);
bool RunGreedyThreeOptPass(unsigned startPos, unsigned endPos);

bool RunThreeOptPass(BasicBlock* startBlock, BasicBlock* endBlock);
bool RunThreeOptPass();

public:
ThreeOptLayout(Compiler* comp);
Expand Down
82 changes: 23 additions & 59 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4931,7 +4931,6 @@ Compiler::ThreeOptLayout::ThreeOptLayout(Compiler* comp)
, blockOrder(nullptr)
, tempOrder(nullptr)
, numCandidateBlocks(0)
, currEHRegion(0)
{
}

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

// Ignore cross-region branches
if ((srcBlk->bbTryIndex != currEHRegion) || (dstBlk->bbTryIndex != currEHRegion))
if (!BasicBlock::sameTryRegion(srcBlk, dstBlk))
{
return;
}
Expand Down Expand Up @@ -5224,8 +5223,7 @@ void Compiler::ThreeOptLayout::AddNonFallthroughPreds(unsigned blockPos)
}

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

// Repurpose 'bbPostorderNum' for the block's ordinal
block->bbPostorderNum = numCandidateBlocks++;

// While walking the span of blocks to reorder,
// remember where each try region ends within this span.
// We'll use this information to run 3-opt per region.
EHblkDsc* const HBtab = compiler->ehGetBlockTryDsc(block);
if (HBtab != nullptr)
{
HBtab->ebdTryLast = block;
}
}

// Reorder try regions first
bool modified = false;
for (EHblkDsc* const HBtab : EHClauses(compiler))
{
// If multiple region indices map to the same region,
// make sure we reorder its blocks only once
BasicBlock* const tryBeg = HBtab->ebdTryBeg;
if (tryBeg->getTryIndex() != currEHRegion++)
{
continue;
}

// Only reorder try regions within the candidate span of blocks
if ((tryBeg->bbPostorderNum < numCandidateBlocks) && (blockOrder[tryBeg->bbPostorderNum] == tryBeg))
{
JITDUMP("Running 3-opt for try region #%d\n", (currEHRegion - 1));
modified |= RunThreeOptPass(tryBeg, HBtab->ebdTryLast);
}
}

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

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

if (block->NextIs(next))
{
continue;
}

// Only reorder within EH regions to maintain contiguity.
// TODO: Allow moving blocks in different regions when 'next' is the region entry.
// This would allow us to move entire regions up/down because of the contiguity requirement.
if (!block->NextIs(next) && BasicBlock::sameEHRegion(block, next))
if (!BasicBlock::sameEHRegion(block, next))
{
continue;
}

// Don't move the entry of an EH region.
if (compiler->bbIsTryBeg(next) || compiler->bbIsHandlerBeg(next))
{
compiler->fgUnlinkBlock(next);
compiler->fgInsertBBafter(block, next);
continue;
}

compiler->fgUnlinkBlock(next);
compiler->fgInsertBBafter(block, next);
}
}
}
Expand Down Expand Up @@ -5466,12 +5443,6 @@ bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned
continue;
}

// Don't consider any cut points that would disturb other EH regions
if (!BasicBlock::sameEHRegion(s2Block, s3Block))
{
continue;
}

// Compute the cost delta of this partition
const weight_t currCost = currCostBase + GetCost(s3BlockPrev, s3Block);
const weight_t newCost =
Expand Down Expand Up @@ -5529,22 +5500,15 @@ bool Compiler::ThreeOptLayout::RunGreedyThreeOptPass(unsigned startPos, unsigned
}

//-----------------------------------------------------------------------------
// Compiler::ThreeOptLayout::RunThreeOptPass: Runs 3-opt for the given block range.
//
// Parameters:
// startBlock - The first block of the range to reorder
// endBlock - The last block (inclusive) of the range to reorder
// Compiler::ThreeOptLayout::RunThreeOptPass: Runs 3-opt on the candidate span of blocks.
//
// Returns:
// True if we reordered anything, false otherwise
//
bool Compiler::ThreeOptLayout::RunThreeOptPass(BasicBlock* startBlock, BasicBlock* endBlock)
bool Compiler::ThreeOptLayout::RunThreeOptPass()
{
assert(startBlock != nullptr);
assert(endBlock != nullptr);

const unsigned startPos = startBlock->bbPostorderNum;
const unsigned endPos = endBlock->bbPostorderNum;
const unsigned startPos = 0;
const unsigned endPos = numCandidateBlocks - 1;
const unsigned numBlocks = (endPos - startPos + 1);
assert(startPos <= endPos);

Expand Down

0 comments on commit 84f4c2a

Please sign in to comment.