[llvm-branch-commits] [llvm] release/23.x: Revert "[SCEV] Speed up forgetLoop by avoiding def-use walk for loop-header PHIs" (#212485) (PR #214651)

[Douglas Yung via llvm-branch-commits]

https://github.com/dyung updated https://github.com/llvm/llvm-project/pull/214651

>From 6278eba367e2f0eebfdc31ab8b29a02ca27e6f15 Mon Sep 17 00:00:00 2001
From: Mingjie Xu <xumingjie.enna1 at bytedance.com>
Date: Fri, 7 Aug 2026 09:15:47 +0800
Subject: [PATCH] Revert "[SCEV] Speed up forgetLoop by avoiding def-use walk
 for loop-header PHIs" (#212485)

Reverts https://github.com/llvm/llvm-project/pull/201572
Multiple miscompilations are reported, see
https://github.com/llvm/llvm-project/issues/207744,
https://github.com/llvm/llvm-project/issues/212027

That commit made forgetLoop() rely on LoopUsers[L] and stop walking the
def-use chain of the loop-header PHIs. This is insufficient, because
some cached data is derived from the underlying IR of SCEVUnknown, it is
not reachable from LoopUsers[L].
After that commit, forgetLoop() no longer invalidated them, so stale
UnsignedRanges / SignedRanges, ConstantMultipleCache, ValuesAtScopes
cause miscompilations.

(cherry picked from commit 1c0eda0d2371a6a755f90299892a7b89dc917442)
---
 llvm/lib/Analysis/ScalarEvolution.cpp         | 22 ++++++++---
 .../Analysis/ScalarEvolutionTest.cpp          | 39 -------------------
 2 files changed, 16 insertions(+), 45 deletions(-)

diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 134be6ac097e0..ab8e73f514cd2 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -8680,6 +8680,18 @@ bool ScalarEvolution::isBackedgeTakenCountMaxOrZero(const Loop *L) {
   return getBackedgeTakenInfo(L).isConstantMaxOrZero(this);
 }
 
+/// Push PHI nodes in the header of the given loop onto the given Worklist.
+static void PushLoopPHIs(const Loop *L,
+                         SmallVectorImpl<Instruction *> &Worklist,
+                         SmallPtrSetImpl<Instruction *> &Visited) {
+  BasicBlock *Header = L->getHeader();
+
+  // Push all Loop-header PHIs onto the Worklist stack.
+  for (PHINode &PN : Header->phis())
+    if (Visited.insert(&PN).second)
+      Worklist.push_back(&PN);
+}
+
 ScalarEvolution::BackedgeTakenInfo &
 ScalarEvolution::getPredicatedBackedgeTakenInfo(const Loop *L) {
   auto &BTI = getBackedgeTakenInfo(L);
@@ -8789,6 +8801,8 @@ void ScalarEvolution::visitAndClearUsers(
 
 void ScalarEvolution::forgetLoop(const Loop *L) {
   SmallVector<const Loop *, 16> LoopWorklist(1, L);
+  SmallVector<Instruction *, 32> Worklist;
+  SmallPtrSet<Instruction *, 16> Visited;
   SmallVector<SCEVUse, 16> ToForget;
 
   // Iterate over all the loops and sub-loops to drop SCEV information.
@@ -8808,12 +8822,8 @@ void ScalarEvolution::forgetLoop(const Loop *L) {
       llvm::append_range(ToForget, LoopUsersItr->second);
 
     // Drop information about expressions based on loop-header PHIs.
-    for (PHINode &PN : CurrL->getHeader()->phis()) {
-      ConstantEvolutionLoopExitValue.erase(&PN);
-      auto VIt = ValueExprMap.find_as(static_cast<Value *>(&PN));
-      if (VIt != ValueExprMap.end())
-        ToForget.push_back(VIt->second);
-    }
+    PushLoopPHIs(CurrL, Worklist, Visited);
+    visitAndClearUsers(Worklist, Visited, ToForget);
 
     LoopPropertiesCache.erase(CurrL);
     // Forget all contained loops too, to avoid dangling entries in the
diff --git a/llvm/unittests/Analysis/ScalarEvolutionTest.cpp b/llvm/unittests/Analysis/ScalarEvolutionTest.cpp
index 99edc5b25f3e2..1e290684d3aab 100644
--- a/llvm/unittests/Analysis/ScalarEvolutionTest.cpp
+++ b/llvm/unittests/Analysis/ScalarEvolutionTest.cpp
@@ -1709,45 +1709,6 @@ TEST_F(ScalarEvolutionsTest, ForgetValueWithOverflowInst) {
   });
 }
 
-TEST_F(ScalarEvolutionsTest, ForgetLoopPreservesUnrelatedCachesInLoopBody) {
-  LLVMContext C;
-  SMDiagnostic Err;
-  std::unique_ptr<Module> M =
-      parseAssemblyString("define void @foo(i32 %n) { "
-                          "entry: "
-                          "  br label %loop "
-                          "loop: "
-                          "  %iv = phi i32 [ 0, %entry ], [ %iv.next, %loop ] "
-                          "  %iv.next = add nsw i32 %iv, 1 "
-                          "  %cmp = icmp slt i32 %iv, %n "
-                          "  br i1 %cmp, label %loop, label %exit "
-                          "exit: "
-                          "  ret void "
-                          "} ",
-                          Err, C);
-
-  ASSERT_TRUE(M && "Could not parse module?");
-  ASSERT_TRUE(!verifyModule(*M) && "Must have been well formed!");
-
-  runWithSE(*M, "foo", [](Function &F, LoopInfo &LI, ScalarEvolution &SE) {
-    auto *IV = getInstructionByName(F, "iv");
-    auto *Cmp = getInstructionByName(F, "cmp");
-
-    const SCEV *IVScev = SE.getSCEV(IV);
-    EXPECT_NE(IVScev, nullptr);
-    EXPECT_TRUE(isa<SCEVAddRecExpr>(IVScev));
-
-    const SCEV *CmpScev = SE.getSCEV(Cmp);
-    EXPECT_NE(CmpScev, nullptr);
-    EXPECT_TRUE(isa<SCEVUnknown>(CmpScev));
-
-    Loop *L = *LI.begin();
-    SE.forgetLoop(L);
-    EXPECT_EQ(SE.getExistingSCEV(IV), nullptr);
-    EXPECT_EQ(SE.getExistingSCEV(Cmp), CmpScev);
-  });
-}
-
 TEST_F(ScalarEvolutionsTest, ComplexityComparatorIsStrictWeakOrdering) {
   // Regression test for a case where caching of equivalent values caused the
   // comparator to get inconsistent.