[LegacyPM] Make the 'addLoop' method accept a loop to add rather than having it internally allocate the loop.

[chandlerc]

Summary

This is a much more flexible API and necessary in the new loop unswitch
to reasonably support both new and old PMs in common code. It also just
seems like a cleaner separation of concerns.

NFC, this should just be a pure refactoring.

[davide]

This is fine assuming you audited all the callers (a quick grep shows just these two).

lib/Analysis/LoopPass.cpp

76 ↗

(On Diff #100189)

this should be insert, I think, but it's not your fault.

[chandlerc]

Thanks, landing!

In D33528#763997, @davide wrote:

This is fine assuming you audited all the callers (a quick grep shows just these two).

Yeah, my grep concludes the same.

lib/Analysis/LoopPass.cpp

76 ↗

(On Diff #100189)

I'll fix the typo while here.

llvm/trunk/include/llvm/Analysis/LoopPass.h

LoopPass *getContainedPass(unsigned N) {
     return LP;
   }
 
   PassManagerType getPassManagerType() const override {
     return PMT_LoopPassManager;
   }
 
 public:
-  // Add a new loop into the loop queue as a child of the given parent, or at
-  // the top level if \c ParentLoop is null.
-  Loop &addLoop(Loop *ParentLoop);
+  // Add a new loop into the loop queue.
+  void addLoop(Loop &L);
 
   //===--------------------------------------------------------------------===//
   /// SimpleAnalysis - Provides simple interface to update analysis info
   /// maintained by various passes. Note, if required this interface can
   /// be extracted into a separate abstract class but it would require
   /// additional use of multiple inheritance in Pass class hierarchy, something
   /// we are trying to avoid.
 

llvm/trunk/lib/Analysis/LoopPass.cpp

 char LPPassManager::ID = 0;
 
 LPPassManager::LPPassManager()
   : FunctionPass(ID), PMDataManager() {
   LI = nullptr;
   CurrentLoop = nullptr;
 }
 
-// Inset loop into loop nest (LoopInfo) and loop queue (LQ).
-Loop &LPPassManager::addLoop(Loop *ParentLoop) {
-  // Create a new loop. LI will take ownership.
-  Loop *L = new Loop();
-
-  // Insert into the loop nest and the loop queue.
-  if (!ParentLoop) {
+// Insert loop into loop nest (LoopInfo) and loop queue (LQ).
+void LPPassManager::addLoop(Loop &L) {
+  if (!L.getParentLoop()) {
     // This is the top level loop.
-    LI->addTopLevelLoop(L);
-    LQ.push_front(L);
-    return *L;
+    LQ.push_front(&L);
+    return;
   }
 
-  ParentLoop->addChildLoop(L);
   // Insert L into the loop queue after the parent loop.
   for (auto I = LQ.begin(), E = LQ.end(); I != E; ++I) {
-    if (*I == L->getParentLoop()) {
+    if (*I == L.getParentLoop()) {
       // deque does not support insert after.
       ++I;
-      LQ.insert(I, 1, L);
-      break;
+      LQ.insert(I, 1, &L);
+      return;
     }
   }
-  return *L;
 }
 
 /// cloneBasicBlockSimpleAnalysis - Invoke cloneBasicBlockAnalysis hook for
 /// all loop passes.
 void LPPassManager::cloneBasicBlockSimpleAnalysis(BasicBlock *From,
                                                   BasicBlock *To, Loop *L) {
   for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
     LoopPass *LP = getContainedPass(Index);

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

if (!ParentLoop)
     return;
 
   for (BasicBlock *BB : BBs)
     ParentLoop->addBasicBlockToLoop(BB, LI);
 }
 
 Loop *LoopConstrainer::createClonedLoopStructure(Loop *Original, Loop *Parent,
                                                  ValueToValueMapTy &VM) {
-  Loop &New = LPM.addLoop(Parent);
+  Loop &New = *new Loop();
+  if (Parent)
+    Parent->addChildLoop(&New);
+  else
+    LI.addTopLevelLoop(&New);
+  LPM.addLoop(New);
 
   // Add all of the blocks in Original to the new loop.
   for (auto *BB : Original->blocks())
     if (LI.getLoopFor(BB) == Original)
       New.addBasicBlockToLoop(cast<BasicBlock>(VM[BB]), LI);
 
   // Add all of the subloops to the new loop.
   for (Loop *SubLoop : *Original)

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

bool LoopUnswitch::UnswitchIfProfitable(Value *LoopCond, Constant *Val,
   UnswitchNontrivialCondition(LoopCond, Val, currentLoop, TI);
   return true;
 }
 
 /// Recursively clone the specified loop and all of its children,
 /// mapping the blocks with the specified map.
 static Loop *CloneLoop(Loop *L, Loop *PL, ValueToValueMapTy &VM,
                        LoopInfo *LI, LPPassManager *LPM) {
-  Loop &New = LPM->addLoop(PL);
+  Loop &New = *new Loop();
+  if (PL)
+    PL->addChildLoop(&New);
+  else
+    LI->addTopLevelLoop(&New);
+  LPM->addLoop(New);
 
   // Add all of the blocks in L to the new loop.
   for (Loop::block_iterator I = L->block_begin(), E = L->block_end();
        I != E; ++I)
     if (LI->getLoopFor(*I) == L)
       New.addBasicBlockToLoop(cast<BasicBlock>(VM[*I]), *LI);
 
   // Add all of the subloops to the new loop.