Tidy up getFirstNonBoxedLoopFor [NFC]

[sabuasal]

Summary

Move the function getFirstNonBoxedLoopFor which is used in ScopBuilder and in ScopInfo to Support/ScopHelpers to make it reusable
in other locations. No functionality change.

[sabuasal]

Move the function getFirstNonBoxedLoopFor which is used in
ScopBuilder and in ScopInfo to Support/ScopHelpers to make it reusable
in other locations.

No functionality change.

[grosser]

Dear Sameer,

thank you for the patch. Cleanups are always welcome. Some minore nitpicks, but otherwise this patch is good to go. Feel free to commit!

include/polly/Support/ScopHelper.h

440

What is this comment about? It seems to be leftover.

443

Also not needed?

448

The "Do not call ..." does not seem to make a lot of sense, no? It seems to be an outdated comment. Can you just drop it as part of this cleanup?

lib/Support/ScopHelper.cpp

586

Nice Cleanup.

A minor nitpick. I would prefer 'Loop *L' over 'auto L*' (even though the original code said 'auto *L). This is more in line with the LLVM coding standards [1], which suggest to only use auto if it makes the code clearly more readable.

[1] http://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

[sabuasal]

Fixed comments by Tobi.

[sabuasal]

@efriedma

Can you commit this on my behalf, I don't have commit rights.

[Meinersbur]

An overhaul of the boxed loop / surrounded loop mechanism could be useful. There's also BlockGenerator::getLoopForStmt with the same functionality. We don't even need to remember the boxed loops if we know the surrounding loop as everything inside will be boxed loops. The surrounding loop is either the top entry of LoopNest, or the loop the whole Scop is enclosed in, if any.

include/polly/Support/ScopHelper.h

 /// size available for the outermost array dimension.
 ///
 /// @param GEP The GetElementPtr instruction to analyze.
 ///
 /// @return A tuple with the subscript expressions and the dimension sizes.
 std::tuple<std::vector<const llvm::SCEV *>, std::vector<int>>
 getIndexExpressionsFromGEP(llvm::GetElementPtrInst *GEP,
                            llvm::ScalarEvolution &SE);
+
+// If the loop is nonaffine/boxed, return the first non-boxed surrounding loop
+// for Polly. If the loop is affine, return the loop itself.
+//
+// @param L             Pointer to the Loop object to analyze.
+// @param LI            Reference to the LoopInfo.
+// @param Boxed Loops   Set of Boxed Loops we get from the SCoP.
+llvm::Loop *getFirstNonBoxedLoopFor(llvm::Loop *L, llvm::LoopInfo &LI,
+                                    const BoxedLoopsSetTy &BoxedLoops);
+
+// If the Basic Block belongs to a loop that is nonaffine/boxed, return the
+// first non-boxed surrounding loop for Polly. If the loop is affine, return
+// the loop itself.
+//
+// // @param BB            Pointer to the Basic Block to analyze.
+// @param LI            Reference to the LoopInfo.
+// @param Boxed Loops   Set of Boxed Loops we get from the SCoP.
+llvm::Loop *getFirstNonBoxedLoopFor(llvm::BasicBlock *BB, llvm::LoopInfo &LI,
+                                    const BoxedLoopsSetTy &BoxedLoops);
 } // namespace polly
 #endif

lib/Analysis/ScopBuilder.cpp

 
 #define DEBUG_TYPE "polly-scops"
 
 STATISTIC(ScopFound, "Number of valid Scops");
 STATISTIC(RichScopFound, "Number of Scops containing a loop");
 STATISTIC(InfeasibleScops,
           "Number of SCoPs with statically infeasible context.");
 
-// If the loop is nonaffine/boxed, return the first non-boxed surrounding loop
-// for Polly. If the loop is affine, return the loop itself. Do not call
-// `getSCEVAtScope()` on the result of `getFirstNonBoxedLoopFor()`, as we need
-// to analyze the memory accesses of the nonaffine/boxed loops.
-static Loop *getFirstNonBoxedLoopFor(Loop *L, LoopInfo &LI,
-                                     const BoxedLoopsSetTy &BoxedLoops) {
-  while (BoxedLoops.count(L))
-    L = L->getParentLoop();
-  return L;
-}
-
 static cl::opt<bool> ModelReadOnlyScalars(
     "polly-analyze-read-only-scalars",
     cl::desc("Model read-only scalar values in the scop description"),
     cl::Hidden, cl::ZeroOrMore, cl::init(true), cl::cat(PollyCategory));
 
 void ScopBuilder::buildPHIAccesses(PHINode *PHI, Region *NonAffineSubRegion,
                                    bool IsExitBlock) {
 

lib/Analysis/ScopInfo.cpp

bool Scop::buildDomains(Region *R, DominatorTree &DT, LoopInfo &LI) {
   // parameter combination it would be reached via an error block in its
   // InvalidDomain. This information is needed during load hoisting.
   if (!propagateInvalidStmtDomains(R, DT, LI))
     return false;
 
   return true;
 }
 
-// If the loop is nonaffine/boxed, return the first non-boxed surrounding loop
-// for Polly. If the loop is affine, return the loop itself. Do not call
-// `getSCEVAtScope()` on the result of `getFirstNonBoxedLoopFor()`, as we need
-// to analyze the memory accesses of the nonaffine/boxed loops.
-static Loop *getFirstNonBoxedLoopFor(BasicBlock *BB, LoopInfo &LI,
-                                     const BoxedLoopsSetTy &BoxedLoops) {
-  auto *L = LI.getLoopFor(BB);
-  while (BoxedLoops.count(L))
-    L = L->getParentLoop();
-  return L;
-}
-
 /// Adjust the dimensions of @p Dom that was constructed for @p OldL
 ///        to be compatible to domains constructed for loop @p NewL.
 ///
 /// This function assumes @p NewL and @p OldL are equal or there is a CFG
 /// edge from @p OldL to @p NewL.
 static __isl_give isl_set *adjustDomainDimensions(Scop &S,
                                                   __isl_take isl_set *Dom,
                                                   Loop *OldL, Loop *NewL) {

lib/Support/ScopHelper.cpp

for (unsigned i = 1; i < GEP->getNumOperands(); i++) {
     if (!(DroppedFirstDim && i == 2))
       Sizes.push_back(ArrayTy->getNumElements());
 
     Ty = ArrayTy->getElementType();
   }
 
   return std::make_tuple(Subscripts, Sizes);
 }
+
+llvm::Loop *polly::getFirstNonBoxedLoopFor(llvm::Loop *L, llvm::LoopInfo &LI,
+                                           const BoxedLoopsSetTy &BoxedLoops) {
+  while (BoxedLoops.count(L))
+    L = L->getParentLoop();
+  return L;
+}
+
+llvm::Loop *polly::getFirstNonBoxedLoopFor(llvm::BasicBlock *BB,
+                                           llvm::LoopInfo &LI,
+                                           const BoxedLoopsSetTy &BoxedLoops) {
+  Loop *L = LI.getLoopFor(BB);
+  return getFirstNonBoxedLoopFor(L, LI, BoxedLoops);
+}