[Polly] Refactor buildScop

[Meinersbur]

Summary

The extracted function buildBBScopStmt will be needed later to be invoked individually on the region's exit block.

[jdoerfert]

1 Comment but otherwise LGTM.

include/polly/ScopInfo.h

914

Usually method names start with a small letter, what about combineInSequence(..., ...) ?

[grosser]

LGTM.

Thanks for helping to keep the functions small.

include/polly/ScopInfo.h

891

BasicBlock

893

Points missing (for both sentences).

[Meinersbur]

Addressed comments.
Made combineInSequence a static function instead private method because it does not need a object reference.

[grosser]

Looks good.

[Meinersbur]

Committed as rL244443

include/polly/ScopInfo.h

private:
   /// @param R          The region we build the statement for (or null).
   /// @param tempScop   The temp SCoP we use as model.
   /// @param CurRegion  The SCoP region.
   /// @param NestLoops  A vector of all surrounding loops.
   ScopStmt *addScopStmt(BasicBlock *BB, Region *R, TempScop &tempScop,
                         const Region &CurRegion,
                         SmallVectorImpl<Loop *> &NestLoops);
 
+  /// @brief Create the ScopStmt for a BasicBlock and return its schedule.
+  ///
+  /// Returns null if the BB is trivial and no stmt has been created.
+  ///
+  /// @param BB         The basic block we build the statement for.
+  /// @param tempScop   The temp SCoP we use as model.
+  /// @param CurRegion  The SCoP region.
+  /// @param NestLoops  A vector of all surrounding loops.
+  ///
+  /// @return The ScopStmt's schedule.
+  __isl_give isl_schedule *buildBBScopStmt(BasicBlock *BB, TempScop &tempScop,
+                                           const Region &CurRegion,
+                                           SmallVectorImpl<Loop *> &NestLoops);
+
   /// @brief Build Scop and ScopStmts from a given TempScop.
   ///
   /// @param TempScop  The temporary scop that is translated into an actual
   ///                  scop.
   /// @param CurRegion The subregion of the current scop that we are currently
   ///                  translating.
   /// @param NestLoop  The set of loops that surround the current subregion.
   /// @param LI        The LoopInfo object.
   /// @param SD        The ScopDetection object.
   __isl_give isl_schedule *buildScop(TempScop &TempScop,
                                      const Region &CurRegion,
                                      SmallVectorImpl<Loop *> &NestLoops,
                                      LoopInfo &LI, ScopDetection &SD);
 
   /// @name Helper function for printing the Scop.
   ///
   ///{
   void printContext(raw_ostream &OS) const;

lib/Analysis/ScopInfo.cpp

static cl::opt<unsigned> RunTimeChecksMaxParameters(
     cl::desc("The maximal number of parameters allowed in RTCs."), cl::Hidden,
     cl::ZeroOrMore, cl::init(8), cl::cat(PollyCategory));
 
 static cl::opt<unsigned> RunTimeChecksMaxArraysPerGroup(
     "polly-rtc-max-arrays-per-group",
     cl::desc("The maximal number of arrays to compare in each alias group."),
     cl::Hidden, cl::ZeroOrMore, cl::init(20), cl::cat(PollyCategory));
 
+// Create a sequence of two schedules. Either argument may be null and is
+// interpreted as the empty schedule. Can also return null if both schedules are
+// empty.
+static __isl_give isl_schedule *
+combineInSequence(__isl_take isl_schedule *Prev,
+                  __isl_take isl_schedule *Succ) {
+  if (!Prev)
+    return Succ;
+  if (!Succ)
+    return Prev;
+
+  return isl_schedule_sequence(Prev, Succ);
+}
+
 /// Translate a 'const SCEV *' expression in an isl_pw_aff.
 struct SCEVAffinator : public SCEVVisitor<SCEVAffinator, isl_pw_aff *> {
 public:
   /// @brief Translate a 'const SCEV *' to an isl_pw_aff.
   ///
   /// @param Stmt The location at which the scalar evolution expression
   ///             is evaluated.
   /// @param Expr The expression that is translated.
if (BB) {
     Stmts.emplace_back(*this, tempScop, CurRegion, *R, NestLoops);
     Stmt = &Stmts.back();
     for (BasicBlock *BB : R->blocks())
       StmtMap[BB] = Stmt;
   }
   return Stmt;
 }
 
+__isl_give isl_schedule *
+Scop::buildBBScopStmt(BasicBlock *BB, TempScop &tempScop,
+                      const Region &CurRegion,
+                      SmallVectorImpl<Loop *> &NestLoops) {
+  if (isTrivialBB(BB, tempScop))
+    return nullptr;
+
+  auto *Stmt = addScopStmt(BB, nullptr, tempScop, CurRegion, NestLoops);
+  auto *Domain = Stmt->getDomain();
+  return isl_schedule_from_domain(isl_union_set_from_set(Domain));
+}
+
 __isl_give isl_schedule *Scop::buildScop(TempScop &tempScop,
                                          const Region &CurRegion,
                                          SmallVectorImpl<Loop *> &NestLoops,
                                          LoopInfo &LI, ScopDetection &SD) {
   if (SD.isNonAffineSubRegion(&CurRegion, &getRegion())) {
     auto *Stmt = addScopStmt(nullptr, const_cast<Region *>(&CurRegion),
                              tempScop, CurRegion, NestLoops);
     auto *Domain = Stmt->getDomain();
__isl_give isl_schedule *Scop::buildScop(TempScop &tempScop,
   for (Region::const_element_iterator I = CurRegion.element_begin(),
                                       E = CurRegion.element_end();
        I != E; ++I) {
     isl_schedule *StmtSchedule = nullptr;
     if (I->isSubRegion()) {
       StmtSchedule =
           buildScop(tempScop, *I->getNodeAs<Region>(), NestLoops, LI, SD);
     } else {
-      BasicBlock *BB = I->getNodeAs<BasicBlock>();
-
-      if (isTrivialBB(BB, tempScop)) {
-        continue;
-      } else {
-        auto *Stmt = addScopStmt(BB, nullptr, tempScop, CurRegion, NestLoops);
-        auto *Domain = Stmt->getDomain();
-        StmtSchedule = isl_schedule_from_domain(isl_union_set_from_set(Domain));
-      }
+      StmtSchedule = buildBBScopStmt(I->getNodeAs<BasicBlock>(), tempScop,
+                                     CurRegion, NestLoops);
     }
-
-    if (!Schedule)
-      Schedule = StmtSchedule;
-    else if (StmtSchedule)
-      Schedule = isl_schedule_sequence(Schedule, StmtSchedule);
+    Schedule = combineInSequence(Schedule, StmtSchedule);
   }
 
   if (!L)
     return Schedule;
 
   auto *Domain = isl_schedule_get_domain(Schedule);
   if (!isl_union_set_is_empty(Domain)) {
     auto *MUPA = mapToDimension(isl_union_set_copy(Domain), loopDepth);