Skip to content

Commit 3abbf99

Browse files
JIT: Boost inversion for oversize loops with bounds checks (#118078)
Addresses some of the regressions in #116486. If a loop has bounds checks in it, it might benefit from cloning, so perhaps we ought to tolerate going a bit over the inversion size limit to enable downstream optimizations. I ought to do something for GDV checks; perhaps as a follow-up, I'll move the checks in loop cloning to something I can reuse here. The 1.25x figure isn't all that scientific -- I found it to be the smallest factor necessary to make a dent in the regressions from less cloning I looked at, and despite the size increases, it's a net PerfScore win across collections.
1 parent 8ea8809 commit 3abbf99

File tree

3 files changed

+40
-15
lines changed

3 files changed

+40
-15
lines changed

src/coreclr/jit/compiler.hpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5573,14 +5573,7 @@ bool Compiler::optLoopComplexityExceeds(FlowGraphNaturalLoop* loop, unsigned lim
55735573
: BasicBlockVisit::Continue;
55745574
});
55755575

5576-
if (result == BasicBlockVisit::Abort)
5577-
{
5578-
JITDUMP("Loop " FMT_LP ": exceeds complexity limit %u\n", loop->GetIndex(), limit);
5579-
return true;
5580-
}
5581-
5582-
JITDUMP("Loop " FMT_LP ": complexity %u does not exceed limit %u\n", loop->GetIndex(), loopComplexity, limit);
5583-
return false;
5576+
return (result == BasicBlockVisit::Abort);
55845577
}
55855578

55865579
/*****************************************************************************/

src/coreclr/jit/loopcloning.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3108,6 +3108,7 @@ PhaseStatus Compiler::optCloneLoops()
31083108
// then the method returns false.
31093109
else if ((sizeLimit >= 0) && optLoopComplexityExceeds(loop, (unsigned)sizeLimit, countNode))
31103110
{
3111+
JITDUMP(FMT_LP " exceeds cloning size limit %d\n", loop->GetIndex(), sizeLimit);
31113112
context.CancelLoopOptInfo(loop->GetIndex());
31123113
}
31133114
}

src/coreclr/jit/optimizer.cpp

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,14 +1903,45 @@ bool Compiler::optTryInvertWhileLoop(FlowGraphNaturalLoop* loop)
19031903

19041904
// Check if loop is small enough to consider for inversion.
19051905
// Large loops are less likely to benefit from inversion.
1906-
const int sizeLimit = JitConfig.JitLoopInversionSizeLimit();
1907-
auto countNode = [](GenTree* tree) -> unsigned {
1908-
return 1;
1909-
};
1906+
const int invertSizeLimit = JitConfig.JitLoopInversionSizeLimit();
1907+
if (invertSizeLimit >= 0)
1908+
{
1909+
const int cloneSizeLimit = JitConfig.JitCloneLoopsSizeLimit();
1910+
bool mightBenefitFromCloning = false;
1911+
unsigned loopSize = 0;
1912+
1913+
// Loops with bounds checks can benefit from cloning, which depends on inversion running.
1914+
// Thus, we will try to proceed with inversion for slightly oversize loops if they show potential for cloning.
1915+
auto countNode = [&mightBenefitFromCloning, &loopSize](GenTree* tree) -> unsigned {
1916+
mightBenefitFromCloning |= tree->OperIs(GT_BOUNDS_CHECK);
1917+
loopSize++;
1918+
return 1;
1919+
};
19101920

1911-
if ((sizeLimit >= 0) && optLoopComplexityExceeds(loop, (unsigned)sizeLimit, countNode))
1912-
{
1913-
return false;
1921+
optLoopComplexityExceeds(loop, (unsigned)max(invertSizeLimit, cloneSizeLimit), countNode);
1922+
if (loopSize > (unsigned)invertSizeLimit)
1923+
{
1924+
// Don't try to invert oversize loops if they don't show cloning potential,
1925+
// or if they're too big to be cloned anyway.
1926+
JITDUMP(FMT_LP " exceeds inversion size limit of %d\n", loop->GetIndex(), invertSizeLimit);
1927+
const bool tooBigToClone = (cloneSizeLimit >= 0) && (loopSize > (unsigned)cloneSizeLimit);
1928+
if (!mightBenefitFromCloning || tooBigToClone)
1929+
{
1930+
JITDUMP("No inversion for " FMT_LP ": %s\n", loop->GetIndex(),
1931+
tooBigToClone ? "too big to clone" : "unlikely to benefit from cloning");
1932+
return false;
1933+
}
1934+
1935+
// If the loop shows cloning potential, tolerate some excess size.
1936+
const unsigned liberalInvertSizeLimit = (unsigned)(invertSizeLimit * 1.25);
1937+
if (loopSize > liberalInvertSizeLimit)
1938+
{
1939+
JITDUMP(FMT_LP " might benefit from cloning, but is too large to invert.\n", loop->GetIndex());
1940+
return false;
1941+
}
1942+
1943+
JITDUMP(FMT_LP " might benefit from cloning. Continuing.\n", loop->GetIndex());
1944+
}
19141945
}
19151946

19161947
unsigned estDupCostSz = 0;

0 commit comments

Comments
 (0)