Tighten the invariants around LoopBase::invalidate

[sanjoy]

Summary

With this change:

• Methods in LoopBase trip an assert if the receiver has been invalidated
• LoopBase::invalidate frees up the memory held the LoopBase instance

This change also shuffles things around as necessary to work with this stricter invariant.

[chandlerc]

Generally like the API changes here. The enum seems a big improvement.

include/llvm/Analysis/LoopInfo.h

68–82 ↗

(On Diff #115904)

Given the number of formatting changes here, can you do an initial NFC commit w/ clang-format and rebase?

85 ↗

(On Diff #115904)

I find clear a better name for these methods. And eventually you may just run the destructor.

I don't want it to be confused with invalidating analyses *on* the loop with a loop analysis manager.

86 ↗

(On Diff #115904)

This seems redundant if you set ParentLoop to null? I guess not for top-level loops.

But it is also redundant with an empty Blocks vector. Anyways, I'd try to remove this at some point.

[sanjoy]

• Address review comments

[sanjoy]

include/llvm/Analysis/LoopInfo.h

86 ↗

(On Diff #115904)

Yes, this is equivalent to Blocks.empty(). Do you want me to roll that into this change? Or is a separate change fine?

[chandlerc]

Generally looks good. Description should likely be updated to reflect changes made during review. Feel free to submit!

include/llvm/Transforms/Scalar/LoopPassManager.h

169 ↗

(On Diff #115949)

This seems like an unrelated change? (doesn't seem bad...)

[sanjoy]

include/llvm/Transforms/Scalar/LoopPassManager.h

169 ↗

(On Diff #115949)

markLoopAsDeleted is called after L has been cleared. If L is the same as CurrentL then CurrentL->contains(&L) will assert.

[sberg]

llvm/trunk/include/llvm/Transforms/Scalar/LoopPassManager.h

170

there's parentheses missing around the || here, right?

[sanjoy]

llvm/trunk/include/llvm/Transforms/Scalar/LoopPassManager.h

170

Since the string evaluates to true, this shouldn't matter -- "(A || B) && True" == "(A || B)" == "A || (B && True)".

Is this causing a warning on some compiler? If so, let me know and I'll fix it (or feel free to directly commit a fix yourself).

[sberg]

llvm/trunk/include/llvm/Transforms/Scalar/LoopPassManager.h

170

Since the string evaluates to true, this shouldn't matter -- "(A || B) && True" == "(A || B)" == "A || (B && True)".

Sure, but caused -Wparentheses with GCC. Anyway, found fixed meanwhile with r313780.

llvm/trunk/include/llvm/Analysis/LoopInfo.h

template <class BlockT, class LoopT> class LoopBase {
 
   /// Indicator that this loop is no longer a valid loop.
   bool IsInvalid = false;
 
   LoopBase(const LoopBase<BlockT, LoopT> &) = delete;
   const LoopBase<BlockT, LoopT> &
   operator=(const LoopBase<BlockT, LoopT> &) = delete;
 
+  void clear() {
+    IsInvalid = true;
+    SubLoops.clear();
+    Blocks.clear();
+    DenseBlockSet.clear();
+    ParentLoop = nullptr;
+  }
+
 public:
   /// This creates an empty loop.
   LoopBase() : ParentLoop(nullptr) {}
 
   /// Return the nesting level of this loop.  An outer-most loop has depth 1,
   /// for consistency with loop depth values used for basic blocks, where depth
   /// 0 is used for blocks not inside any loops.
   unsigned getLoopDepth() const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     unsigned D = 1;
     for (const LoopT *CurLoop = ParentLoop; CurLoop;
          CurLoop = CurLoop->ParentLoop)
       ++D;
     return D;
   }
-  BlockT *getHeader() const { return Blocks.front(); }
+  BlockT *getHeader() const { return getBlocks().front(); }
   LoopT *getParentLoop() const { return ParentLoop; }
 
   /// This is a raw interface for bypassing addChildLoop.
-  void setParentLoop(LoopT *L) { ParentLoop = L; }
+  void setParentLoop(LoopT *L) {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    ParentLoop = L;
+  }
 
   /// Return true if the specified loop is contained within in this loop.
   bool contains(const LoopT *L) const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     if (L == this)
       return true;
     if (!L)
       return false;
     return contains(L->getParentLoop());
   }
 
   /// Return true if the specified basic block is in this loop.
-  bool contains(const BlockT *BB) const { return DenseBlockSet.count(BB); }
+  bool contains(const BlockT *BB) const {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    return DenseBlockSet.count(BB);
+  }
 
   /// Return true if the specified instruction is in this loop.
   template <class InstT> bool contains(const InstT *Inst) const {
     return contains(Inst->getParent());
   }
 
   /// Return the loops contained entirely within this loop.
-  const std::vector<LoopT *> &getSubLoops() const { return SubLoops; }
-  std::vector<LoopT *> &getSubLoopsVector() { return SubLoops; }
+  const std::vector<LoopT *> &getSubLoops() const {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    return SubLoops;
+  }
+  std::vector<LoopT *> &getSubLoopsVector() {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    return SubLoops;
+  }
   typedef typename std::vector<LoopT *>::const_iterator iterator;
   typedef
       typename std::vector<LoopT *>::const_reverse_iterator reverse_iterator;
-  iterator begin() const { return SubLoops.begin(); }
-  iterator end() const { return SubLoops.end(); }
-  reverse_iterator rbegin() const { return SubLoops.rbegin(); }
-  reverse_iterator rend() const { return SubLoops.rend(); }
-  bool empty() const { return SubLoops.empty(); }
+  iterator begin() const { return getSubLoops().begin(); }
+  iterator end() const { return getSubLoops().end(); }
+  reverse_iterator rbegin() const { return getSubLoops().rbegin(); }
+  reverse_iterator rend() const { return getSubLoops().rend(); }
+  bool empty() const { return getSubLoops().empty(); }
 
   /// Get a list of the basic blocks which make up this loop.
-  const std::vector<BlockT *> &getBlocks() const { return Blocks; }
+  const std::vector<BlockT *> &getBlocks() const {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    return Blocks;
+  }
   typedef typename std::vector<BlockT *>::const_iterator block_iterator;
-  block_iterator block_begin() const { return Blocks.begin(); }
-  block_iterator block_end() const { return Blocks.end(); }
+  block_iterator block_begin() const { return getBlocks().begin(); }
+  block_iterator block_end() const { return getBlocks().end(); }
   inline iterator_range<block_iterator> blocks() const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     return make_range(block_begin(), block_end());
   }
 
   /// Get the number of blocks in this loop in constant time.
-  unsigned getNumBlocks() const { return Blocks.size(); }
-
   /// Invalidate the loop, indicating that it is no longer a loop.
-  void invalidate() { IsInvalid = true; }
+  unsigned getNumBlocks() const {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    return Blocks.size();
+  }
 
   /// Return true if this loop is no longer valid.
-  bool isInvalid() { return IsInvalid; }
+  bool isInvalid() const { return IsInvalid; }
 
   /// True if terminator in the block can branch to another block that is
   /// outside of the current loop.
   bool isLoopExiting(const BlockT *BB) const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     for (const auto &Succ : children<const BlockT *>(BB)) {
       if (!contains(Succ))
         return true;
     }
     return false;
   }
 
   /// Returns true if \p BB is a loop-latch.
   /// A latch block is a block that contains a branch back to the header.
   /// This function is useful when there are multiple latches in a loop
   /// because \fn getLoopLatch will return nullptr in that case.
   bool isLoopLatch(const BlockT *BB) const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     assert(contains(BB) && "block does not belong to the loop");
 
     BlockT *Header = getHeader();
     auto PredBegin = GraphTraits<Inverse<BlockT *>>::child_begin(Header);
     auto PredEnd = GraphTraits<Inverse<BlockT *>>::child_end(Header);
     return std::find(PredBegin, PredEnd, BB) != PredEnd;
   }
 
   /// Calculate the number of back edges to the loop header.
   unsigned getNumBackEdges() const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     unsigned NumBackEdges = 0;
     BlockT *H = getHeader();
 
     for (const auto Pred : children<Inverse<BlockT *>>(H))
       if (contains(Pred))
         ++NumBackEdges;
 
     return NumBackEdges;
public:
 
   /// If there is a single latch block for this loop, return it.
   /// A latch block is a block that contains a branch back to the header.
   BlockT *getLoopLatch() const;
 
   /// Return all loop latch blocks of this loop. A latch block is a block that
   /// contains a branch back to the header.
   void getLoopLatches(SmallVectorImpl<BlockT *> &LoopLatches) const {
+    assert(!isInvalid() && "Loop not in a valid state!");
     BlockT *H = getHeader();
     for (const auto Pred : children<Inverse<BlockT *>>(H))
       if (contains(Pred))
         LoopLatches.push_back(Pred);
   }
 
   //===--------------------------------------------------------------------===//
   // APIs for updating loop information after changing the CFG
public:
   /// our children list with NewChild, and updates the parent pointer of
   /// OldChild to be null and the NewChild to be this loop.
   /// This updates the loop depth of the new child.
   void replaceChildLoopWith(LoopT *OldChild, LoopT *NewChild);
 
   /// Add the specified loop to be a child of this loop.
   /// This updates the loop depth of the new child.
   void addChildLoop(LoopT *NewChild) {
+    assert(!isInvalid() && "Loop not in a valid state!");
     assert(!NewChild->ParentLoop && "NewChild already has a parent!");
     NewChild->ParentLoop = static_cast<LoopT *>(this);
     SubLoops.push_back(NewChild);
   }
 
   /// This removes the specified child from being a subloop of this loop. The
   /// loop is not deleted, as it will presumably be inserted into another loop.
   LoopT *removeChildLoop(iterator I) {
+    assert(!isInvalid() && "Loop not in a valid state!");
     assert(I != SubLoops.end() && "Cannot remove end iterator!");
     LoopT *Child = *I;
     assert(Child->ParentLoop == this && "Child is not a child of this loop!");
     SubLoops.erase(SubLoops.begin() + (I - begin()));
     Child->ParentLoop = nullptr;
     return Child;
   }
 
   /// This adds a basic block directly to the basic block list.
   /// This should only be used by transformations that create new loops.  Other
   /// transformations should use addBasicBlockToLoop.
   void addBlockEntry(BlockT *BB) {
+    assert(!isInvalid() && "Loop not in a valid state!");
     Blocks.push_back(BB);
     DenseBlockSet.insert(BB);
   }
 
   /// interface to reverse Blocks[from, end of loop] in this loop
   void reverseBlock(unsigned from) {
+    assert(!isInvalid() && "Loop not in a valid state!");
     std::reverse(Blocks.begin() + from, Blocks.end());
   }
 
   /// interface to do reserve() for Blocks
-  void reserveBlocks(unsigned size) { Blocks.reserve(size); }
+  void reserveBlocks(unsigned size) {
+    assert(!isInvalid() && "Loop not in a valid state!");
+    Blocks.reserve(size);
+  }
 
   /// This method is used to move BB (which must be part of this loop) to be the
   /// loop header of the loop (the block that dominates all others).
   void moveToHeader(BlockT *BB) {
+    assert(!isInvalid() && "Loop not in a valid state!");
     if (Blocks[0] == BB)
       return;
     for (unsigned i = 0;; ++i) {
       assert(i != Blocks.size() && "Loop does not contain BB!");
       if (Blocks[i] == BB) {
         Blocks[i] = Blocks[0];
         Blocks[0] = BB;
         return;
       }
     }
   }
 
   /// This removes the specified basic block from the current loop, updating the
   /// Blocks as appropriate. This does not update the mapping in the LoopInfo
   /// class.
   void removeBlockFromLoop(BlockT *BB) {
+    assert(!isInvalid() && "Loop not in a valid state!");
     auto I = find(Blocks, BB);
     assert(I != Blocks.end() && "N is not in this list!");
     Blocks.erase(I);
 
     DenseBlockSet.erase(BB);
   }
 
   /// Verify loop structure
public:
   /// cannot find a terminating instruction with location information,
   /// it returns an unknown location.
   DebugLoc getStartLoc() const;
 
   /// Return the source code span of the loop.
   LocRange getLocRange() const;
 
   StringRef getName() const {
+    if (isInvalid())
+      return "<invalidated loop>";
     if (BasicBlock *Header = getHeader())
       if (Header->hasName())
         return Header->getName();
     return "<unnamed loop>";
   }
 
 private:
   friend class LoopInfoBase<BasicBlock, Loop>;
public:
 
   /// Create the loop forest using a stable algorithm.
   void analyze(const DominatorTreeBase<BlockT, false> &DomTree);
 
   // Debugging
   void print(raw_ostream &OS) const;
 
   void verify(const DominatorTreeBase<BlockT, false> &DomTree) const;
+
+protected:
+  static void clearLoop(LoopT &L) { L.clear(); }
 };
 
 // Implementation in LoopInfoImpl.h
 extern template class LoopInfoBase<BasicBlock, Loop>;
 
 class LoopInfo : public LoopInfoBase<BasicBlock, Loop> {
   typedef LoopInfoBase<BasicBlock, Loop> BaseT;
 

llvm/trunk/include/llvm/Analysis/LoopInfoImpl.h

 
 /// getExitingBlocks - Return all blocks inside the loop that have successors
 /// outside of the loop.  These are the blocks _inside of the current loop_
 /// which branch out.  The returned list is always unique.
 ///
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::getExitingBlocks(
     SmallVectorImpl<BlockT *> &ExitingBlocks) const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   for (const auto BB : blocks())
     for (const auto &Succ : children<BlockT *>(BB))
       if (!contains(Succ)) {
         // Not in current loop? It must be an exit block.
         ExitingBlocks.push_back(BB);
         break;
       }
 }
 
 /// getExitingBlock - If getExitingBlocks would return exactly one block,
 /// return that block. Otherwise return null.
 template <class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getExitingBlock() const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   SmallVector<BlockT *, 8> ExitingBlocks;
   getExitingBlocks(ExitingBlocks);
   if (ExitingBlocks.size() == 1)
     return ExitingBlocks[0];
   return nullptr;
 }
 
 /// getExitBlocks - Return all of the successor blocks of this loop.  These
 /// are the blocks _outside of the current loop_ which are branched to.
 ///
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::getExitBlocks(
     SmallVectorImpl<BlockT *> &ExitBlocks) const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   for (const auto BB : blocks())
     for (const auto &Succ : children<BlockT *>(BB))
       if (!contains(Succ))
         // Not in current loop? It must be an exit block.
         ExitBlocks.push_back(Succ);
 }
 
 /// getExitBlock - If getExitBlocks would return exactly one block,
 /// return that block. Otherwise return null.
 template <class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getExitBlock() const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   SmallVector<BlockT *, 8> ExitBlocks;
   getExitBlocks(ExitBlocks);
   if (ExitBlocks.size() == 1)
     return ExitBlocks[0];
   return nullptr;
 }
 
 /// getExitEdges - Return all pairs of (_inside_block_,_outside_block_).
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::getExitEdges(
     SmallVectorImpl<Edge> &ExitEdges) const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   for (const auto BB : blocks())
     for (const auto &Succ : children<BlockT *>(BB))
       if (!contains(Succ))
         // Not in current loop? It must be an exit block.
         ExitEdges.emplace_back(BB, Succ);
 }
 
 /// getLoopPreheader - If there is a preheader for this loop, return it.  A
 /// loop has a preheader if there is only one edge to the header of the loop
 /// from outside of the loop and it is legal to hoist instructions into the
 /// predecessor. If this is the case, the block branching to the header of the
 /// loop is the preheader node.
 ///
 /// This method returns null if there is no preheader for the loop.
 ///
 template <class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopPreheader() const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   // Keep track of nodes outside the loop branching to the header...
   BlockT *Out = getLoopPredecessor();
   if (!Out)
     return nullptr;
 
   // Make sure we are allowed to hoist instructions into the predecessor.
   if (!Out->isLegalToHoistInto())
     return nullptr;

 
 /// getLoopPredecessor - If the given loop's header has exactly one unique
 /// predecessor outside the loop, return it. Otherwise return null.
 /// This is less strict that the loop "preheader" concept, which requires
 /// the predecessor to have exactly one successor.
 ///
 template <class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopPredecessor() const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   // Keep track of nodes outside the loop branching to the header...
   BlockT *Out = nullptr;
 
   // Loop over the predecessors of the header node...
   BlockT *Header = getHeader();
   for (const auto Pred : children<Inverse<BlockT *>>(Header)) {
     if (!contains(Pred)) { // If the block is not in the loop...
       if (Out && Out != Pred)
         return nullptr; // Multiple predecessors outside the loop
       Out = Pred;
     }
   }
 
   // Make sure there is only one exit out of the preheader.
   assert(Out && "Header of loop has no predecessors from outside loop?");
   return Out;
 }
 
 /// getLoopLatch - If there is a single latch block for this loop, return it.
 /// A latch block is a block that contains a branch back to the header.
 template <class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopLatch() const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   BlockT *Header = getHeader();
   BlockT *Latch = nullptr;
   for (const auto Pred : children<Inverse<BlockT *>>(Header)) {
     if (contains(Pred)) {
       if (Latch)
         return nullptr;
       Latch = Pred;
     }

 /// information.  NewBB is set to be a new member of the current loop.
 /// Because of this, it is added as a member of all parent loops, and is added
 /// to the specified LoopInfo object as being in the current basic block.  It
 /// is not valid to replace the loop header with this method.
 ///
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::addBasicBlockToLoop(
     BlockT *NewBB, LoopInfoBase<BlockT, LoopT> &LIB) {
+  assert(!isInvalid() && "Loop not in a valid state!");
 #ifndef NDEBUG
   if (!Blocks.empty()) {
     auto SameHeader = LIB[getHeader()];
     assert(contains(SameHeader) && getHeader() == SameHeader->getHeader() &&
            "Incorrect LI specified for this loop!");
   }
 #endif
   assert(NewBB && "Cannot add a null basic block to the loop!");

 
 /// replaceChildLoopWith - This is used when splitting loops up.  It replaces
 /// the OldChild entry in our children list with NewChild, and updates the
 /// parent pointer of OldChild to be null and the NewChild to be this loop.
 /// This updates the loop depth of the new child.
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::replaceChildLoopWith(LoopT *OldChild,
                                                    LoopT *NewChild) {
+  assert(!isInvalid() && "Loop not in a valid state!");
   assert(OldChild->ParentLoop == this && "This loop is already broken!");
   assert(!NewChild->ParentLoop && "NewChild already has a parent!");
   typename std::vector<LoopT *>::iterator I = find(SubLoops, OldChild);
   assert(I != SubLoops.end() && "OldChild not in loop!");
   *I = NewChild;
   OldChild->ParentLoop = nullptr;
   NewChild->ParentLoop = static_cast<LoopT *>(this);
 }
 
 /// verifyLoop - Verify loop structure
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::verifyLoop() const {
+  assert(!isInvalid() && "Loop not in a valid state!");
 #ifndef NDEBUG
   assert(!Blocks.empty() && "Loop header is missing");
 
   // Setup for using a depth-first iterator to visit every block in the loop.
   SmallVector<BlockT *, 8> ExitBBs;
   getExitBlocks(ExitBBs);
   df_iterator_default_set<BlockT *> VisitSet;
   VisitSet.insert(ExitBBs.begin(), ExitBBs.end());
#ifndef NDEBUG
   }
 #endif
 }
 
 /// verifyLoop - Verify loop structure of this loop and all nested loops.
 template <class BlockT, class LoopT>
 void LoopBase<BlockT, LoopT>::verifyLoopNest(
     DenseSet<const LoopT *> *Loops) const {
+  assert(!isInvalid() && "Loop not in a valid state!");
   Loops->insert(static_cast<const LoopT *>(this));
   // Verify this loop.
   verifyLoop();
   // Verify the subloops.
   for (iterator I = begin(), E = end(); I != E; ++I)
     (*I)->verifyLoopNest(Loops);
 }
 

llvm/trunk/include/llvm/Transforms/Scalar/LoopPassManager.h

public:
   /// current loop. This routine must be called prior to removing the loop from
   /// the loop nest.
   ///
   /// If this is called for the current loop, in addition to clearing any
   /// state, this routine will mark that the current loop should be skipped by
   /// the rest of the pass management infrastructure.
   void markLoopAsDeleted(Loop &L) {
     LAM.clear(L);
-    assert(CurrentL->contains(&L) && "Cannot delete a loop outside of the "
+    assert(&L == CurrentL ||
+           CurrentL->contains(&L) && "Cannot delete a loop outside of the "
                                      "subloop tree currently being processed.");
     if (&L == CurrentL)
       SkipCurrentLoop = true;
   }
 
   /// Loop passes should use this method to indicate they have added new child
   /// loops of the current loop.
   ///

llvm/trunk/include/llvm/Transforms/Utils/UnrollLoop.h

 class ScalarEvolution;
 
 typedef SmallDenseMap<const Loop *, Loop *, 4> NewLoopsMap;
 
 const Loop* addClonedBlockToLoopInfo(BasicBlock *OriginalBB,
                                      BasicBlock *ClonedBB, LoopInfo *LI,
                                      NewLoopsMap &NewLoops);
 
-bool UnrollLoop(Loop *L, unsigned Count, unsigned TripCount, bool Force,
-                bool AllowRuntime, bool AllowExpensiveTripCount,
-                bool PreserveCondBr, bool PreserveOnlyFirst,
-                unsigned TripMultiple, unsigned PeelCount, bool UnrollRemainder,
-                LoopInfo *LI, ScalarEvolution *SE, DominatorTree *DT,
-                AssumptionCache *AC, OptimizationRemarkEmitter *ORE,
-                bool PreserveLCSSA);
+/// Represents the result of a \c UnrollLoop invocation.
+enum class LoopUnrollStatus {
+  /// The loop was not modified.
+  Unmodified,
+
+  /// The loop was partially unrolled -- we still have a loop, but with a
+  /// smaller trip count.  We may also have emitted epilogue loop if the loop
+  /// had a non-constant trip count.
+  PartiallyUnrolled,
+
+  /// The loop was fully unrolled into straight-line code.  We no longer have
+  /// any back-edges.
+  FullyUnrolled
+};
+
+LoopUnrollStatus UnrollLoop(Loop *L, unsigned Count, unsigned TripCount,
+                            bool Force, bool AllowRuntime,
+                            bool AllowExpensiveTripCount, bool PreserveCondBr,
+                            bool PreserveOnlyFirst, unsigned TripMultiple,
+                            unsigned PeelCount, bool UnrollRemainder,
+                            LoopInfo *LI, ScalarEvolution *SE,
+                            DominatorTree *DT, AssumptionCache *AC,
+                            OptimizationRemarkEmitter *ORE, bool PreserveLCSSA);
 
 bool UnrollRuntimeLoopRemainder(Loop *L, unsigned Count,
                                 bool AllowExpensiveTripCount,
                                 bool UseEpilogRemainder, bool UnrollRemainder,
                                 LoopInfo *LI,
                                 ScalarEvolution *SE, DominatorTree *DT,
                                 AssumptionCache *AC,
                                 OptimizationRemarkEmitter *ORE,

llvm/trunk/lib/Analysis/LoopInfo.cpp

 // and determine the loop depth of various nodes of the CFG.  Note that the
 // loops identified may actually be several natural loops that share the same
 // header node... not just a single natural loop.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Analysis/LoopInfoImpl.h"
 #include "llvm/Analysis/LoopIterator.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/Dominators.h"
bool LoopInfo::invalidate(Function &F, const PreservedAnalyses &PA,
   // CFG have been preserved.
   auto PAC = PA.getChecker<LoopAnalysis>();
   return !(PAC.preserved() || PAC.preservedSet<AllAnalysesOn<Function>>() ||
            PAC.preservedSet<CFGAnalyses>());
 }
 
 void LoopInfo::markAsErased(Loop *Unloop) {
   assert(!Unloop->isInvalid() && "Loop has already been erased!");
-  Unloop->invalidate();
   RemovedLoops.push_back(Unloop);
 
+  auto InvalidateOnExit =
+      make_scope_exit([&]() { BaseT::clearLoop(*Unloop); });
+
   // First handle the special case of no parent loop to simplify the algorithm.
   if (!Unloop->getParentLoop()) {
     // Since BBLoop had no parent, Unloop blocks are no longer in a loop.
     for (Loop::block_iterator I = Unloop->block_begin(),
                               E = Unloop->block_end();
          I != E; ++I) {
 
       // Don't reparent blocks in subloops.

llvm/trunk/lib/Analysis/LoopPass.cpp

for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
         PassManagerPrettyStackEntry X(P, *CurrentLoop->getHeader());
         TimeRegion PassTimer(getPassTimer(P));
 
         Changed |= P->runOnLoop(CurrentLoop, *this);
       }
       LoopWasDeleted = CurrentLoop->isInvalid();
 
       if (Changed)
-        dumpPassInfo(P, MODIFICATION_MSG, ON_LOOP_MSG,
-                     LoopWasDeleted ? "<deleted>"
-                                    : CurrentLoop->getHeader()->getName());
+        dumpPassInfo(P, MODIFICATION_MSG, ON_LOOP_MSG, CurrentLoop->getName());
       dumpPreservedSet(P);
 
       if (LoopWasDeleted) {
         // Notify passes that the loop is being deleted.
         deleteSimpleAnalysisLoop(CurrentLoop);
       } else {
         // Manually check that this loop is still healthy. This is done
         // instead of relying on LoopInfo::verifyLoop since LoopInfo

llvm/trunk/lib/Transforms/Scalar/LoopUnrollPass.cpp

bool IsCountSetExplicitly =
                          TripMultiple, LoopSize, UP, UseUpperBound);
   if (!UP.Count)
     return false;
   // Unroll factor (Count) must be less or equal to TripCount.
   if (TripCount && UP.Count > TripCount)
     UP.Count = TripCount;
 
   // Unroll the loop.
-  if (!UnrollLoop(L, UP.Count, TripCount, UP.Force, UP.Runtime,
-                  UP.AllowExpensiveTripCount, UseUpperBound, MaxOrZero,
-                  TripMultiple, UP.PeelCount, UP.UnrollRemainder,
-                  LI, &SE, &DT, &AC, &ORE,
-                  PreserveLCSSA))
+  LoopUnrollStatus UnrollStatus = UnrollLoop(
+      L, UP.Count, TripCount, UP.Force, UP.Runtime, UP.AllowExpensiveTripCount,
+      UseUpperBound, MaxOrZero, TripMultiple, UP.PeelCount, UP.UnrollRemainder,
+      LI, &SE, &DT, &AC, &ORE, PreserveLCSSA);
+  if (UnrollStatus == LoopUnrollStatus::Unmodified)
     return false;
 
   // If loop has an unroll count pragma or unrolled by explicitly set count
   // mark loop as unrolled to prevent unrolling beyond that requested.
   // If the loop was peeled, we already "used up" the profile information
   // we had, so we don't want to unroll or peel again.
-  if (IsCountSetExplicitly || UP.PeelCount)
+  if (UnrollStatus != LoopUnrollStatus::FullyUnrolled &&
+      (IsCountSetExplicitly || UP.PeelCount))
     SetLoopAlreadyUnrolled(L);
 
   return true;
 }
 
 namespace {
 class LoopUnroll : public LoopPass {
 public:

llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp

for (Instruction &BBI : *Header) {
     if (!PN)
       break;
     if (isa<ConstantInt>(PN->getIncomingValueForBlock(PreHeader)))
       return true;
   }
   return false;
 }
 
-/// Unroll the given loop by Count. The loop must be in LCSSA form. Returns true
-/// if unrolling was successful, or false if the loop was unmodified. Unrolling
+/// Unroll the given loop by Count. The loop must be in LCSSA form.  Unrolling
 /// can only fail when the loop's latch block is not terminated by a conditional
 /// branch instruction. However, if the trip count (and multiple) are not known,
 /// loop unrolling will mostly produce more code that is no faster.
 ///
 /// TripCount is the upper bound of the iteration on which control exits
 /// LatchBlock. Control may exit the loop prior to TripCount iterations either
 /// via an early branch in other loop block or via LatchBlock terminator. This
 /// is relaxed from the general definition of trip count which is the number of

 ///
 /// If AllowRuntime is true then UnrollLoop will consider unrolling loops that
 /// have a runtime (i.e. not compile time constant) trip count.  Unrolling these
 /// loops require a unroll "prologue" that runs "RuntimeTripCount % Count"
 /// iterations before branching into the unrolled loop.  UnrollLoop will not
 /// runtime-unroll the loop if computing RuntimeTripCount will be expensive and
 /// AllowExpensiveTripCount is false.
 ///
-/// If we want to perform PGO-based loop peeling, PeelCount is set to the 
+/// If we want to perform PGO-based loop peeling, PeelCount is set to the
 /// number of iterations we want to peel off.
 ///
 /// The LoopInfo Analysis that is passed will be kept consistent.
 ///
 /// This utility preserves LoopInfo. It will also preserve ScalarEvolution and
 /// DominatorTree if they are non-null.
-bool llvm::UnrollLoop(Loop *L, unsigned Count, unsigned TripCount, bool Force,
-                      bool AllowRuntime, bool AllowExpensiveTripCount,
-                      bool PreserveCondBr, bool PreserveOnlyFirst,
-                      unsigned TripMultiple, unsigned PeelCount,
-                      bool UnrollRemainder, LoopInfo *LI,
-                      ScalarEvolution *SE, DominatorTree *DT,
-                      AssumptionCache *AC, OptimizationRemarkEmitter *ORE,
-                      bool PreserveLCSSA) {
+LoopUnrollStatus llvm::UnrollLoop(
+    Loop *L, unsigned Count, unsigned TripCount, bool Force, bool AllowRuntime,
+    bool AllowExpensiveTripCount, bool PreserveCondBr, bool PreserveOnlyFirst,
+    unsigned TripMultiple, unsigned PeelCount, bool UnrollRemainder,
+    LoopInfo *LI, ScalarEvolution *SE, DominatorTree *DT, AssumptionCache *AC,
+    OptimizationRemarkEmitter *ORE, bool PreserveLCSSA) {
 
   BasicBlock *Preheader = L->getLoopPreheader();
   if (!Preheader) {
     DEBUG(dbgs() << "  Can't unroll; loop preheader-insertion failed.\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   BasicBlock *LatchBlock = L->getLoopLatch();
   if (!LatchBlock) {
     DEBUG(dbgs() << "  Can't unroll; loop exit-block-insertion failed.\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   // Loops with indirectbr cannot be cloned.
   if (!L->isSafeToClone()) {
     DEBUG(dbgs() << "  Can't unroll; Loop body cannot be cloned.\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   // The current loop unroll pass can only unroll loops with a single latch
   // that's a conditional branch exiting the loop.
   // FIXME: The implementation can be extended to work with more complicated
   // cases, e.g. loops with multiple latches.
   BasicBlock *Header = L->getHeader();
   BranchInst *BI = dyn_cast<BranchInst>(LatchBlock->getTerminator());
 
   if (!BI || BI->isUnconditional()) {
     // The loop-rotate pass can be helpful to avoid this in many cases.
     DEBUG(dbgs() <<
              "  Can't unroll; loop not terminated by a conditional branch.\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   auto CheckSuccessors = [&](unsigned S1, unsigned S2) {
     return BI->getSuccessor(S1) == Header && !L->contains(BI->getSuccessor(S2));
   };
 
   if (!CheckSuccessors(0, 1) && !CheckSuccessors(1, 0)) {
     DEBUG(dbgs() << "Can't unroll; only loops with one conditional latch"
                     " exiting the loop can be unrolled\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   if (Header->hasAddressTaken()) {
     // The loop-rotate pass can be helpful to avoid this in many cases.
     DEBUG(dbgs() <<
           "  Won't unroll loop: address of header block is taken.\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   if (TripCount != 0)
     DEBUG(dbgs() << "  Trip Count = " << TripCount << "\n");
   if (TripMultiple != 1)
     DEBUG(dbgs() << "  Trip Multiple = " << TripMultiple << "\n");
 
   // Effectively "DCE" unrolled iterations that are beyond the tripcount
   // and will never be executed.
   if (TripCount != 0 && Count > TripCount)
     Count = TripCount;
 
   // Don't enter the unroll code if there is nothing to do.
   if (TripCount == 0 && Count < 2 && PeelCount == 0) {
     DEBUG(dbgs() << "Won't unroll; almost nothing to do\n");
-    return false;
+    return LoopUnrollStatus::Unmodified;
   }
 
   assert(Count > 0);
   assert(TripMultiple > 0);
   assert(TripCount == 0 || TripCount % TripMultiple == 0);
 
   // Are we eliminating the loop control altogether?
   bool CompletelyUnroll = Count == TripCount;
if (RuntimeTripCount && TripMultiple % Count != 0 &&
                                   LI, SE, DT, AC, ORE,
                                   PreserveLCSSA)) {
     if (Force)
       RuntimeTripCount = false;
     else {
       DEBUG(
           dbgs() << "Wont unroll; remainder loop could not be generated"
                     "when assuming runtime trip count\n");
-      return false;
+      return LoopUnrollStatus::Unmodified;
     }
   }
 
   // Notify ScalarEvolution that the loop will be substantially changed,
   // if not outright eliminated.
   if (SE)
     SE->forgetLoop(L);
 
if (OuterL) {
       simplifyLoop(OuterL, DT, LI, SE, AC, PreserveLCSSA);
     } else {
       // Simplify loops for which we might've broken loop-simplify form.
       for (Loop *SubLoop : LoopsToSimplify)
         simplifyLoop(SubLoop, DT, LI, SE, AC, PreserveLCSSA);
     }
   }
 
-  return true;
+  return CompletelyUnroll ? LoopUnrollStatus::FullyUnrolled
+                          : LoopUnrollStatus::PartiallyUnrolled;
 }
 
 /// Given an llvm.loop loop id metadata node, returns the loop hint metadata
 /// node with the given name (for example, "llvm.loop.unroll.count"). If no
 /// such metadata node exists, then nullptr is returned.
 MDNode *llvm::GetUnrollMetadata(MDNode *LoopID, StringRef Name) {
   // First operand should refer to the loop id itself.
   assert(LoopID->getNumOperands() > 0 && "requires at least one operand");

llvm/trunk/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll

 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
 ; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
 ; CHECK: Finished Loop pass manager run.
 ; CHECK: Starting Loop pass manager run.
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
 ; CHECK: Running analysis: LoopAccessAnalysis on outer.header
 ; CHECK: Finished Loop pass manager run.
 ; CHECK: Running pass: LoopUnrollPass
-; CHECK: Clearing all analysis results for: inner2.header
-; CHECK: Clearing all analysis results for: outer.header
+; CHECK: Clearing all analysis results for: <invalidated loop>
+; CHECK: Clearing all analysis results for: <invalidated loop>
 ; CHECK: Invalidating all non-preserved analyses for: test
 ; CHECK: Invalidating all non-preserved analyses for: inner1.header
 ; CHECK: Invalidating analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Invalidating all non-preserved analyses for: inner1.header.1
 ; CHECK-NOT: Invalidating analysis: LoopAccessAnalysis on inner1.header.1
 ; CHECK: Running pass: FunctionToLoopPassAdaptor
 ; CHECK: Starting Loop pass manager run.
 ; CHECK: Running pass: LoopAccessInfoPrinterPass