Skip to content

JIT: refactor jump threading a bit #76108

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.

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

Merged
merged 1 commit into from
Sep 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class SpanningTreeVisitor; // defined in fgprofile.cpp
class CSE_DataFlow; // defined in OptCSE.cpp
class OptBoolsDsc; // defined in optimizer.cpp
struct RelopImplicationInfo; // defined in redundantbranchopts.cpp
struct JumpThreadInfo; // defined in redundantbranchopts.cpp
#ifdef DEBUG
struct IndentStack;
#endif
Expand Down Expand Up @@ -6970,6 +6971,7 @@ class Compiler
bool optRedundantRelop(BasicBlock* const block);
bool optRedundantBranch(BasicBlock* const block);
bool optJumpThread(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop);
bool optJumpThreadCore(JumpThreadInfo& jti);
bool optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock);
BitVecTraits* optReachableBitVecTraits;
BitVec optReachableBitVec;
Expand Down
220 changes: 143 additions & 77 deletions src/coreclr/jit/redundantbranchopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,52 @@ bool Compiler::optRedundantBranch(BasicBlock* const block)
return true;
}

//------------------------------------------------------------------------
// JumpThreadInfo
//
// Describes the relationship between a block-ending predicate value and the
// block's predecessors.
//
struct JumpThreadInfo
{
JumpThreadInfo(Compiler* comp, BasicBlock* block)
: m_block(block)
, m_trueTarget(block->bbJumpDest)
, m_falseTarget(block->bbNext)
, m_fallThroughPred(nullptr)
, m_truePreds(BlockSetOps::MakeEmpty(comp))
, m_ambiguousPreds(BlockSetOps::MakeEmpty(comp))
, m_numPreds(0)
, m_numAmbiguousPreds(0)
, m_numTruePreds(0)
, m_numFalsePreds(0)
{
}

// Block we're trying to optimize
BasicBlock* const m_block;
// Block successor if predicate is true
BasicBlock* const m_trueTarget;
// Block successor if predicate is false
BasicBlock* const m_falseTarget;
// Unique pred that falls through to block, if any
BasicBlock* m_fallThroughPred = nullptr;
// Pred blocks for which the predicate will be true
BlockSet m_truePreds;
// Pred blocks that can't be threaded or for which the predicate
// value can't be determined
BlockSet m_ambiguousPreds;
// Total number of predecessors
int m_numPreds;
// Number of predecessors that can't be threaded or for which the predicate
// value can't be determined
int m_numAmbiguousPreds;
// Number of predecessors for which predicate is true
int m_numTruePreds;
// Number of predecessors for which predicate is false
int m_numFalsePreds;
};

//------------------------------------------------------------------------
// optJumpThread: try and bypass the current block by rerouting
// flow from predecessors directly to successors.
Expand Down Expand Up @@ -900,40 +946,32 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
// latter should prove useful in subsequent work, where we aim to enable jump
// threading in cases where block has side effects.
//
int numPreds = 0;
int numAmbiguousPreds = 0;
int numTruePreds = 0;
int numFalsePreds = 0;
BasicBlock* fallThroughPred = nullptr;
BasicBlock* const trueSuccessor = domIsSameRelop ? domBlock->bbJumpDest : domBlock->bbNext;
BasicBlock* const falseSuccessor = domIsSameRelop ? domBlock->bbNext : domBlock->bbJumpDest;
BasicBlock* const trueTarget = block->bbJumpDest;
BasicBlock* const falseTarget = block->bbNext;
BlockSet truePreds = BlockSetOps::MakeEmpty(this);
BlockSet ambiguousPreds = BlockSetOps::MakeEmpty(this);
BasicBlock* const domTrueSuccessor = domIsSameRelop ? domBlock->bbJumpDest : domBlock->bbNext;
BasicBlock* const domFalseSuccessor = domIsSameRelop ? domBlock->bbNext : domBlock->bbJumpDest;
JumpThreadInfo jti(this, block);

for (BasicBlock* const predBlock : block->PredBlocks())
{
numPreds++;
jti.m_numPreds++;

// Treat switch preds as ambiguous for now.
//
if (predBlock->bbJumpKind == BBJ_SWITCH)
{
JITDUMP(FMT_BB " is a switch pred\n", predBlock->bbNum);
BlockSetOps::AddElemD(this, ambiguousPreds, predBlock->bbNum);
numAmbiguousPreds++;
BlockSetOps::AddElemD(this, jti.m_ambiguousPreds, predBlock->bbNum);
jti.m_numAmbiguousPreds++;
continue;
}

const bool isTruePred =
((predBlock == domBlock) && (trueSuccessor == block)) || optReachable(trueSuccessor, predBlock, domBlock);
const bool isFalsePred =
((predBlock == domBlock) && (falseSuccessor == block)) || optReachable(falseSuccessor, predBlock, domBlock);
const bool isTruePred = ((predBlock == domBlock) && (domTrueSuccessor == block)) ||
optReachable(domTrueSuccessor, predBlock, domBlock);
const bool isFalsePred = ((predBlock == domBlock) && (domFalseSuccessor == block)) ||
optReachable(domFalseSuccessor, predBlock, domBlock);

if (isTruePred == isFalsePred)
{
// Either both reach, or neither reaches.
// Either both dom successors reach, or neither reaches.
//
// We should rarely see (false,false) given that optReachable is returning
// up to date results, but as we optimize we create unreachable blocks,
Expand All @@ -942,38 +980,38 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
// lead to more complications, and it isn't that common. So we tolerate it.
//
JITDUMP(FMT_BB " is an ambiguous pred\n", predBlock->bbNum);
BlockSetOps::AddElemD(this, ambiguousPreds, predBlock->bbNum);
numAmbiguousPreds++;
BlockSetOps::AddElemD(this, jti.m_ambiguousPreds, predBlock->bbNum);
jti.m_numAmbiguousPreds++;
continue;
}

if (isTruePred)
{
if (!BasicBlock::sameEHRegion(predBlock, trueTarget))
if (!BasicBlock::sameEHRegion(predBlock, jti.m_trueTarget))
{
JITDUMP(FMT_BB " is an eh constrained pred\n", predBlock->bbNum);
numAmbiguousPreds++;
BlockSetOps::AddElemD(this, ambiguousPreds, predBlock->bbNum);
jti.m_numAmbiguousPreds++;
BlockSetOps::AddElemD(this, jti.m_ambiguousPreds, predBlock->bbNum);
continue;
}

numTruePreds++;
BlockSetOps::AddElemD(this, truePreds, predBlock->bbNum);
jti.m_numTruePreds++;
BlockSetOps::AddElemD(this, jti.m_truePreds, predBlock->bbNum);
JITDUMP(FMT_BB " is a true pred\n", predBlock->bbNum);
}
else
{
assert(isFalsePred);

if (!BasicBlock::sameEHRegion(predBlock, falseTarget))
if (!BasicBlock::sameEHRegion(predBlock, jti.m_falseTarget))
{
JITDUMP(FMT_BB " is an eh constrained pred\n", predBlock->bbNum);
BlockSetOps::AddElemD(this, ambiguousPreds, predBlock->bbNum);
numAmbiguousPreds++;
BlockSetOps::AddElemD(this, jti.m_ambiguousPreds, predBlock->bbNum);
jti.m_numAmbiguousPreds++;
continue;
}

numFalsePreds++;
jti.m_numFalsePreds++;
JITDUMP(FMT_BB " is a false pred\n", predBlock->bbNum);
}

Expand All @@ -982,59 +1020,87 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
if (predBlock->bbNext == block)
{
JITDUMP(FMT_BB " is the fall-through pred\n", predBlock->bbNum);
assert(fallThroughPred == nullptr);
fallThroughPred = predBlock;
assert(jti.m_fallThroughPred == nullptr);
jti.m_fallThroughPred = predBlock;
}
}

// Do the optimization.
//
return optJumpThreadCore(jti);
}

//------------------------------------------------------------------------
// optJumpThreadCore: restructure block flow based on jump thread information
//
// Arguments:
// jit - information on how to jump thread this block
//
// Returns:
// True if the branch was optimized.
//
bool Compiler::optJumpThreadCore(JumpThreadInfo& jti)
{
// All preds should have been classified.
//
assert(numPreds == numTruePreds + numFalsePreds + numAmbiguousPreds);
assert(jti.m_numPreds == jti.m_numTruePreds + jti.m_numFalsePreds + jti.m_numAmbiguousPreds);

if ((numTruePreds == 0) && (numFalsePreds == 0))
// There should be at least one pred that can bypass block.
//
if ((jti.m_numTruePreds == 0) && (jti.m_numFalsePreds == 0))
{
// This is possible, but should be rare.
//
JITDUMP(FMT_BB " only has ambiguous preds, not optimizing\n", block->bbNum);
JITDUMP(FMT_BB " only has ambiguous preds, not jump threading\n", jti.m_block->bbNum);
return false;
}

if ((numAmbiguousPreds > 0) && (fallThroughPred != nullptr))
if ((jti.m_numAmbiguousPreds > 0) && (jti.m_fallThroughPred != nullptr))
{
// Treat the fall through pred as an ambiguous pred.
JITDUMP(FMT_BB " has both ambiguous preds and a fall through pred\n", block->bbNum);
JITDUMP("Treating fall through pred " FMT_BB " as an ambiguous pred\n", fallThroughPred->bbNum);
JITDUMP(FMT_BB " has both ambiguous preds and a fall through pred\n", jti.m_block->bbNum);
JITDUMP("Treating fall through pred " FMT_BB " as an ambiguous pred\n", jti.m_fallThroughPred->bbNum);

if (BlockSetOps::IsMember(this, truePreds, fallThroughPred->bbNum))
if (BlockSetOps::IsMember(this, jti.m_truePreds, jti.m_fallThroughPred->bbNum))
{
BlockSetOps::RemoveElemD(this, truePreds, fallThroughPred->bbNum);
assert(numTruePreds > 0);
numTruePreds--;
BlockSetOps::RemoveElemD(this, jti.m_truePreds, jti.m_fallThroughPred->bbNum);
assert(jti.m_numTruePreds > 0);
jti.m_numTruePreds--;
}
else
{
assert(numFalsePreds > 0);
numFalsePreds--;
assert(jti.m_numFalsePreds > 0);
jti.m_numFalsePreds--;
}

assert(!(BlockSetOps::IsMember(this, ambiguousPreds, fallThroughPred->bbNum)));
BlockSetOps::AddElemD(this, ambiguousPreds, fallThroughPred->bbNum);
numAmbiguousPreds++;
fallThroughPred = nullptr;
assert(!(BlockSetOps::IsMember(this, jti.m_ambiguousPreds, jti.m_fallThroughPred->bbNum)));
BlockSetOps::AddElemD(this, jti.m_ambiguousPreds, jti.m_fallThroughPred->bbNum);
jti.m_numAmbiguousPreds++;
jti.m_fallThroughPred = nullptr;
}

// There still should be at least one pred that can bypass block.
//
if ((jti.m_numTruePreds == 0) && (jti.m_numFalsePreds == 0))
{
// This is possible, but also should be rare.
//
JITDUMP(FMT_BB " now only has ambiguous preds, not jump threading\n", jti.m_block->bbNum);
return false;
}

// Determine if either set of preds will route via block.
//
bool truePredsWillReuseBlock = false;
bool falsePredsWillReuseBlock = false;

if (fallThroughPred != nullptr)
if (jti.m_fallThroughPred != nullptr)
{
assert(numAmbiguousPreds == 0);
truePredsWillReuseBlock = BlockSetOps::IsMember(this, truePreds, fallThroughPred->bbNum);
assert(jti.m_numAmbiguousPreds == 0);
truePredsWillReuseBlock = BlockSetOps::IsMember(this, jti.m_truePreds, jti.m_fallThroughPred->bbNum);
falsePredsWillReuseBlock = !truePredsWillReuseBlock;
}
else if (numAmbiguousPreds == 0)
else if (jti.m_numAmbiguousPreds == 0)
{
truePredsWillReuseBlock = true;
falsePredsWillReuseBlock = !truePredsWillReuseBlock;
Expand All @@ -1050,35 +1116,36 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
//
if (truePredsWillReuseBlock)
{
Statement* lastStmt = block->lastStmt();
fgRemoveStmt(block, lastStmt);
JITDUMP(" repurposing " FMT_BB " to always jump to " FMT_BB "\n", block->bbNum, trueTarget->bbNum);
fgRemoveRefPred(block->bbNext, block);
block->bbJumpKind = BBJ_ALWAYS;
Statement* const lastStmt = jti.m_block->lastStmt();
fgRemoveStmt(jti.m_block, lastStmt);
JITDUMP(" repurposing " FMT_BB " to always jump to " FMT_BB "\n", jti.m_block->bbNum, jti.m_trueTarget->bbNum);
fgRemoveRefPred(jti.m_falseTarget, jti.m_block);
jti.m_block->bbJumpKind = BBJ_ALWAYS;
}
else if (falsePredsWillReuseBlock)
{
Statement* lastStmt = block->lastStmt();
fgRemoveStmt(block, lastStmt);
JITDUMP(" repurposing " FMT_BB " to always fall through to " FMT_BB "\n", block->bbNum, falseTarget->bbNum);
fgRemoveRefPred(block->bbJumpDest, block);
block->bbJumpKind = BBJ_NONE;
Statement* const lastStmt = jti.m_block->lastStmt();
fgRemoveStmt(jti.m_block, lastStmt);
JITDUMP(" repurposing " FMT_BB " to always fall through to " FMT_BB "\n", jti.m_block->bbNum,
jti.m_falseTarget->bbNum);
fgRemoveRefPred(jti.m_trueTarget, jti.m_block);
jti.m_block->bbJumpKind = BBJ_NONE;
}

// Now reroute the flow from the predecessors.
// If this pred is in the set that will reuse block, do nothing.
// Else revise pred to branch directly to the appropriate successor of block.
//
for (BasicBlock* const predBlock : block->PredBlocks())
for (BasicBlock* const predBlock : jti.m_block->PredBlocks())
{
// If this was an ambiguous pred, skip.
//
if (BlockSetOps::IsMember(this, ambiguousPreds, predBlock->bbNum))
if (BlockSetOps::IsMember(this, jti.m_ambiguousPreds, predBlock->bbNum))
{
continue;
}

const bool isTruePred = BlockSetOps::IsMember(this, truePreds, predBlock->bbNum);
const bool isTruePred = BlockSetOps::IsMember(this, jti.m_truePreds, predBlock->bbNum);

// Do we need to alter flow from this pred?
//
Expand All @@ -1087,36 +1154,35 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock
// No, we can leave as is.
//
JITDUMP("%s pred " FMT_BB " will continue to target " FMT_BB "\n", isTruePred ? "true" : "false",
predBlock->bbNum, block->bbNum);
predBlock->bbNum, jti.m_block->bbNum);
continue;
}

// Yes, we need to jump to the appropriate successor.
// Note we should not be altering flow for the fall-through pred.
//
assert(predBlock != fallThroughPred);
assert(predBlock->bbNext != block);
assert(predBlock != jti.m_fallThroughPred);
assert(predBlock->bbNext != jti.m_block);

if (isTruePred)
{
assert(!optReachable(falseSuccessor, predBlock, domBlock));
JITDUMP("Jump flow from pred " FMT_BB " -> " FMT_BB
" implies predicate true; we can safely redirect flow to be " FMT_BB " -> " FMT_BB "\n",
predBlock->bbNum, block->bbNum, predBlock->bbNum, trueTarget->bbNum);
predBlock->bbNum, jti.m_block->bbNum, predBlock->bbNum, jti.m_trueTarget->bbNum);

fgRemoveRefPred(block, predBlock);
fgReplaceJumpTarget(predBlock, trueTarget, block);
fgAddRefPred(trueTarget, predBlock);
fgRemoveRefPred(jti.m_block, predBlock);
fgReplaceJumpTarget(predBlock, jti.m_trueTarget, jti.m_block);
fgAddRefPred(jti.m_trueTarget, predBlock);
}
else
{
JITDUMP("Jump flow from pred " FMT_BB " -> " FMT_BB
" implies predicate false; we can safely redirect flow to be " FMT_BB " -> " FMT_BB "\n",
predBlock->bbNum, block->bbNum, predBlock->bbNum, falseTarget->bbNum);
predBlock->bbNum, jti.m_block->bbNum, predBlock->bbNum, jti.m_falseTarget->bbNum);

fgRemoveRefPred(block, predBlock);
fgReplaceJumpTarget(predBlock, falseTarget, block);
fgAddRefPred(falseTarget, predBlock);
fgRemoveRefPred(jti.m_block, predBlock);
fgReplaceJumpTarget(predBlock, jti.m_falseTarget, jti.m_block);
fgAddRefPred(jti.m_falseTarget, predBlock);
}
}

Expand Down