Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: generalize the branch around empty flow optimization #51409

Merged
merged 2 commits into from
Apr 17, 2021
Merged

JIT: generalize the branch around empty flow optimization #51409

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9422a10
JIT: generalize the branch around empty flow optimization
AndyAyersMS Apr 16, 2021
7c45f54
Fix issue with callfinallies on x86.
AndyAyersMS Apr 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 87 additions & 12 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5314,18 +5314,46 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication)
}
}

// Check for a conditional branch that just skips over an empty BBJ_ALWAYS block

// Check for cases where reversing the branch condition may enable
// other flow opts.
//
// Current block falls through to an empty bNext BBJ_ALWAYS, and
// (a) block jump target is bNext's bbNext.
// (b) block jump target is elsewhere but join free, and
// bNext's jump target has a join.
//
if ((block->bbJumpKind == BBJ_COND) && // block is a BBJ_COND block
(bNext != nullptr) && // block is not the last block
(bNext->bbRefs == 1) && // No other block jumps to bNext
(bNext->bbNext == bDest) && // The block after bNext is the BBJ_COND jump dest
(bNext->bbJumpKind == BBJ_ALWAYS) && // The next block is a BBJ_ALWAYS block
bNext->isEmpty() && // and it is an an empty block
(bNext != bNext->bbJumpDest) && // special case for self jumps
(bDest != fgFirstColdBlock))
{
bool optimizeJump = true;
// case (a)
//
const bool isJumpAroundEmpty = (bNext->bbNext == bDest);

// case (b)
//
// Note the asymetric checks for refs == 1 and refs > 1 ensures that we
// differentiate the roles played by bDest and bNextJumpDest. We need some
// sense of which arrangement is preferable to avoid getting stuck in a loop
// reversing and re-reversing.
//
// Other tiebreaking criteria could be considered.
//
// Pragmatic constraints:
//
// * don't consider lexical predecessors, or we may confuse loop recognition
// * don't consider blocks of different rarities
//
BasicBlock* const bNextJumpDest = bNext->bbJumpDest;
const bool isJumpToJoinFree = !isJumpAroundEmpty && (bDest->bbRefs == 1) &&
(bNextJumpDest->bbRefs > 1) && (bDest->bbNum > block->bbNum) &&
(block->isRunRarely() == bDest->isRunRarely());

bool optimizeJump = isJumpAroundEmpty || isJumpToJoinFree;

// We do not optimize jumps between two different try regions.
// However jumping to a block that is not in any try region is OK
Expand Down Expand Up @@ -5357,18 +5385,65 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication)
}
}

if (optimizeJump)
if (optimizeJump && isJumpToJoinFree)
{
#ifdef DEBUG
if (verbose)
// In the join free case, we also need to move bDest right after bNext
// to create same flow as in the isJumpAroundEmpty case.
//
if (!fgEhAllowsMoveBlock(bNext, bDest))
{
printf("\nReversing a conditional jump around an unconditional jump (" FMT_BB " -> " FMT_BB
" -> " FMT_BB ")\n",
block->bbNum, bDest->bbNum, bNext->bbJumpDest->bbNum);
optimizeJump = false;
}
#endif // DEBUG
/* Reverse the jump condition */
else
{
// We don't expect bDest to already be right after bNext.
//
assert(bDest != bNext->bbNext);

JITDUMP("\nMoving " FMT_BB " after " FMT_BB " to enable reversal\n", bDest->bbNum,
bNext->bbNum);

// If bDest can fall through we'll need to create a jump
// block after it too. Remember where to jump to.
//
BasicBlock* const bDestNext = bDest->bbNext;

// Move bDest
//
if (ehIsBlockEHLast(bDest))
{
ehUpdateLastBlocks(bDest, bDest->bbPrev);
}

fgUnlinkBlock(bDest);
fgInsertBBafter(bNext, bDest);

if (ehIsBlockEHLast(bNext))
{
ehUpdateLastBlocks(bNext, bDest);
}

// Add fall through fixup block, if needed.
//
if (bDest->bbFallsThrough())
{
BasicBlock* const bFixup = fgNewBBafter(BBJ_ALWAYS, bDest, true);
bFixup->inheritWeight(bDestNext);
bFixup->bbJumpDest = bDestNext;
fgReplacePred(bDestNext, bDest, bFixup);
fgAddRefPred(bFixup, bDest);
}
}
}

if (optimizeJump)
{
JITDUMP("\nReversing a conditional jump around an unconditional jump (" FMT_BB " -> " FMT_BB
" -> " FMT_BB ")\n",
block->bbNum, bDest->bbNum, bNextJumpDest->bbNum);

// Reverse the jump condition
//
GenTree* test = block->lastNode();
noway_assert(test->OperIsConditionalJump());

Expand Down