Refactor collectChildrenInLoop to LoopUtils [NFC]

[asbirlea]

Summary

Move to LoopUtils method that collects all children of a node inside a loop.

[sanjoy]

LGTM

I added a couple of stylistic issues that were present in the earlier function. I'll leave it up to you if you want to fix these.

lib/Transforms/Utils/LoopUtils.cpp

1124

LLVM style is AddRegionToWorklist.

1133

For integer loop counters, I've usually seem LLVM use i instead of I.

1135

This can probably be for (DomTreeNode *Child : Worklist[i]), at which point the { braces won't be needed either.

[asbirlea]

Address comments.

[asbirlea]

lib/Transforms/Utils/LoopUtils.cpp

1133

That's true. I've seen both. Keeping I per the variable style guide.

include/llvm/Transforms/Utils/LoopUtils.h

 bool promoteLoopAccessesToScalars(const SmallSetVector<Value *, 8> &,
                                   SmallVectorImpl<BasicBlock *> &,
                                   SmallVectorImpl<Instruction *> &,
                                   PredIteratorCache &, LoopInfo *,
                                   DominatorTree *, const TargetLibraryInfo *,
                                   Loop *, AliasSetTracker *, LoopSafetyInfo *,
                                   OptimizationRemarkEmitter *);
 
+/// Does a BFS from a given node to all of its children inside a given loop.
+/// The returned vector of nodes includes the starting point.
+SmallVector<DomTreeNode *, 16> collectChildrenInLoop(DomTreeNode *N,
+                                                     const Loop *CurLoop);
+
 /// \brief Computes safety information for a loop
 /// checks loop body & header for the possibility of may throw
 /// exception, it takes LoopSafetyInfo and loop as argument.
 /// Updates safety information in LoopSafetyInfo argument.
 void computeLoopSafetyInfo(LoopSafetyInfo *, Loop *);
 
 /// Returns true if the instruction in a loop is guaranteed to execute at least
 /// once.

lib/Transforms/Scalar/LICM.cpp

bool LoopInvariantCodeMotion::runOnLoop(Loop *L, AliasAnalysis *AA,
   else
     delete CurAST;
 
   if (Changed && SE)
     SE->forgetLoopDispositions(L);
   return Changed;
 }
 
-// Does a BFS from a given node to all of its children inside a given loop.
-// The returned vector of nodes includes the starting point.
-static SmallVector<DomTreeNode *, 16>
-collectChildrenInLoop(DomTreeNode *N, const Loop *CurLoop) {
-  SmallVector<DomTreeNode *, 16> Worklist;
-  auto add_region_to_worklist = [&](DomTreeNode *DTN) {
-    // Only include subregions in the top level loop.
-    BasicBlock *BB = DTN->getBlock();
-    if (CurLoop->contains(BB))
-      Worklist.push_back(DTN);
-  };
-
-  add_region_to_worklist(N);
-
-  for (size_t I = 0; I < Worklist.size(); I++) {
-    DomTreeNode *DTN = Worklist[I];
-    for (DomTreeNode *Child : DTN->getChildren())
-      add_region_to_worklist(Child);
-  }
-
-  return Worklist;
-}
-
 /// Walk the specified region of the CFG (defined by all blocks dominated by
 /// the specified block, and that are in the current loop) in reverse depth
 /// first order w.r.t the DominatorTree.  This allows us to visit uses before
 /// definitions, allowing us to sink a loop body in one pass without iteration.
 ///
 bool llvm::sinkRegion(DomTreeNode *N, AliasAnalysis *AA, LoopInfo *LI,
                       DominatorTree *DT, TargetLibraryInfo *TLI, Loop *CurLoop,
                       AliasSetTracker *CurAST, LoopSafetyInfo *SafetyInfo,

lib/Transforms/Utils/LoopUtils.cpp

if (Name.equals(S->getString()))
         return &MD->getOperand(1);
       default:
         llvm_unreachable("loop metadata has 0 or 1 operand");
       }
   }
   return None;
 }
 
+/// Does a BFS from a given node to all of its children inside a given loop.
+/// The returned vector of nodes includes the starting point.
+SmallVector<DomTreeNode *, 16>
+llvm::collectChildrenInLoop(DomTreeNode *N, const Loop *CurLoop) {
+  SmallVector<DomTreeNode *, 16> Worklist;
+  auto AddRegionToWorklist = [&](DomTreeNode *DTN) {
+    // Only include subregions in the top level loop.
+    BasicBlock *BB = DTN->getBlock();
+    if (CurLoop->contains(BB))
+      Worklist.push_back(DTN);
+  };
+
+  AddRegionToWorklist(N);
+
+  for (size_t I = 0; I < Worklist.size(); I++)
+    for (DomTreeNode *Child : Worklist[I]->getChildren())
+      AddRegionToWorklist(Child);
+
+  return Worklist;
+}
+
 /// Returns true if the instruction in a loop is guaranteed to execute at least
 /// once.
 bool llvm::isGuaranteedToExecute(const Instruction &Inst,
                                  const DominatorTree *DT, const Loop *CurLoop,
                                  const LoopSafetyInfo *SafetyInfo) {
   // We have to check to make sure that the instruction dominates all
   // of the exit blocks.  If it doesn't, then there is a path out of the loop
   // which does not execute this instruction, so we can't hoist it.