Skip to content

Commit 24bc1b7

Browse files
authored
JIT: Remove BBJ_RETURN loop cloning quirk (#96555)
With the new loop representation there will never be any `BBJ_RETURN` blocks in loops we are cloning -- add an assert for this. Then remove the quirk that avoids cloning loops when the old loop cloning would create too many `BBJ_RETURN` blocks. Large size wise regressions are expected due to new cloning.
1 parent d5bc37d commit 24bc1b7

1 file changed

Lines changed: 14 additions & 32 deletions

File tree

src/coreclr/jit/loopcloning.cpp

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1790,7 +1790,6 @@ bool Compiler::optIsLoopClonable(FlowGraphNaturalLoop* loop, LoopCloneContext* c
17901790
// for the cloned loop (and its embedded EH regions).
17911791
//
17921792
BasicBlockVisit result = loop->VisitLoopBlocks([=](BasicBlock* blk) {
1793-
assert(!blk->KindIs(BBJ_RETURN));
17941793
if (bbIsTryBeg(blk))
17951794
{
17961795
JITDUMP("Loop cloning: rejecting loop " FMT_LP ". It has a `try` begin.\n", loop->GetIndex());
@@ -1805,6 +1804,19 @@ bool Compiler::optIsLoopClonable(FlowGraphNaturalLoop* loop, LoopCloneContext* c
18051804
return false;
18061805
}
18071806

1807+
#ifdef DEBUG
1808+
// With the EH constraint above verified it is not possible for
1809+
// BBJ_RETURN blocks to be part of the loop; a BBJ_RETURN block can
1810+
// only be part of the loop if its exceptional flow can reach the
1811+
// header, but that would require the handler to also be part of
1812+
// the loop, which guarantees that the loop contains two distinct
1813+
// EH regions.
1814+
loop->VisitLoopBlocks([](BasicBlock* block) {
1815+
assert(!block->KindIs(BBJ_RETURN));
1816+
return BasicBlockVisit::Continue;
1817+
});
1818+
#endif
1819+
18081820
// Is the entry block a handler or filter start? If so, then if we cloned, we could create a jump
18091821
// into the middle of a handler (to go to the cloned copy.) Reject.
18101822
if (bbIsHandlerBeg(loop->GetHeader()))
@@ -1842,37 +1854,7 @@ bool Compiler::optIsLoopClonable(FlowGraphNaturalLoop* loop, LoopCloneContext* c
18421854
return false;
18431855
}
18441856

1845-
// TODO-Quirk: Reject loops that with the old loop cloning would put us
1846-
// above max number of returns. Return blocks are not considered part of
1847-
// loops in the new loop finding since they are never part of the SCC (and
1848-
// they aren't present inside try-regions).
1849-
//
1850-
LoopDsc* oldLoop = m_newToOldLoop[loop->GetIndex()];
1851-
unsigned loopRetCount = 0;
1852-
for (BasicBlock* const blk : oldLoop->LoopBlocks())
1853-
{
1854-
if (blk->KindIs(BBJ_RETURN))
1855-
{
1856-
loopRetCount++;
1857-
}
1858-
}
1859-
1860-
// We've previously made a decision whether to have separate return epilogs, or branch to one.
1861-
// There's a GCInfo limitation in the x86 case, so that there can be no more than SET_EPILOGCNT_MAX separate
1862-
// epilogs. Other architectures have a limit of 4 here for "historical reasons", but this should be revisited
1863-
// (or return blocks should not be considered part of the loop, rendering this issue moot).
1864-
unsigned epilogLimit = 4;
1865-
#ifdef JIT32_GCENCODER
1866-
epilogLimit = SET_EPILOGCNT_MAX;
1867-
#endif // JIT32_GCENCODER
1868-
if (fgReturnCount + loopRetCount > epilogLimit)
1869-
{
1870-
JITDUMP("Loop cloning: rejecting loop " FMT_LP ". It has %d returns;"
1871-
" if added to previously existing %d returns, it would exceed the limit of %d.\n",
1872-
loop->GetIndex(), loopRetCount, fgReturnCount, epilogLimit);
1873-
return false;
1874-
}
1875-
1857+
LoopDsc* oldLoop = m_newToOldLoop[loop->GetIndex()];
18761858
assert(!requireIterable || !lvaVarAddrExposed(iterInfo->IterVar));
18771859

18781860
if (requireIterable)

0 commit comments

Comments
 (0)