LoopInfo: Simplify ownership of Loop objects

It's strange that LoopInfo mostly owns the Loop objects, but that it
defers deleting them to the loop pass manager. Instead, change the
oddly named "updateUnloop" to "markAsRemoved" and have it queue the
Loop object for deletion. We can't delete the Loop immediately when we
remove it, since we need its pointer identity still, so we'll mark the
object as "invalid" so that clients can see what's going on.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@257191 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Analysis/LoopInfo.h b/include/llvm/Analysis/LoopInfo.h
index c219bd85a48abb0ba972c2e225f081fe20d68a86..a2bd6574e615b585ab5a0a350d76e0df7752bf22 100644 (file)
--- a/include/llvm/Analysis/LoopInfo.h
+++ b/include/llvm/Analysis/LoopInfo.h
@@ -73,9 +73,8 @@ class LoopBase {
 
   SmallPtrSet<const BlockT*, 8> DenseBlockSet;
 
-  /// Indicator that this loops has been "unlooped", so there's no loop here
-  /// anymore.
-  bool IsUnloop = false;
+  /// Indicator that this loop is no longer a valid loop.
+  bool IsInvalid = false;
 
   LoopBase(const LoopBase<BlockT, LoopT> &) = delete;
   const LoopBase<BlockT, LoopT>&
@@ -154,12 +153,11 @@ public:
     return Blocks.size();
   }
 
-  /// Mark this loop as having been unlooped - the last backedge was removed and
-  /// we no longer have a loop.
-  void markUnlooped() { IsUnloop = true; }
+  /// Invalidate the loop, indicating that it is no longer a loop.
+  void invalidate() { IsInvalid = true; }
 
-  /// Return true if this no longer represents a loop.
-  bool isUnloop() const { return IsUnloop; }
+  /// Return true if this loop is no longer valid.
+  bool isInvalid() { return IsInvalid; }
 
   /// isLoopExiting - True if terminator in the block can branch to another
   /// block that is outside of the current loop.
@@ -507,6 +505,8 @@ class LoopInfoBase {
   // BBMap - Mapping of basic blocks to the inner most loop they occur in
   DenseMap<const BlockT *, LoopT *> BBMap;
   std::vector<LoopT *> TopLevelLoops;
+  std::vector<LoopT *> RemovedLoops;
+
   friend class LoopBase<BlockT, LoopT>;
   friend class LoopInfo;
 
@@ -538,6 +538,9 @@ public:
     for (auto *L : TopLevelLoops)
       delete L;
     TopLevelLoops.clear();
+    for (auto *L : RemovedLoops)
+      delete L;
+    RemovedLoops.clear();
   }
 
   /// iterator/begin/end - The interface to the top-level loops in the current
@@ -670,12 +673,11 @@ public:
 
   // Most of the public interface is provided via LoopInfoBase.
 
-  /// updateUnloop - Update LoopInfo after removing the last backedge from a
-  /// loop--now the "unloop". This updates the loop forest and parent loops for
-  /// each block so that Unloop is no longer referenced, but does not actually
-  /// delete the Unloop object. Generally, the loop pass manager should manage
-  /// deleting the Unloop.
-  void updateUnloop(Loop *Unloop);
+  /// Update LoopInfo after removing the last backedge from a loop. This updates
+  /// the loop forest and parent loops for each block so that \c L is no longer
+  /// referenced, but does not actually delete \c L immediately. The pointer
+  /// will remain valid until this LoopInfo's memory is released.
+  void markAsRemoved(Loop *L);
 
   /// replacementPreservesLCSSAForm - Returns true if replacing From with To
   /// everywhere is guaranteed to preserve LCSSA form.

diff --git a/lib/Analysis/LoopInfo.cpp b/lib/Analysis/LoopInfo.cpp
index 9ab9eead584fc64956114623d9d2e0a2f2b55acb..0c725fcadff74316d40d3c91be5ebacfbe5b2603 100644 (file)
--- a/lib/Analysis/LoopInfo.cpp
+++ b/lib/Analysis/LoopInfo.cpp
@@ -637,8 +637,10 @@ LoopInfo::LoopInfo(const DominatorTreeBase<BasicBlock> &DomTree) {
   analyze(DomTree);
 }
 
-void LoopInfo::updateUnloop(Loop *Unloop) {
-  Unloop->markUnlooped();
+void LoopInfo::markAsRemoved(Loop *Unloop) {
+  assert(!Unloop->isInvalid() && "Loop has already been removed");
+  Unloop->invalidate();
+  RemovedLoops.push_back(Unloop);
 
   // First handle the special case of no parent loop to simplify the algorithm.
   if (!Unloop->getParentLoop()) {

diff --git a/lib/Analysis/LoopPass.cpp b/lib/Analysis/LoopPass.cpp
index e24a9e46fc1577a3a6104ff2818dbcc226937c4c..8163231c3323a1e832d8aaaf7a548d39ce4fdc99 100644 (file)
--- a/lib/Analysis/LoopPass.cpp
+++ b/lib/Analysis/LoopPass.cpp
@@ -178,8 +178,9 @@ bool LPPassManager::runOnFunction(Function &F) {
 
   // Walk Loops
   while (!LQ.empty()) {
-
+    bool LoopWasDeleted = false;
     CurrentLoop = LQ.back();
+
     // Run all passes on the current Loop.
     for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
       LoopPass *P = getContainedPass(Index);
@@ -196,15 +197,15 @@ bool LPPassManager::runOnFunction(Function &F) {
 
         Changed |= P->runOnLoop(CurrentLoop, *this);
       }
+      LoopWasDeleted = CurrentLoop->isInvalid();
 
       if (Changed)
         dumpPassInfo(P, MODIFICATION_MSG, ON_LOOP_MSG,
-                     CurrentLoop->isUnloop()
-                         ? "<deleted>"
-                         : CurrentLoop->getHeader()->getName());
+                     LoopWasDeleted ? "<deleted>"
+                                    : CurrentLoop->getHeader()->getName());
       dumpPreservedSet(P);
 
-      if (CurrentLoop->isUnloop()) {
+      if (LoopWasDeleted) {
         // Notify passes that the loop is being deleted.
         deleteSimpleAnalysisLoop(CurrentLoop);
       } else {
@@ -226,12 +227,11 @@ bool LPPassManager::runOnFunction(Function &F) {
 
       removeNotPreservedAnalysis(P);
       recordAvailableAnalysis(P);
-      removeDeadPasses(P, CurrentLoop->isUnloop()
-                              ? "<deleted>"
-                              : CurrentLoop->getHeader()->getName(),
+      removeDeadPasses(P, LoopWasDeleted ? "<deleted>"
+                                         : CurrentLoop->getHeader()->getName(),
                        ON_LOOP_MSG);
 
-      if (CurrentLoop->isUnloop())
+      if (LoopWasDeleted)
         // Do not run other passes on this loop.
         break;
     }
@@ -239,12 +239,11 @@ bool LPPassManager::runOnFunction(Function &F) {
     // If the loop was deleted, release all the loop passes. This frees up
     // some memory, and avoids trouble with the pass manager trying to call
     // verifyAnalysis on them.
-    if (CurrentLoop->isUnloop()) {
+    if (LoopWasDeleted) {
       for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
         Pass *P = getContainedPass(Index);
         freePass(P, "<deleted>", ON_LOOP_MSG);
       }
-      delete CurrentLoop;
     }
 
     // Pop the loop from queue after running all passes.

diff --git a/lib/Transforms/IPO/LoopExtractor.cpp b/lib/Transforms/IPO/LoopExtractor.cpp
index 8e4ad642ddd50b340aaaa3841282edf0dbb6a948..3c6a7bb7a17ab2cee24f6bd15636ea51af3d1222 100644 (file)
--- a/lib/Transforms/IPO/LoopExtractor.cpp
+++ b/lib/Transforms/IPO/LoopExtractor.cpp
@@ -38,7 +38,7 @@ namespace {
     static char ID; // Pass identification, replacement for typeid
     unsigned NumLoops;
 
-    explicit LoopExtractor(unsigned numLoops = ~0) 
+    explicit LoopExtractor(unsigned numLoops = ~0)
       : LoopPass(ID), NumLoops(numLoops) {
         initializeLoopExtractorPass(*PassRegistry::getPassRegistry());
       }
@@ -143,7 +143,7 @@ bool LoopExtractor::runOnLoop(Loop *L, LPPassManager &) {
       Changed = true;
       // After extraction, the loop is replaced by a function call, so
       // we shouldn't try to run any more loop passes on it.
-      LI.updateUnloop(L);
+      LI.markAsRemoved(L);
     }
     ++NumExtracted;
   }

diff --git a/lib/Transforms/Scalar/LoopDeletion.cpp b/lib/Transforms/Scalar/LoopDeletion.cpp
index bc00ff3f3a424ca94e3a564dd845f44ae8e41077..7b1940b48c31bcdbaaf7360521b01072418fb474 100644 (file)
--- a/lib/Transforms/Scalar/LoopDeletion.cpp
+++ b/lib/Transforms/Scalar/LoopDeletion.cpp
@@ -245,7 +245,7 @@ bool LoopDeletion::runOnLoop(Loop *L, LPPassManager &) {
     loopInfo.removeBlock(BB);
 
   // The last step is to update LoopInfo now that we've eliminated this loop.
-  loopInfo.updateUnloop(L);
+  loopInfo.markAsRemoved(L);
   Changed = true;
 
   ++NumDeleted;

diff --git a/lib/Transforms/Utils/LoopUnroll.cpp b/lib/Transforms/Utils/LoopUnroll.cpp
index 2499b88741fe363f57f3b50a57cf3829adb396e3..eea9237ba80c68a47ab40b92a23835a428ac6324 100644 (file)
--- a/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/lib/Transforms/Utils/LoopUnroll.cpp
@@ -528,7 +528,7 @@ bool llvm::UnrollLoop(Loop *L, unsigned Count, unsigned TripCount,
   Loop *OuterL = L->getParentLoop();
   // Update LoopInfo if the loop is completely removed.
   if (CompletelyUnroll)
-    LI->updateUnloop(L);;
+    LI->markAsRemoved(L);
 
   // If we have a pass and a DominatorTree we should re-simplify impacted loops
   // to ensure subsequent analyses can rely on this form. We want to simplify
@@ -542,7 +542,7 @@ bool llvm::UnrollLoop(Loop *L, unsigned Count, unsigned TripCount,
 
       // LCSSA must be performed on the outermost affected loop. The unrolled
       // loop's last loop latch is guaranteed to be in the outermost loop after
-      // LoopInfo's been updated by updateUnloop.
+      // LoopInfo's been updated by markAsRemoved.
       Loop *LatchLoop = LI->getLoopFor(Latches.back());
       if (!OuterL->contains(LatchLoop))
         while (OuterL->getParentLoop() != LatchLoop)

diff --git a/test/Transforms/LoopUnroll/unloop.ll b/test/Transforms/LoopUnroll/unloop.ll
index b98b4a3fffba968f2be4905a4e0d57f46b6df009..720b2ae1bdbcea21e2c50e28bef86dd64aadfc1c 100644 (file)
--- a/test/Transforms/LoopUnroll/unloop.ll
+++ b/test/Transforms/LoopUnroll/unloop.ll
@@ -1,6 +1,6 @@
 ; RUN: opt < %s -S -loop-unroll -verify-loop-info | FileCheck %s
 ;
-; Unit tests for LoopInfo::updateUnloop.
+; Unit tests for LoopInfo::markAsRemoved.
 
 declare i1 @check() nounwind