Skip to content

Commit

Permalink
JIT: refactor jump threading a bit (#76108)
Browse files Browse the repository at this point in the history 
In anticipation that phi disambiguation will end up reusing the core
part of this transformation.
  • Loading branch information
@AndyAyersMS
AndyAyersMS committed Sep 25, 2022
1 parent 2600909 commit a4e3a42
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 77 deletions.
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
View file
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 @@ -6968,6 +6969,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
View file
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

0 comments on commit a4e3a42

Please sign in to comment.