diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index cb56cc780209a8..3845678c08a542 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7659,14 +7659,26 @@ class Compiler // Redundant branch opts // - PhaseStatus optRedundantBranches(); - bool optRedundantRelop(BasicBlock* const block); - bool optRedundantBranch(BasicBlock* const block); - bool optJumpThreadDom(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop); - bool optJumpThreadPhi(BasicBlock* const block, GenTree* tree, ValueNum treeNormVN); - bool optJumpThreadCheck(BasicBlock* const block, BasicBlock* const domBlock); - bool optJumpThreadCore(JumpThreadInfo& jti); - bool optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock); + PhaseStatus optRedundantBranches(); + bool optRedundantRelop(BasicBlock* const block); + bool optRedundantBranch(BasicBlock* const block); + bool optJumpThreadDom(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop); + bool optJumpThreadPhi(BasicBlock* const block, GenTree* tree, ValueNum treeNormVN); + bool optJumpThreadCheck(BasicBlock* const block, BasicBlock* const domBlock); + bool optJumpThreadCore(JumpThreadInfo& jti); + + enum class ReachabilityResult + { + BudgetExceeded, + Unreachable, + Reachable + }; + ReachabilityResult optReachableWithBudget(BasicBlock* const fromBlock, + BasicBlock* const toBlock, + BasicBlock* const excludedBlock, + int* pBudget); + bool optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock); + BitVecTraits* optReachableBitVecTraits; BitVec optReachableBitVec; void optRelopImpliesRelop(RelopImplicationInfo* rii); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index ca6a2a7eb223cb..602612767a2567 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -2916,15 +2916,15 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef return; } - fgDebugCheckBlockLinks(); - - if (fgBBcount > 10000 && expensiveDebugCheckLevel < 1) + if ((fgBBcount > 10000) && (expensiveDebugCheckLevel < 1)) { // The basic block checks are too expensive if there are too many blocks, // so give up unless we've been told to try hard. return; } + fgDebugCheckBlockLinks(); + bool reachedFirstFunclet = false; if (fgFuncletsCreated) { @@ -3796,13 +3796,10 @@ void Compiler::fgDebugCheckLinkedLocals() void Compiler::fgDebugCheckLinks(bool morphTrees) { - // This used to be only on for stress, and there was a comment stating that - // it was "quite an expensive operation" but I did not find that to be true. - // Set DO_SANITY_DEBUG_CHECKS to false to revert to that behavior. - const bool DO_SANITY_DEBUG_CHECKS = true; - - if (!DO_SANITY_DEBUG_CHECKS && !compStressCompile(STRESS_CHK_FLOW_UPDATE, 30)) + if ((fgBBcount > 10000) && (expensiveDebugCheckLevel < 1)) { + // The basic block checks are too expensive if there are too many blocks, + // so give up unless we've been told to try hard. return; } diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp index abf7f53a6b0250..361cae9caa4990 100644 --- a/src/coreclr/jit/ifconversion.cpp +++ b/src/coreclr/jit/ifconversion.cpp @@ -65,7 +65,7 @@ class OptIfConversionDsc #endif public: - bool optIfConvert(); + bool optIfConvert(int* pReachabilityBudget); }; //----------------------------------------------------------------------------- @@ -408,6 +408,9 @@ void OptIfConversionDsc::IfConvertDump() // Find blocks representing simple if statements represented by conditional jumps // over another block. Try to replace the jumps by use of SELECT nodes. // +// Arguments: +// pReachabilityBudget -- budget for optReachability +// // Returns: // true if any IR changes possibly made. // @@ -551,8 +554,13 @@ void OptIfConversionDsc::IfConvertDump() // ------------ BB04 [00D..010), preds={} succs={BB06} // ------------ BB05 [00D..010), preds={} succs={BB06} // -bool OptIfConversionDsc::optIfConvert() +bool OptIfConversionDsc::optIfConvert(int* pReachabilityBudget) { + if ((*pReachabilityBudget) <= 0) + { + return false; + } + // Does the block end by branching via a JTRUE after a compare? if (!m_startBlock->KindIs(BBJ_COND) || (m_startBlock->NumSucc() != 2)) { @@ -668,9 +676,16 @@ bool OptIfConversionDsc::optIfConvert() } // We may be inside an unnatural loop, so do the expensive check. - if (m_comp->optReachable(m_finalBlock, m_startBlock, nullptr)) + Compiler::ReachabilityResult reachability = + m_comp->optReachableWithBudget(m_finalBlock, m_startBlock, nullptr, pReachabilityBudget); + if (reachability == Compiler::ReachabilityResult::Reachable) { - JITDUMP("Skipping if-conversion inside loop (via FG walk)\n"); + JITDUMP("Skipping if-conversion inside loop (via reachability)\n"); + return false; + } + else if (reachability == Compiler::ReachabilityResult::BudgetExceeded) + { + JITDUMP("Skipping if-conversion since we ran out of reachability budget\n"); return false; } } @@ -1023,10 +1038,13 @@ PhaseStatus Compiler::optIfConversion() #if defined(TARGET_ARM64) || defined(TARGET_XARCH) || defined(TARGET_RISCV64) // Reverse iterate through the blocks. BasicBlock* block = fgLastBB; + + // Budget for optReachability - to avoid spending too much time detecting loops in large methods. + int reachabilityBudget = 20000; while (block != nullptr) { OptIfConversionDsc optIfConversionDsc(this, block); - madeChanges |= optIfConversionDsc.optIfConvert(); + madeChanges |= optIfConversionDsc.optIfConvert(&reachabilityBudget); block = block->Prev(); } #endif diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 9528ac3065f798..e9ff9b1fdec0f2 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -2243,8 +2243,8 @@ bool Compiler::optRedundantRelop(BasicBlock* const block) // including paths involving EH flow. // // Arguments: -// fromBlock - staring block -// toBlock - ending block +// fromBlock - staring block +// toBlock - ending block // excludedBlock - ignore paths that flow through this block // // Returns: @@ -2258,10 +2258,35 @@ bool Compiler::optRedundantRelop(BasicBlock* const block) // finallies with multiple continuations. // bool Compiler::optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock) +{ + ReachabilityResult result = optReachableWithBudget(fromBlock, toBlock, excludedBlock, nullptr); + assert(result != ReachabilityResult::BudgetExceeded); + return result == ReachabilityResult::Reachable; +} + +//------------------------------------------------------------------------ +// optReachableWithBudget: see if there's a path from one block to another, +// including paths involving EH flow. Same as optReachable, but with a budget check. +// +// Arguments: +// fromBlock - staring block +// toBlock - ending block +// excludedBlock - ignore paths that flow through this block +// pBudget - number of blocks to examine before returning BudgetExceeded +// +// Returns: +// ReachabilityResult::Reachable if there is a path from fromBlock to toBlock, +// ReachabilityResult::Unreachable if there is no such path, +// ReachabilityResult::BudgetExceeded if we ran out of budget before finding either. +// +Compiler::ReachabilityResult Compiler::optReachableWithBudget(BasicBlock* const fromBlock, + BasicBlock* const toBlock, + BasicBlock* const excludedBlock, + int* pBudget) { if (fromBlock == toBlock) { - return true; + return ReachabilityResult::Reachable; } if (optReachableBitVecTraits == nullptr) @@ -2287,27 +2312,52 @@ bool Compiler::optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlo { continue; } + BasicBlockVisit result; + bool budgetExceeded = false; + if (pBudget == nullptr) + { + result = nextBlock->VisitAllSuccs(this, [this, toBlock, &stack](BasicBlock* succ) { + if (succ == toBlock) + { + return BasicBlockVisit::Abort; + } - BasicBlockVisit result = nextBlock->VisitAllSuccs(this, [this, toBlock, &stack](BasicBlock* succ) { - if (succ == toBlock) - { - return BasicBlockVisit::Abort; - } - - if (!BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum)) - { + if (BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum)) + { + stack.Push(succ); + } return BasicBlockVisit::Continue; - } + }); + } + else + { + result = + nextBlock->VisitAllSuccs(this, [this, toBlock, &stack, &budgetExceeded, pBudget](BasicBlock* succ) { + if (succ == toBlock) + { + return BasicBlockVisit::Abort; + } - stack.Push(succ); - return BasicBlockVisit::Continue; - }); + if (--(*pBudget) <= 0) + { + budgetExceeded = true; + return BasicBlockVisit::Abort; + } + + if (BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum)) + { + stack.Push(succ); + } + + return BasicBlockVisit::Continue; + }); + } if (result == BasicBlockVisit::Abort) { - return true; + return budgetExceeded ? ReachabilityResult::BudgetExceeded : ReachabilityResult::Reachable; } } - return false; + return ReachabilityResult::Unreachable; }