diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8504476d2cdf6..c16c3547791fa 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -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 @@ -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; diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index a333cad69aca7..43465c31f48ae 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -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. @@ -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, @@ -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); } @@ -982,45 +1020,73 @@ 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. @@ -1028,13 +1094,13 @@ bool Compiler::optJumpThread(BasicBlock* const block, BasicBlock* const domBlock 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; @@ -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? // @@ -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); } }