[ScopBuilder] Move buildContext function from ScopInfo

[domada]

Summary

Move buildContext function from ScopInfo to ScopBuilder.

[Meinersbur]

Thank you for your contribution. A few remarks for your first upload for Polly on Phabricator:

• Since these patches are mailed to llvm-commits, we were asked to add [Polly] into the title to make it immediately obvious that it's a patch for a subproject. The tag would be removed for the commit again.
• Append "NFC" (No Function Change intended) to the title for a refactoring patch. This one stays for the commit message.
• Add polly-dev (and llvm-commits) to subscribers.
• Add grosser to the reviewer list. He's code owner for Polly.

I am split with this patch. I generally think that we should avoid calling methods in constructors because it might be surprising which method is actually called. IMHO constructors are for initializing fields but that's exactly what buildContext does. Unfortunately, moving it to ScopBuilder means it directly accesses a foreign object's private fields. That's arguably worse. At some point we would like to drop the friend class ScopBuilder in ScopInfo such that such accesses are not possible anymore.

Methods that I think are better suited to be moved from ScopInfo to ScopBuilder:

• buildInvariantEquivalenceClasses
• buildDomains
• addUserAssumptions
• buildSchedule
• finalizeAccesses
• addUserContext
• addRecordedAssumptions
• buildAliasChecks
• hoistInvariantLoads
• canonicalizeDynamicBasePtrs
• verifyInvariantLoads

polly/include/polly/ScopBuilder.h

class ScopBuilder {
   Value *findFADAllocationVisible(MemAccInst Inst);
 
   // @}
 
   // Build the SCoP for Region @p R.
   void buildScop(Region &R, AssumptionCache &AC,
                  OptimizationRemarkEmitter &ORE);
 
+  /// Build the Context of the Scop.
+  void buildContext();
+
   /// Try to build a multi-dimensional fixed sized MemoryAccess from the
   /// Load/Store instruction.
   ///
   /// @param Inst       The Load/Store instruction that access the memory
   /// @param Stmt       The parent statement of the instruction
   ///
   /// @returns True if the access could be built, False otherwise.
   bool buildAccessMultiDimFixed(MemAccInst Inst, ScopStmt *Stmt);

polly/include/polly/ScopInfo.h

bool canAlwaysBeHoisted(MemoryAccess *MA, bool StmtInvalidCtxIsEmpty,
                           bool NonHoistableCtxIsEmpty);
 
   /// Add invariant loads listed in @p InvMAs with the domain of @p Stmt.
   void addInvariantLoads(ScopStmt &Stmt, InvariantAccessesTy &InvMAs);
 
   /// Create an id for @p Param and store it in the ParameterIds map.
   void createParameterId(const SCEV *Param);
 
-  /// Build the Context of the Scop.
-  void buildContext();
-
   /// Add user provided parameter constraints to context (source code).
   void addUserAssumptions(AssumptionCache &AC, DominatorTree &DT, LoopInfo &LI,
                           DenseMap<BasicBlock *, isl::set> &InvalidDomainMap);
 
   /// Add user provided parameter constraints to context (command line).
   void addUserContext();
 
   /// Add the bounds of the parameters to the context.

polly/lib/Analysis/ScopBuilder.cpp

static cl::opt<GranularityChoice> StmtGranularity(
     cl::values(clEnumValN(GranularityChoice::BasicBlocks, "bb",
                           "One statement per basic block"),
                clEnumValN(GranularityChoice::ScalarIndependence, "scalar-indep",
                           "Scalar independence heuristic"),
                clEnumValN(GranularityChoice::Stores, "store",
                           "Store-level granularity")),
     cl::init(GranularityChoice::ScalarIndependence), cl::cat(PollyCategory));
 
+void ScopBuilder::buildContext() {
+  isl::space Space = isl::space::params_alloc(scop->getIslCtx(), 0);
+  scop->Context = isl::set::universe(Space);
+  scop->InvalidContext = isl::set::empty(Space);
+  scop->AssumedContext = isl::set::universe(Space);
+}
+
 void ScopBuilder::buildPHIAccesses(ScopStmt *PHIStmt, PHINode *PHI,
                                    Region *NonAffineSubRegion,
                                    bool IsExitBlock) {
   // PHI nodes that are in the exit block of the region, hence if IsExitBlock is
   // true, are not modeled as ordinary PHI nodes as they are not part of the
   // region. However, we model the operands in the predecessor blocks that are
   // part of the region as regular scalar accesses.
 
static inline BasicBlock *getRegionNodeBasicBlock(RegionNode *RN) {
   return RN->isSubRegion() ? RN->getNodeAs<Region>()->getEntry()
                            : RN->getNodeAs<BasicBlock>();
 }
 
 void ScopBuilder::buildScop(Region &R, AssumptionCache &AC,
                             OptimizationRemarkEmitter &ORE) {
   scop.reset(new Scop(R, SE, LI, DT, *SD.getDetectionContext(&R), ORE));
 
+  buildContext();
+
   buildStmts(R);
 
   // Create all invariant load instructions first. These are categorized as
   // 'synthesizable', therefore are not part of any ScopStmt but need to be
   // created somewhere.
   const InvariantLoadsSetTy &RIL = scop->getRequiredInvariantLoads();
   for (BasicBlock *BB : scop->getRegion().blocks()) {
     if (isErrorBlock(*BB, scop->getRegion(), LI, DT))

polly/lib/Analysis/ScopInfo.cpp

for (LoadInst *LInst : RIL) {
     }
 
     ClassRep = LInst;
     InvariantEquivClasses.emplace_back(
         InvariantEquivClassTy{PointerSCEV, MemoryAccessList(), nullptr, Ty});
   }
 }
 
-void Scop::buildContext() {
-  isl::space Space = isl::space::params_alloc(getIslCtx(), 0);
-  Context = isl::set::universe(Space);
-  InvalidContext = isl::set::empty(Space);
-  AssumedContext = isl::set::universe(Space);
-}
-
 void Scop::addParameterBounds() {
   unsigned PDim = 0;
   for (auto *Parameter : Parameters) {
     ConstantRange SRange = SE->getSignedRange(Parameter);
     Context = addRangeBoundsToSet(Context, SRange, PDim++, isl::dim::param);
   }
 }
 
Scop::Scop(Region &R, ScalarEvolution &ScalarEvolution, LoopInfo &LI,
            DominatorTree &DT, ScopDetection::DetectionContext &DC,
            OptimizationRemarkEmitter &ORE)
     : IslCtx(isl_ctx_alloc(), isl_ctx_free), SE(&ScalarEvolution), DT(&DT),
       R(R), name(None), HasSingleExitEdge(R.getExitingBlock()), DC(DC),
       ORE(ORE), Affinator(this, LI),
       ID(getNextID((*R.getEntry()->getParent()).getName().str())) {
   if (IslOnErrorAbort)
     isl_options_set_on_error(getIslCtx().get(), ISL_ON_ERROR_ABORT);
-  buildContext();
 }
 
 Scop::~Scop() = default;
 
 void Scop::foldSizeConstantsToRight() {
   isl::union_set Accessed = getAccesses().range();
 
   for (auto Array : arrays()) {