[llvm] 92f698f - Revert "[SCEV] Support clearing Block/LoopDispositions for a single value."

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 7 09:59:21 PDT 2022


Author: Florian Hahn
Date: 2022-10-07T17:58:54+01:00
New Revision: 92f698f01fa0bb8967233a6391ada2d51849fa68

URL: https://github.com/llvm/llvm-project/commit/92f698f01fa0bb8967233a6391ada2d51849fa68
DIFF: https://github.com/llvm/llvm-project/commit/92f698f01fa0bb8967233a6391ada2d51849fa68.diff

LOG: Revert "[SCEV] Support clearing Block/LoopDispositions for a single value."

This reverts commit 9e931439ddb9b6b8f655940b9d8ed6db50c2a7e2.

This commit causes a crash when TSan, e.g. with
https://lab.llvm.org/buildbot/#/builders/70/builds/28309/steps/10/logs/stdio

Reverting while I extract a reproducer and submit a fix.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/ScalarEvolution.h
    llvm/lib/Analysis/ScalarEvolution.cpp
    llvm/lib/Transforms/Scalar/LoopDeletion.cpp
    llvm/lib/Transforms/Scalar/LoopSink.cpp
    llvm/lib/Transforms/Utils/LoopSimplify.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index 580fef9f7a5d6..83f8a9e94bfe6 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -944,7 +944,7 @@ class ScalarEvolution {
   ///
   /// We don't have a way to invalidate per-loop/per-block dispositions. Clear
   /// and recompute is simpler.
-  void forgetBlockAndLoopDispositions(Value *V = nullptr);
+  void forgetBlockAndLoopDispositions();
 
   /// Determine the minimum number of zero bits that S is guaranteed to end in
   /// (at every loop iteration).  It is, at the same time, the minimum number

diff  --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 932be39358656..ac36aba386682 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -8384,36 +8384,9 @@ void ScalarEvolution::forgetValue(Value *V) {
 
 void ScalarEvolution::forgetLoopDispositions() { LoopDispositions.clear(); }
 
-void ScalarEvolution::forgetBlockAndLoopDispositions(Value *V) {
-  // Unless a specific value is passed to invalidation, completely clear both
-  // caches.
-  if (!V) {
-    BlockDispositions.clear();
-    LoopDispositions.clear();
-    return;
-  }
-
-  const SCEV *S = getExistingSCEV(V);
-  if (!S)
-    return;
-
-  // Invalidate the block and loop dispositions cached for S. Dispositions of
-  // S's users may change if S's disposition changes (i.e. a user may change to
-  // loop-invariant, if S changes to loop invariant), so also invalidate
-  // dispositions of S's users recursively.
-  SmallVector<const SCEV *, 8> Worklist = {S};
-  SmallPtrSet<const SCEV *, 8> Seen = {S};
-  while (!Worklist.empty()) {
-    const SCEV *Curr = Worklist.pop_back_val();
-    if (!LoopDispositions.erase(Curr) && !BlockDispositions.erase(S))
-      continue;
-
-    auto Users = SCEVUsers.find(Curr);
-    if (Users != SCEVUsers.end())
-      for (const auto *User : Users->second)
-        if (Seen.insert(User).second)
-          Worklist.push_back(User);
-  }
+void ScalarEvolution::forgetBlockAndLoopDispositions() {
+  BlockDispositions.clear();
+  LoopDispositions.clear();
 }
 
 /// Get the exact loop backedge taken count considering all loop exits. A

diff  --git a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
index f19f53e8153d3..d969126b3ad26 100644
--- a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
@@ -90,21 +90,22 @@ static bool isLoopDead(Loop *L, ScalarEvolution &SE,
         break;
 
       if (Instruction *I = dyn_cast<Instruction>(incoming)) {
-        bool InstrMoved = false;
-        if (!L->makeLoopInvariant(I, InstrMoved, Preheader->getTerminator())) {
+        if (!L->makeLoopInvariant(I, Changed, Preheader->getTerminator())) {
           AllEntriesInvariant = false;
           break;
         }
-        Changed |= InstrMoved;
-        if (InstrMoved) {
+        if (Changed) {
           // Moving I to a 
diff erent location may change its block disposition,
           // so invalidate its SCEV.
-          SE.forgetBlockAndLoopDispositions(I);
+          SE.forgetValue(I);
         }
       }
     }
   }
 
+  if (Changed)
+    SE.forgetLoopDispositions();
+
   if (!AllEntriesInvariant || !AllOutgoingValuesSame)
     return false;
 

diff  --git a/llvm/lib/Transforms/Scalar/LoopSink.cpp b/llvm/lib/Transforms/Scalar/LoopSink.cpp
index ff937273428e4..131af32e870db 100644
--- a/llvm/lib/Transforms/Scalar/LoopSink.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopSink.cpp
@@ -312,13 +312,12 @@ static bool sinkLoopInvariantInstructions(Loop &L, AAResults &AA, LoopInfo &LI,
     if (!canSinkOrHoistInst(I, &AA, &DT, &L, MSSAU, false, LICMFlags))
       continue;
     if (sinkInstruction(L, I, ColdLoopBBs, LoopBlockNumber, LI, DT, BFI,
-                        &MSSAU)) {
+                        &MSSAU))
       Changed = true;
-      if (SE)
-        SE->forgetBlockAndLoopDispositions(&I);
-    }
   }
 
+  if (Changed && SE)
+    SE->forgetLoopDispositions();
   return Changed;
 }
 

diff  --git a/llvm/lib/Transforms/Utils/LoopSimplify.cpp b/llvm/lib/Transforms/Utils/LoopSimplify.cpp
index f247a7eb33f87..16085f47449a5 100644
--- a/llvm/lib/Transforms/Utils/LoopSimplify.cpp
+++ b/llvm/lib/Transforms/Utils/LoopSimplify.cpp
@@ -647,22 +647,20 @@ static bool simplifyOneLoop(Loop *L, SmallVectorImpl<Loop *> &Worklist,
         Instruction *Inst = &*I++;
         if (Inst == CI)
           continue;
-        bool InstInvariant = false;
         if (!L->makeLoopInvariant(
-                Inst, InstInvariant,
+                Inst, AnyInvariant,
                 Preheader ? Preheader->getTerminator() : nullptr, MSSAU)) {
           AllInvariant = false;
           break;
         }
-        if (InstInvariant && SE) {
-          // The loop disposition of all SCEV expressions that depend on any
-          // hoisted values have also changed.
-          SE->forgetBlockAndLoopDispositions(Inst);
-        }
-        AnyInvariant |= InstInvariant;
       }
-      if (AnyInvariant)
+      if (AnyInvariant) {
         Changed = true;
+        // The loop disposition of all SCEV expressions that depend on any
+        // hoisted values have also changed.
+        if (SE)
+          SE->forgetLoopDispositions();
+      }
       if (!AllInvariant) continue;
 
       // The block has now been cleared of all instructions except for


        


More information about the llvm-commits mailing list