[Polly] Refactoring isReductionParallel() and getMinimalDependenceDistance() from Integer Set Libarary(ISL) to take the C++ wrapper

[prateekpardeshi]

Summary

Polly use algorithms from the Integer Set Library (isl), which is a library written in C and which is incompatible with the rest of the LLVM as it is written in C++.

Changes made:

• Refactoring isReductionParallel() and getMinimalDependenceDistance() to take C++ bindings instead of reference-counting in C isl lib.
• Addition of manage_copy() to be used as reference for C objects instead of isReductionParallel() and getMinimalDependenceDistance()

Tests:

• ninja opt: Passed ✔️
• ninja check-polly-tests : Passed ✔️
• ninja polly-update-format: Passed ✔️

[Meinersbur]

Thank for the patch.

Could you change "Refabricating" to "Refactor"? It refers to Code Refactoring.

polly/include/polly/CodeGen/IslAst.h

162–163

For the refactoring of this function to be complete, can you also make it return a isl::pw_aff?

[prateekpardeshi]

Changes made:
making changes in code

[Meinersbur]

In D98765#2631785, @prateekpardeshi wrote:

Changes made:
making changes in code

For future updates, could you be more detailed?

polly/include/polly/CodeGen/IslAst.h

162–163

Removing __isl_give is not the change I asked for,

polly/include/polly/CodeGen/IslAst.h

public:
 
   /// Is this loop an outermost parallel loop?
   static bool isOutermostParallel(const isl::ast_node &Node);
 
   /// Is this loop an innermost parallel loop?
   static bool isInnermostParallel(const isl::ast_node &Node);
 
   /// Is this loop a reduction parallel loop?
-  static bool isReductionParallel(__isl_keep isl_ast_node *Node);
+  static bool isReductionParallel(const isl::ast_node &Node);
 
   /// Will the loop be run as thread parallel?
   static bool isExecutedInParallel(__isl_keep isl_ast_node *Node);
 
   /// Get the nodes schedule or a nullptr if not available.
   static __isl_give isl_union_map *getSchedule(__isl_keep isl_ast_node *Node);
 
   /// Get minimal dependence distance or nullptr if not available.
-  static __isl_give isl_pw_aff *
-  getMinimalDependenceDistance(__isl_keep isl_ast_node *Node);
+  static isl_pw_aff *getMinimalDependenceDistance(const isl::ast_node &Node);
 
   /// Get the nodes broken reductions or a nullptr if not available.
   static MemoryAccessSet *getBrokenReductions(__isl_keep isl_ast_node *Node);
 
   /// Get the nodes build context or a nullptr if not available.
   static __isl_give isl_ast_build *getBuild(__isl_keep isl_ast_node *Node);
 
   ///}

polly/lib/CodeGen/IslAst.cpp

static const std::string getBrokenReductionsStr(__isl_keep isl_ast_node *Node) {
 
   return str;
 }
 
 /// Callback executed for each for node in the ast in order to print it.
 static isl_printer *cbPrintFor(__isl_take isl_printer *Printer,
                                __isl_take isl_ast_print_options *Options,
                                __isl_keep isl_ast_node *Node, void *) {
-  isl_pw_aff *DD = IslAstInfo::getMinimalDependenceDistance(Node);
+  isl_pw_aff *DD =
+      IslAstInfo::getMinimalDependenceDistance(isl::manage_copy(Node));
   const std::string BrokenReductionsStr = getBrokenReductionsStr(Node);
   const std::string KnownParallelStr = "#pragma known-parallel";
   const std::string DepDisPragmaStr = "#pragma minimal dependence distance: ";
   const std::string SimdPragmaStr = "#pragma simd";
   const std::string OmpPragmaStr = "#pragma omp parallel for";
 
   if (DD)
     Printer = printLine(Printer, DepDisPragmaStr, DD);
isl_ast_node_foreach_descendant_top_down(
         case isl_ast_node_for:
           NumForLoops++;
           if (IslAstInfo::isParallel(Node))
             NumParallel++;
           if (IslAstInfo::isInnermostParallel(isl::manage_copy(Node)))
             NumInnermostParallel++;
           if (IslAstInfo::isOutermostParallel(isl::manage_copy(Node)))
             NumOutermostParallel++;
-          if (IslAstInfo::isReductionParallel(Node))
+          if (IslAstInfo::isReductionParallel(isl::manage_copy(Node)))
             NumReductionParallel++;
           if (IslAstInfo::isExecutedInParallel(Node))
             NumExecutedInParallel++;
           break;
 
         case isl_ast_node_if:
           NumIfConditions++;
           break;
bool IslAstInfo::isInnermostParallel(const isl::ast_node &Node) {
   return Payload && Payload->IsInnermostParallel;
 }
 
 bool IslAstInfo::isOutermostParallel(const isl::ast_node &Node) {
   IslAstUserPayload *Payload = getNodePayload(Node);
   return Payload && Payload->IsOutermostParallel;
 }
 
-bool IslAstInfo::isReductionParallel(__isl_keep isl_ast_node *Node) {
-  IslAstUserPayload *Payload = getNodePayload(isl::manage_copy(Node));
+bool IslAstInfo::isReductionParallel(const isl::ast_node &Node) {
+  IslAstUserPayload *Payload = getNodePayload(Node);
   return Payload && Payload->IsReductionParallel;
 }
 
 bool IslAstInfo::isExecutedInParallel(__isl_keep isl_ast_node *Node) {
   if (!PollyParallel)
     return false;
 
   // Do not parallelize innermost loops.
   //
   // Parallelizing innermost loops is often not profitable, especially if
   // they have a low number of iterations.
   //
   // TODO: Decide this based on the number of loop iterations that will be
   //       executed. This can possibly require run-time checks, which again
   //       raises the question of both run-time check overhead and code size
   //       costs.
   if (!PollyParallelForce && isInnermost(Node))
     return false;
 
   return isOutermostParallel(isl::manage_copy(Node)) &&
-         !isReductionParallel(Node);
+         !isReductionParallel(isl::manage_copy(Node));
 }
 
 __isl_give isl_union_map *
 IslAstInfo::getSchedule(__isl_keep isl_ast_node *Node) {
   IslAstUserPayload *Payload = getNodePayload(isl::manage_copy(Node));
   return Payload ? isl_ast_build_get_schedule(Payload->Build) : nullptr;
 }
 
 __isl_give isl_pw_aff *
-IslAstInfo::getMinimalDependenceDistance(__isl_keep isl_ast_node *Node) {
-  IslAstUserPayload *Payload = getNodePayload(isl::manage_copy(Node));
+IslAstInfo::getMinimalDependenceDistance(const isl::ast_node &Node) {
+  IslAstUserPayload *Payload = getNodePayload(Node);
   return Payload ? Payload->MinimalDependenceDistance.copy() : nullptr;
 }
 
 IslAstInfo::MemoryAccessSet *
 IslAstInfo::getBrokenReductions(__isl_keep isl_ast_node *Node) {
   IslAstUserPayload *Payload = getNodePayload(isl::manage_copy(Node));
   return Payload ? &Payload->BrokenReductions : nullptr;
 }

polly/lib/CodeGen/IslNodeBuilder.cpp

return isl_ast_node_foreach_descendant_top_down(
              },
              nullptr) == isl_stat_error;
 }
 
 void IslNodeBuilder::createFor(__isl_take isl_ast_node *For) {
   bool Vector = PollyVectorizerChoice == VECTORIZER_POLLY;
 
   if (Vector && IslAstInfo::isInnermostParallel(isl::manage_copy(For)) &&
-      !IslAstInfo::isReductionParallel(For)) {
+      !IslAstInfo::isReductionParallel(isl::manage_copy(For))) {
     int VectorWidth = getNumberOfIterations(isl::manage_copy(For));
     if (1 < VectorWidth && VectorWidth <= 16 && !hasPartialAccesses(For)) {
       createForVector(For, VectorWidth);
       return;
     }
   }
 
   if (IslAstInfo::isExecutedInParallel(For)) {
     createForParallel(For);
     return;
   }
-  bool Parallel =
-      (IslAstInfo::isParallel(For) && !IslAstInfo::isReductionParallel(For));
+  bool Parallel = (IslAstInfo::isParallel(For) &&
+                   !IslAstInfo::isReductionParallel(isl::manage_copy(For)));
   createForSequential(isl::manage(For), Parallel);
 }
 
 void IslNodeBuilder::createIf(__isl_take isl_ast_node *If) {
   isl_ast_expr *Cond = isl_ast_node_if_get_cond(If);
 
   Function *F = Builder.GetInsertBlock()->getParent();
   LLVMContext &Context = F->getContext();