[llvm] [IndVarSimplify] Batch forgetValue calls in sinkUnusedInvariants (PR #219025)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Anonmiraj (AnonMiraj)

<details>
<summary>Changes</summary>

It looks like every `forgetValue` clears the cached SCEV for an instruction and everything downstream of it, by walking its def-use children in `visitAndClearUsers` / `PushDefUseChildren` with a fresh `Visited` each call. Since `sinkUnusedInvariants` calls `forgetValue` once per sunk instruction, the overlapping users get re-walked over and over, so it ends up $O(n^2)$.
```cpp
void ScalarEvolution::forgetValue(Value *V) {
  SmallPtrSet<Instruction *, 8> Visited;
  visitAndClearUsers(Worklist, Visited, ToForget);
  ...
}

// visitAndClearUsers
while (!Worklist.empty()) {
  Instruction *I = Worklist.pop_back_val();
  ...
  ValueExprMap ... erase ...
  PushDefUseChildren(I, Worklist, Visited);
}

// PushDefUseChildren
for (User *U : I->users()) {
  auto *UserInsn = cast<Instruction>(U);
  if (Visited.insert(UserInsn).second)
    Worklist.push_back(UserInsn);
}
```


Collecting the sunk instructions and clearing them in one batched call fixes the issue.

Before / after on the reproducer from the issue

| | before | after |
|---|---|---|
| `IndVarSimplifyPass` | 226.70s | 0.087s |
| total compile (wall) | 3:45.7 | 3.9s |

fixes #<!-- -->194308 


---
Full diff: https://github.com/llvm/llvm-project/pull/219025.diff


3 Files Affected:

- (modified) llvm/include/llvm/Analysis/ScalarEvolution.h (+4) 
- (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+13) 
- (modified) llvm/lib/Transforms/Scalar/IndVarSimplify.cpp (+5-1) 


``````````diff
diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index 0a9a4370d5e80..147da1af980cf 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -1154,6 +1154,10 @@ class ScalarEvolution {
   /// def-use chain linking it to a loop.
   LLVM_ABI void forgetValue(Value *V);
 
+  /// Batched forgetValue: invalidates all \p Values in one shared def-use walk,
+  /// avoiding the redundant re-traversal of overlapping users.
+  LLVM_ABI void forgetValues(ArrayRef<Value *> Values);
+
   /// Forget LCSSA phi node V of loop L to which a new predecessor was added,
   /// such that it may no longer be trivial.
   LLVM_ABI void forgetLcssaPhiWithNewPredecessor(Loop *L, PHINode *V);
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index c74708fc73e5a..2e95b8db6def2 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -8856,6 +8856,19 @@ void ScalarEvolution::forgetValue(Value *V) {
   forgetMemoizedResults(ToForget);
 }
 
+void ScalarEvolution::forgetValues(ArrayRef<Value *> Values) {
+  SmallVector<Instruction *, 16> Worklist;
+  SmallPtrSet<Instruction *, 8> Visited;
+  SmallVector<SCEVUse, 8> ToForget;
+  for (Value *V : Values)
+    if (auto *I = dyn_cast<Instruction>(V))
+      if (Visited.insert(I).second)
+        Worklist.push_back(I);
+  visitAndClearUsers(Worklist, Visited, ToForget);
+
+  forgetMemoizedResults(ToForget);
+}
+
 void ScalarEvolution::forgetLcssaPhiWithNewPredecessor(Loop *L, PHINode *V) {
   if (!isSCEVable(V->getType()))
     return;
diff --git a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
index c92efadded635..a5eefade336b7 100644
--- a/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ b/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -1211,6 +1211,7 @@ bool IndVarSimplify::sinkUnusedInvariants(Loop *L) {
   if (!Preheader) return false;
 
   bool MadeAnyChanges = false;
+  SmallVector<Value *, 16> SunkInsts;
   for (Instruction &I : llvm::make_early_inc_range(llvm::reverse(*Preheader))) {
 
     // Skip BB Terminator.
@@ -1268,10 +1269,13 @@ bool IndVarSimplify::sinkUnusedInvariants(Loop *L) {
 
     // Otherwise, sink it to the exit block.
     I.moveBefore(ExitBlock->getFirstInsertionPt());
-    SE->forgetValue(&I);
+    SunkInsts.push_back(&I);
     MadeAnyChanges = true;
   }
 
+  if (MadeAnyChanges)
+    SE->forgetValues(SunkInsts);
+
   return MadeAnyChanges;
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/219025