Skip to content

Commit d2a3735

Browse files
EgorBoEgorBo
authored
Add budget check to optReachable (#118787)
1 parent 308613e commit d2a3735

File tree

4 files changed

+116
-39
lines changed

4 files changed

+116
-39
lines changed

‎src/coreclr/jit/compiler.h‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7659,14 +7659,26 @@ class Compiler
76597659

76607660
// Redundant branch opts
76617661
//
7662-
PhaseStatus optRedundantBranches();
7663-
bool optRedundantRelop(BasicBlock* const block);
7664-
bool optRedundantBranch(BasicBlock* const block);
7665-
bool optJumpThreadDom(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop);
7666-
bool optJumpThreadPhi(BasicBlock* const block, GenTree* tree, ValueNum treeNormVN);
7667-
bool optJumpThreadCheck(BasicBlock* const block, BasicBlock* const domBlock);
7668-
bool optJumpThreadCore(JumpThreadInfo& jti);
7669-
bool optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock);
7662+
PhaseStatus optRedundantBranches();
7663+
bool optRedundantRelop(BasicBlock* const block);
7664+
bool optRedundantBranch(BasicBlock* const block);
7665+
bool optJumpThreadDom(BasicBlock* const block, BasicBlock* const domBlock, bool domIsSameRelop);
7666+
bool optJumpThreadPhi(BasicBlock* const block, GenTree* tree, ValueNum treeNormVN);
7667+
bool optJumpThreadCheck(BasicBlock* const block, BasicBlock* const domBlock);
7668+
bool optJumpThreadCore(JumpThreadInfo& jti);
7669+
7670+
enum class ReachabilityResult
7671+
{
7672+
BudgetExceeded,
7673+
Unreachable,
7674+
Reachable
7675+
};
7676+
ReachabilityResult optReachableWithBudget(BasicBlock* const fromBlock,
7677+
BasicBlock* const toBlock,
7678+
BasicBlock* const excludedBlock,
7679+
int* pBudget);
7680+
bool optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock);
7681+
76707682
BitVecTraits* optReachableBitVecTraits;
76717683
BitVec optReachableBitVec;
76727684
void optRelopImpliesRelop(RelopImplicationInfo* rii);

‎src/coreclr/jit/fgdiagnostic.cpp‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2916,15 +2916,15 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
29162916
return;
29172917
}
29182918

2919-
fgDebugCheckBlockLinks();
2920-
2921-
if (fgBBcount > 10000 && expensiveDebugCheckLevel < 1)
2919+
if ((fgBBcount > 10000) && (expensiveDebugCheckLevel < 1))
29222920
{
29232921
// The basic block checks are too expensive if there are too many blocks,
29242922
// so give up unless we've been told to try hard.
29252923
return;
29262924
}
29272925

2926+
fgDebugCheckBlockLinks();
2927+
29282928
bool reachedFirstFunclet = false;
29292929
if (fgFuncletsCreated)
29302930
{
@@ -3796,13 +3796,10 @@ void Compiler::fgDebugCheckLinkedLocals()
37963796

37973797
void Compiler::fgDebugCheckLinks(bool morphTrees)
37983798
{
3799-
// This used to be only on for stress, and there was a comment stating that
3800-
// it was "quite an expensive operation" but I did not find that to be true.
3801-
// Set DO_SANITY_DEBUG_CHECKS to false to revert to that behavior.
3802-
const bool DO_SANITY_DEBUG_CHECKS = true;
3803-
3804-
if (!DO_SANITY_DEBUG_CHECKS && !compStressCompile(STRESS_CHK_FLOW_UPDATE, 30))
3799+
if ((fgBBcount > 10000) && (expensiveDebugCheckLevel < 1))
38053800
{
3801+
// The basic block checks are too expensive if there are too many blocks,
3802+
// so give up unless we've been told to try hard.
38063803
return;
38073804
}
38083805

‎src/coreclr/jit/ifconversion.cpp‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class OptIfConversionDsc
6565
#endif
6666

6767
public:
68-
bool optIfConvert();
68+
bool optIfConvert(int* pReachabilityBudget);
6969
};
7070

7171
//-----------------------------------------------------------------------------
@@ -408,6 +408,9 @@ void OptIfConversionDsc::IfConvertDump()
408408
// Find blocks representing simple if statements represented by conditional jumps
409409
// over another block. Try to replace the jumps by use of SELECT nodes.
410410
//
411+
// Arguments:
412+
// pReachabilityBudget -- budget for optReachability
413+
//
411414
// Returns:
412415
// true if any IR changes possibly made.
413416
//
@@ -551,8 +554,13 @@ void OptIfConversionDsc::IfConvertDump()
551554
// ------------ BB04 [00D..010), preds={} succs={BB06}
552555
// ------------ BB05 [00D..010), preds={} succs={BB06}
553556
//
554-
bool OptIfConversionDsc::optIfConvert()
557+
bool OptIfConversionDsc::optIfConvert(int* pReachabilityBudget)
555558
{
559+
if ((*pReachabilityBudget) <= 0)
560+
{
561+
return false;
562+
}
563+
556564
// Does the block end by branching via a JTRUE after a compare?
557565
if (!m_startBlock->KindIs(BBJ_COND) || (m_startBlock->NumSucc() != 2))
558566
{
@@ -668,9 +676,16 @@ bool OptIfConversionDsc::optIfConvert()
668676
}
669677

670678
// We may be inside an unnatural loop, so do the expensive check.
671-
if (m_comp->optReachable(m_finalBlock, m_startBlock, nullptr))
679+
Compiler::ReachabilityResult reachability =
680+
m_comp->optReachableWithBudget(m_finalBlock, m_startBlock, nullptr, pReachabilityBudget);
681+
if (reachability == Compiler::ReachabilityResult::Reachable)
672682
{
673-
JITDUMP("Skipping if-conversion inside loop (via FG walk)\n");
683+
JITDUMP("Skipping if-conversion inside loop (via reachability)\n");
684+
return false;
685+
}
686+
else if (reachability == Compiler::ReachabilityResult::BudgetExceeded)
687+
{
688+
JITDUMP("Skipping if-conversion since we ran out of reachability budget\n");
674689
return false;
675690
}
676691
}
@@ -1023,10 +1038,13 @@ PhaseStatus Compiler::optIfConversion()
10231038
#if defined(TARGET_ARM64) || defined(TARGET_XARCH) || defined(TARGET_RISCV64)
10241039
// Reverse iterate through the blocks.
10251040
BasicBlock* block = fgLastBB;
1041+
1042+
// Budget for optReachability - to avoid spending too much time detecting loops in large methods.
1043+
int reachabilityBudget = 20000;
10261044
while (block != nullptr)
10271045
{
10281046
OptIfConversionDsc optIfConversionDsc(this, block);
1029-
madeChanges |= optIfConversionDsc.optIfConvert();
1047+
madeChanges |= optIfConversionDsc.optIfConvert(&reachabilityBudget);
10301048
block = block->Prev();
10311049
}
10321050
#endif

‎src/coreclr/jit/redundantbranchopts.cpp‎

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2243,8 +2243,8 @@ bool Compiler::optRedundantRelop(BasicBlock* const block)
22432243
// including paths involving EH flow.
22442244
//
22452245
// Arguments:
2246-
// fromBlock - staring block
2247-
// toBlock - ending block
2246+
// fromBlock - staring block
2247+
// toBlock - ending block
22482248
// excludedBlock - ignore paths that flow through this block
22492249
//
22502250
// Returns:
@@ -2258,10 +2258,35 @@ bool Compiler::optRedundantRelop(BasicBlock* const block)
22582258
// finallies with multiple continuations.
22592259
//
22602260
bool Compiler::optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlock, BasicBlock* const excludedBlock)
2261+
{
2262+
ReachabilityResult result = optReachableWithBudget(fromBlock, toBlock, excludedBlock, nullptr);
2263+
assert(result != ReachabilityResult::BudgetExceeded);
2264+
return result == ReachabilityResult::Reachable;
2265+
}
2266+
2267+
//------------------------------------------------------------------------
2268+
// optReachableWithBudget: see if there's a path from one block to another,
2269+
// including paths involving EH flow. Same as optReachable, but with a budget check.
2270+
//
2271+
// Arguments:
2272+
// fromBlock - staring block
2273+
// toBlock - ending block
2274+
// excludedBlock - ignore paths that flow through this block
2275+
// pBudget - number of blocks to examine before returning BudgetExceeded
2276+
//
2277+
// Returns:
2278+
// ReachabilityResult::Reachable if there is a path from fromBlock to toBlock,
2279+
// ReachabilityResult::Unreachable if there is no such path,
2280+
// ReachabilityResult::BudgetExceeded if we ran out of budget before finding either.
2281+
//
2282+
Compiler::ReachabilityResult Compiler::optReachableWithBudget(BasicBlock* const fromBlock,
2283+
BasicBlock* const toBlock,
2284+
BasicBlock* const excludedBlock,
2285+
int* pBudget)
22612286
{
22622287
if (fromBlock == toBlock)
22632288
{
2264-
return true;
2289+
return ReachabilityResult::Reachable;
22652290
}
22662291

22672292
if (optReachableBitVecTraits == nullptr)
@@ -2287,27 +2312,52 @@ bool Compiler::optReachable(BasicBlock* const fromBlock, BasicBlock* const toBlo
22872312
{
22882313
continue;
22892314
}
2315+
BasicBlockVisit result;
2316+
bool budgetExceeded = false;
2317+
if (pBudget == nullptr)
2318+
{
2319+
result = nextBlock->VisitAllSuccs(this, [this, toBlock, &stack](BasicBlock* succ) {
2320+
if (succ == toBlock)
2321+
{
2322+
return BasicBlockVisit::Abort;
2323+
}
22902324

2291-
BasicBlockVisit result = nextBlock->VisitAllSuccs(this, [this, toBlock, &stack](BasicBlock* succ) {
2292-
if (succ == toBlock)
2293-
{
2294-
return BasicBlockVisit::Abort;
2295-
}
2296-
2297-
if (!BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum))
2298-
{
2325+
if (BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum))
2326+
{
2327+
stack.Push(succ);
2328+
}
22992329
return BasicBlockVisit::Continue;
2300-
}
2330+
});
2331+
}
2332+
else
2333+
{
2334+
result =
2335+
nextBlock->VisitAllSuccs(this, [this, toBlock, &stack, &budgetExceeded, pBudget](BasicBlock* succ) {
2336+
if (succ == toBlock)
2337+
{
2338+
return BasicBlockVisit::Abort;
2339+
}
23012340

2302-
stack.Push(succ);
2303-
return BasicBlockVisit::Continue;
2304-
});
2341+
if (--(*pBudget) <= 0)
2342+
{
2343+
budgetExceeded = true;
2344+
return BasicBlockVisit::Abort;
2345+
}
2346+
2347+
if (BitVecOps::TryAddElemD(optReachableBitVecTraits, optReachableBitVec, succ->bbNum))
2348+
{
2349+
stack.Push(succ);
2350+
}
2351+
2352+
return BasicBlockVisit::Continue;
2353+
});
2354+
}
23052355

23062356
if (result == BasicBlockVisit::Abort)
23072357
{
2308-
return true;
2358+
return budgetExceeded ? ReachabilityResult::BudgetExceeded : ReachabilityResult::Reachable;
23092359
}
23102360
}
23112361

2312-
return false;
2362+
return ReachabilityResult::Unreachable;
23132363
}

0 commit comments

Comments
 (0)