diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp index 314baa7c7aee1..82e46e8800b3a 100644 --- a/llvm/lib/Analysis/ScalarEvolution.cpp +++ b/llvm/lib/Analysis/ScalarEvolution.cpp @@ -653,8 +653,7 @@ static int CompareValueComplexity(const LoopInfo *const LI, Value *LV, // If the max analysis depth was reached, return std::nullopt, assuming we do // not know if they are equivalent for sure. static std::optional -CompareSCEVComplexity(EquivalenceClasses &EqCacheSCEV, - const LoopInfo *const LI, const SCEV *LHS, +CompareSCEVComplexity(const LoopInfo *const LI, const SCEV *LHS, const SCEV *RHS, DominatorTree &DT, unsigned Depth = 0) { // Fast-path: SCEVs are uniqued so we can do a quick equality check. if (LHS == RHS) @@ -665,9 +664,6 @@ CompareSCEVComplexity(EquivalenceClasses &EqCacheSCEV, if (LType != RType) return (int)LType - (int)RType; - if (EqCacheSCEV.isEquivalent(LHS, RHS)) - return 0; - if (Depth > MaxSCEVCompareDepth) return std::nullopt; @@ -681,8 +677,6 @@ CompareSCEVComplexity(EquivalenceClasses &EqCacheSCEV, int X = CompareValueComplexity(LI, LU->getValue(), RU->getValue(), Depth + 1); - if (X == 0) - EqCacheSCEV.unionSets(LHS, RHS); return X; } @@ -747,12 +741,10 @@ CompareSCEVComplexity(EquivalenceClasses &EqCacheSCEV, return (int)LNumOps - (int)RNumOps; for (unsigned i = 0; i != LNumOps; ++i) { - auto X = CompareSCEVComplexity(EqCacheSCEV, LI, LOps[i], ROps[i], DT, - Depth + 1); + auto X = CompareSCEVComplexity(LI, LOps[i], ROps[i], DT, Depth + 1); if (X != 0) return X; } - EqCacheSCEV.unionSets(LHS, RHS); return 0; } @@ -775,11 +767,9 @@ static void GroupByComplexity(SmallVectorImpl &Ops, LoopInfo *LI, DominatorTree &DT) { if (Ops.size() < 2) return; // Noop - EquivalenceClasses EqCacheSCEV; - // Whether LHS has provably less complexity than RHS. auto IsLessComplex = [&](const SCEV *LHS, const SCEV *RHS) { - auto Complexity = CompareSCEVComplexity(EqCacheSCEV, LI, LHS, RHS, DT); + auto Complexity = CompareSCEVComplexity(LI, LHS, RHS, DT); return Complexity && *Complexity < 0; }; if (Ops.size() == 2) { diff --git a/llvm/unittests/Analysis/ScalarEvolutionTest.cpp b/llvm/unittests/Analysis/ScalarEvolutionTest.cpp index c72cecbba3cb8..95a4affdd7789 100644 --- a/llvm/unittests/Analysis/ScalarEvolutionTest.cpp +++ b/llvm/unittests/Analysis/ScalarEvolutionTest.cpp @@ -1706,4 +1706,35 @@ TEST_F(ScalarEvolutionsTest, ComplexityComparatorIsStrictWeakOrdering) { }); } +TEST_F(ScalarEvolutionsTest, ComplexityComparatorIsStrictWeakOrdering2) { + // Regression test for a case where caching of equivalent values caused the + // comparator to get inconsistent. + + Type *Int64Ty = Type::getInt64Ty(Context); + Type *PtrTy = PointerType::get(Context, 0); + FunctionType *FTy = FunctionType::get(Type::getVoidTy(Context), + {PtrTy, PtrTy, PtrTy, Int64Ty}, false); + Function *F = Function::Create(FTy, Function::ExternalLinkage, "f", M); + BasicBlock *BB = BasicBlock::Create(Context, "entry", F); + ReturnInst::Create(Context, nullptr, BB); + + ScalarEvolution SE = buildSE(*F); + + const SCEV *S0 = SE.getSCEV(F->getArg(0)); + const SCEV *S1 = SE.getSCEV(F->getArg(1)); + const SCEV *S2 = SE.getSCEV(F->getArg(2)); + + const SCEV *P0 = SE.getPtrToIntExpr(S0, Int64Ty); + const SCEV *P1 = SE.getPtrToIntExpr(S1, Int64Ty); + const SCEV *P2 = SE.getPtrToIntExpr(S2, Int64Ty); + + const SCEV *M0 = SE.getNegativeSCEV(P0); + const SCEV *M2 = SE.getNegativeSCEV(P2); + + SmallVector Ops = {M2, P0, M0, P1, P2}; + // When _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG, this will + // crash if the comparator has the specific caching bug. + SE.getAddExpr(Ops); +} + } // end namespace llvm