Skip to content

Commit

Permalink
JIT: Compact blocks in fgMoveBackwardJumpsToSuccessors (dotnet#102512)
Browse files Browse the repository at this point in the history
Follow-up to dotnet#102461, and part of dotnet#9304. Compacting blocks after establishing an RPO-based layout, but before moving backward jumps to fall into their successors, can enable more opportunities for branch removal; see dotnet#9304 for one such example.
  • Loading branch information
amanasifkhalid authored and steveharter committed May 28, 2024
1 parent 3456bac commit 2762be8
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -6044,7 +6044,7 @@ class Compiler

bool fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext);

void fgCompactBlocks(BasicBlock* block, BasicBlock* bNext);
void fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(bool doDebugCheck = true));

BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst);

Expand Down
27 changes: 23 additions & 4 deletions src/coreclr/jit/fgopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,10 @@ bool Compiler::fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext)
// Arguments:
// block - move all code into this block.
// bNext - bbNext of `block`. This block will be removed.
// doDebugCheck - in Debug builds, check flowgraph for correctness after compaction
// (some callers might compact blocks during destructive flowgraph changes, and thus should skip checks)
//
void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(bool doDebugCheck /* = true */))
{
noway_assert(block != nullptr);
noway_assert(bNext != nullptr);
Expand Down Expand Up @@ -1331,7 +1333,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext)
#endif

#if DEBUG
if (JitConfig.JitSlowDebugChecksEnabled() != 0)
if (doDebugCheck && (JitConfig.JitSlowDebugChecksEnabled() != 0))
{
// Make sure that the predecessor lists are accurate
fgDebugCheckBBlist();
Expand Down Expand Up @@ -4555,6 +4557,23 @@ void Compiler::fgMoveBackwardJumpsToSuccessors()
}
#endif // DEBUG

// Compact blocks before trying to move any jumps.
// This can unlock more opportunities for fallthrough behavior.
//
for (BasicBlock* block = fgFirstBB; block != fgFirstFuncletBB;)
{
if (fgCanCompactBlocks(block, block->Next()))
{
// We haven't fixed EH information yet, so don't do any correctness checks here
//
fgCompactBlocks(block, block->Next() DEBUGARG(/* doDebugCheck */ false));
}
else
{
block = block->Next();
}
}

EnsureBasicBlockEpoch();
BlockSet visitedBlocks(BlockSetOps::MakeEmpty(this));
BlockSetOps::AddElemD(this, visitedBlocks, fgFirstBB->bbNum);
Expand Down Expand Up @@ -4747,8 +4766,6 @@ void Compiler::fgDoReversePostOrderLayout()
}
}

fgMoveBackwardJumpsToSuccessors</* hasEH */ true>();

// Fix up call-finally pairs
//
for (int i = 0; i < callFinallyPairs.Height(); i++)
Expand All @@ -4758,6 +4775,8 @@ void Compiler::fgDoReversePostOrderLayout()
fgInsertBBafter(pair.callFinally, pair.callFinallyRet);
}

fgMoveBackwardJumpsToSuccessors</* hasEH */ true>();

// The RPO won't change the entry blocks of any EH regions, but reordering can change the last block in a region
// (for example, by pushing throw blocks unreachable via normal flow to the end of the region).
// First, determine the new EH region ends.
Expand Down

0 comments on commit 2762be8

Please sign in to comment.