InvariantEquivClassTy: Use struct instead of 4-tuple to increase readability

[grosser]

Summary

With a struct we can use named accessors instead of generic std::get<3>()
calls. This increases readability of the source code.

[grosser]

Johannes currently does not focus on reviewing Polly patches and suggested that we do not wait for his reviews. I consequently close this review, as I was mostly interested to understand why he chose tuples in the first place. I will commit this change now, but very much invite post-commit reviews.

polly/trunk/include/polly/ScopInfo.h

struct InvariantAccess {
   /// @brief The context under which the access is not invariant.
   isl_set *NonHoistableCtx;
 };
 
 /// @brief Ordered container type to hold invariant accesses.
 using InvariantAccessesTy = SmallVector<InvariantAccess, 8>;
 
 /// @brief Type for equivalent invariant accesses and their domain context.
+struct InvariantEquivClassTy {
+
+  /// The pointer that identifies this equivalence class
+  const SCEV *IdentifyingPointer;
+
+  /// Memory accesses now treated invariant
+  ///
+  /// These memory accesses access the pointer location that identifies
+  /// this equivalence class. They are treated as invariant and hoisted during
+  /// code generation.
+  MemoryAccessList InvariantAccesses;
+
+  /// The execution context under which the memory location is accessed
   ///
-/// The first element is the SCEV for the pointer/location that identifies this
-/// equivalence class. The second is a list of memory accesses to that location
-/// that are now treated as invariant and hoisted during code generation. The
-/// third element is the execution context under which the invariant memory
-/// location is accessed, hence the union of all domain contexts for the memory
-/// accesses in the list. The last element describes the type of the invariant
-/// access in order to differentiate between different typed invariant loads of
+  /// It is the union of the execution domains of the memory accesses in the
+  /// InvariantAccesses list.
+  isl_set *ExecutionContext;
+
+  /// The type of the invariant access
+  ///
+  /// It is used to differentiate between differently typed invariant loads from
   /// the same location.
-using InvariantEquivClassTy =
-    std::tuple<const SCEV *, MemoryAccessList, isl_set *, Type *>;
+  Type *Type;
+};
 
 /// @brief Type for invariant accesses equivalence classes.
 using InvariantEquivClassesTy = SmallVector<InvariantEquivClassTy, 8>;
 
 /// @brief Statement of the Scop
 ///
 /// A Scop statement represents an instruction in the Scop.
 ///

polly/trunk/lib/Analysis/ScopInfo.cpp

for (LoadInst *LInst : RIL) {
     Type *Ty = LInst->getType();
     LoadInst *&ClassRep = EquivClasses[std::make_pair(PointerSCEV, Ty)];
     if (ClassRep) {
       InvEquivClassVMap[LInst] = ClassRep;
       continue;
     }
 
     ClassRep = LInst;
-    InvariantEquivClasses.emplace_back(PointerSCEV, MemoryAccessList(), nullptr,
-                                       Ty);
+    InvariantEquivClasses.emplace_back(
+        InvariantEquivClassTy{PointerSCEV, MemoryAccessList(), nullptr, Ty});
   }
 }
 
 void Scop::buildContext() {
   isl_space *Space = isl_space_params_alloc(getIslCtx(), 0);
   Context = isl_set_universe(isl_space_copy(Space));
   InvalidContext = isl_set_empty(isl_space_copy(Space));
   AssumedContext = isl_set_universe(Space);
for (MinMaxVectorPairTy &MinMaxAccessPair : MinMaxAliasGroups) {
     }
     for (MinMaxAccessTy &MMA : MinMaxAccessPair.second) {
       isl_pw_multi_aff_free(MMA.first);
       isl_pw_multi_aff_free(MMA.second);
     }
   }
 
   for (const auto &IAClass : InvariantEquivClasses)
-    isl_set_free(std::get<2>(IAClass));
+    isl_set_free(IAClass.ExecutionContext);
 
   // Explicitly release all Scop objects and the underlying isl objects before
   // we relase the isl context.
   Stmts.clear();
   ScopArrayInfoMap.clear();
   AccFuncMap.clear();
 }
 
if (!LInst)
     return nullptr;
 
   if (Value *Rep = InvEquivClassVMap.lookup(LInst))
     LInst = cast<LoadInst>(Rep);
 
   Type *Ty = LInst->getType();
   const SCEV *PointerSCEV = SE->getSCEV(LInst->getPointerOperand());
   for (auto &IAClass : InvariantEquivClasses) {
-    if (PointerSCEV != std::get<0>(IAClass) || Ty != std::get<3>(IAClass))
+    if (PointerSCEV != IAClass.IdentifyingPointer || Ty != IAClass.Type)
       continue;
 
-    auto &MAs = std::get<1>(IAClass);
+    auto &MAs = IAClass.InvariantAccesses;
     for (auto *MA : MAs)
       if (MA->getAccessInstruction() == Val)
         return &IAClass;
   }
 
   return nullptr;
 }
 
for (auto &InvMA : InvMAs) {
     } else {
       MACtx = isl_set_copy(DomainCtx);
       MACtx = isl_set_subtract(MACtx, isl_set_union(MAInvalidCtx, NHCtx));
       MACtx = isl_set_gist_params(MACtx, getContext());
     }
 
     bool Consolidated = false;
     for (auto &IAClass : InvariantEquivClasses) {
-      if (PointerSCEV != std::get<0>(IAClass) || Ty != std::get<3>(IAClass))
+      if (PointerSCEV != IAClass.IdentifyingPointer || Ty != IAClass.Type)
         continue;
 
       // If the pointer and the type is equal check if the access function wrt.
       // to the domain is equal too. It can happen that the domain fixes
       // parameter values and these can be different for distinct part of the
       // SCoP. If this happens we cannot consolidate the loads but need to
       // create a new invariant load equivalence class.
-      auto &MAs = std::get<1>(IAClass);
+      auto &MAs = IAClass.InvariantAccesses;
       if (!MAs.empty()) {
         auto *LastMA = MAs.front();
 
         auto *AR = isl_map_range(MA->getAccessRelation());
         auto *LastAR = isl_map_range(LastMA->getAccessRelation());
         bool SameAR = isl_set_is_equal(AR, LastAR);
         isl_set_free(AR);
         isl_set_free(LastAR);
 
         if (!SameAR)
           continue;
       }
 
       // Add MA to the list of accesses that are in this class.
       MAs.push_front(MA);
 
       Consolidated = true;
 
       // Unify the execution context of the class and this statement.
-      isl_set *&IAClassDomainCtx = std::get<2>(IAClass);
+      isl_set *&IAClassDomainCtx = IAClass.ExecutionContext;
       if (IAClassDomainCtx)
         IAClassDomainCtx =
             isl_set_coalesce(isl_set_union(IAClassDomainCtx, MACtx));
       else
         IAClassDomainCtx = MACtx;
       break;
     }
 
     if (Consolidated)
       continue;
 
     // If we did not consolidate MA, thus did not find an equivalence class
     // for it, we create a new one.
-    InvariantEquivClasses.emplace_back(PointerSCEV, MemoryAccessList{MA}, MACtx,
-                                       Ty);
+    InvariantEquivClasses.emplace_back(
+        InvariantEquivClassTy{PointerSCEV, MemoryAccessList{MA}, MACtx, Ty});
   }
 
   isl_set_free(DomainCtx);
 }
 
 __isl_give isl_set *Scop::getNonHoistableCtx(MemoryAccess *Access,
                                              __isl_keep isl_union_map *Writes) {
   // TODO: Loads that are not loop carried, hence are in a statement with

 }
 
 void Scop::print(raw_ostream &OS) const {
   OS.indent(4) << "Function: " << getFunction().getName() << "\n";
   OS.indent(4) << "Region: " << getNameStr() << "\n";
   OS.indent(4) << "Max Loop Depth:  " << getMaxLoopDepth() << "\n";
   OS.indent(4) << "Invariant Accesses: {\n";
   for (const auto &IAClass : InvariantEquivClasses) {
-    const auto &MAs = std::get<1>(IAClass);
+    const auto &MAs = IAClass.InvariantAccesses;
     if (MAs.empty()) {
-      OS.indent(12) << "Class Pointer: " << *std::get<0>(IAClass) << "\n";
+      OS.indent(12) << "Class Pointer: " << *IAClass.IdentifyingPointer << "\n";
     } else {
       MAs.front()->print(OS);
-      OS.indent(12) << "Execution Context: " << std::get<2>(IAClass) << "\n";
+      OS.indent(12) << "Execution Context: " << IAClass.ExecutionContext
+                    << "\n";
     }
   }
   OS.indent(4) << "}\n";
   printContext(OS.indent(4));
   printArrayInfo(OS.indent(4));
   printAliasAssumptions(OS);
   printStatements(OS.indent(4));
 }

polly/trunk/lib/CodeGen/IslNodeBuilder.cpp

for (auto *Val : Values) {
         }
       }
 
       if (auto *IAClass = S.lookupInvariantEquivClass(Val)) {
 
         // Check if this invariant access class is empty, hence if we never
         // actually added a loads instruction to it. In that case it has no
         // (meaningful) users and we should not try to code generate it.
-        if (std::get<1>(*IAClass).empty())
+        if (IAClass->InvariantAccesses.empty())
           V = UndefValue::get(ParamSCEV->getType());
 
         if (!preloadInvariantEquivClass(*IAClass)) {
           isl_id_free(Id);
           return false;
         }
       }
     }

 }
 
 bool IslNodeBuilder::preloadInvariantEquivClass(
     InvariantEquivClassTy &IAClass) {
   // For an equivalence class of invariant loads we pre-load the representing
   // element with the unified execution context. However, we have to map all
   // elements of the class to the one preloaded load as they are referenced
   // during the code generation and therefor need to be mapped.
-  const MemoryAccessList &MAs = std::get<1>(IAClass);
+  const MemoryAccessList &MAs = IAClass.InvariantAccesses;
   if (MAs.empty())
     return true;
 
   MemoryAccess *MA = MAs.front();
   assert(MA->isArrayKind() && MA->isRead());
 
   // If the access function was already mapped, the preload of this equivalence
   // class was triggered earlier already and doesn't need to be done again.
   if (ValueMap.count(MA->getAccessInstruction()))
     return true;
 
   // Check for recurrsion which can be caused by additional constraints, e.g.,
   // non-finitie loop contraints. In such a case we have to bail out and insert
   // a "false" runtime check that will cause the original code to be executed.
-  auto PtrId = std::make_pair(std::get<0>(IAClass), std::get<3>(IAClass));
+  auto PtrId = std::make_pair(IAClass.IdentifyingPointer, IAClass.Type);
   if (!PreloadedPtrs.insert(PtrId).second)
     return false;
 
   // The exectution context of the IAClass.
-  isl_set *&ExecutionCtx = std::get<2>(IAClass);
+  isl_set *&ExecutionCtx = IAClass.ExecutionContext;
 
   // If the base pointer of this class is dependent on another one we have to
   // make sure it was preloaded already.
   auto *SAI = MA->getScopArrayInfo();
   if (auto *BaseIAClass = S.lookupInvariantEquivClass(SAI->getBasePtr())) {
     if (!preloadInvariantEquivClass(*BaseIAClass))
       return false;
 
     // After we preloaded the BaseIAClass we adjusted the BaseExecutionCtx and
     // we need to refine the ExecutionCtx.
-    isl_set *BaseExecutionCtx = isl_set_copy(std::get<2>(*BaseIAClass));
+    isl_set *BaseExecutionCtx = isl_set_copy(BaseIAClass->ExecutionContext);
     ExecutionCtx = isl_set_intersect(ExecutionCtx, BaseExecutionCtx);
   }
 
   Instruction *AccInst = MA->getAccessInstruction();
   Type *AccInstTy = AccInst->getType();
 
   Value *PreloadVal = preloadInvariantLoad(*MA, isl_set_copy(ExecutionCtx));
   if (!PreloadVal)