Skip to content

Commit 68b53ea

Browse files
committed
[SCEV] Return nullopt from CompareValueComplexity() if depth limit reached
Similarly to CompareSCEVComplexity() itself, we should return nullopt instead of 0 from CompareValueComplexity(), because we do not know how the values related to each other. This means that the values will not be added to the equality cache, and it can not become inconsistent due to different results at different depths. The test case is adopted from llvm#100721.
1 parent 73c72f2 commit 68b53ea

File tree

2 files changed

+44
-5
lines changed

2 files changed

+44
-5
lines changed

llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -597,11 +597,14 @@ void SCEVUnknown::allUsesReplacedWith(Value *New) {
597597
///
598598
/// Since we do not continue running this routine on expression trees once we
599599
/// have seen unequal values, there is no need to track them in the cache.
600-
static int
600+
static std::optional<int>
601601
CompareValueComplexity(EquivalenceClasses<const Value *> &EqCacheValue,
602602
const LoopInfo *const LI, Value *LV, Value *RV,
603603
unsigned Depth) {
604-
if (Depth > MaxValueCompareDepth || EqCacheValue.isEquivalent(LV, RV))
604+
if (Depth > MaxValueCompareDepth)
605+
return std::nullopt;
606+
607+
if (EqCacheValue.isEquivalent(LV, RV))
605608
return 0;
606609

607610
// Order pointer values after integer values. This helps SCEVExpander form
@@ -660,7 +663,7 @@ CompareValueComplexity(EquivalenceClasses<const Value *> &EqCacheValue,
660663
return (int)LNumOps - (int)RNumOps;
661664

662665
for (unsigned Idx : seq(LNumOps)) {
663-
int Result =
666+
std::optional<int> Result =
664667
CompareValueComplexity(EqCacheValue, LI, LInst->getOperand(Idx),
665668
RInst->getOperand(Idx), Depth + 1);
666669
if (Result != 0)
@@ -705,8 +708,8 @@ CompareSCEVComplexity(EquivalenceClasses<const SCEV *> &EqCacheSCEV,
705708
const SCEVUnknown *LU = cast<SCEVUnknown>(LHS);
706709
const SCEVUnknown *RU = cast<SCEVUnknown>(RHS);
707710

708-
int X = CompareValueComplexity(EqCacheValue, LI, LU->getValue(),
709-
RU->getValue(), Depth + 1);
711+
std::optional<int> X = CompareValueComplexity(
712+
EqCacheValue, LI, LU->getValue(), RU->getValue(), Depth + 1);
710713
if (X == 0)
711714
EqCacheSCEV.unionSets(LHS, RHS);
712715
return X;

llvm/unittests/Analysis/ScalarEvolutionTest.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,4 +1625,40 @@ TEST_F(ScalarEvolutionsTest, ForgetValueWithOverflowInst) {
16251625
});
16261626
}
16271627

1628+
TEST_F(ScalarEvolutionsTest, ComplexityComparatorIsStrictWeakOrdering) {
1629+
// Regression test for a case where caching of equivalent values caused the
1630+
// comparator to get inconsistent.
1631+
LLVMContext C;
1632+
SMDiagnostic Err;
1633+
std::unique_ptr<Module> M = parseAssemblyString(R"(
1634+
define i32 @foo(i32 %arg0) {
1635+
%1 = add i32 %arg0, 1
1636+
%2 = add i32 %arg0, 1
1637+
%3 = xor i32 %2, %1
1638+
%4 = add i32 %3, %2
1639+
%5 = add i32 %arg0, 1
1640+
%6 = xor i32 %5, %arg0
1641+
%7 = add i32 %arg0, %6
1642+
%8 = add i32 %5, %7
1643+
%9 = xor i32 %8, %7
1644+
%10 = add i32 %9, %8
1645+
%11 = xor i32 %10, %9
1646+
%12 = add i32 %11, %10
1647+
%13 = xor i32 %12, %11
1648+
%14 = add i32 %12, %13
1649+
%15 = add i32 %14, %4
1650+
ret i32 %15
1651+
})",
1652+
Err, C);
1653+
1654+
ASSERT_TRUE(M && "Could not parse module?");
1655+
ASSERT_TRUE(!verifyModule(*M) && "Must have been well formed!");
1656+
1657+
runWithSE(*M, "foo", [](Function &F, LoopInfo &LI, ScalarEvolution &SE) {
1658+
// When _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG, this will
1659+
// crash if the comparator has the specific caching bug.
1660+
SE.getSCEV(F.getEntryBlock().getTerminator()->getOperand(0));
1661+
});
1662+
}
1663+
16281664
} // end namespace llvm

0 commit comments

Comments
 (0)