[PATCH] D98455: [Polly] Refactoring astScheduleDimIsParallel to take the C++ wrapper object. NFC

[Kevin Zhou via Phabricator via llvm-commits]

QwertycowMoo created this revision.
QwertycowMoo added a reviewer: Meinersbur.
QwertycowMoo added a project: Polly.
Herald added a reviewer: bollu.
QwertycowMoo requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Polly currently needs to be slowly refactor to use the C++ wrapper objects to handle the reference counters automatically.
I took the function of astScheduleDimIsParallel and refactored it so that it uses the C++ wrapper function as much as possible.

There are some problems with the IsParallel since it expects the C objects, so the C++ wrapper functions must be .release() and .get() first before they are able to be used with IsParallel.

When checking the ReductionDependencies Parallelism with the Build's Schedule, I opted to keep the union map as a C object rather than a C++ object. Eventually, changes will need to be made to IsParallel to refactor it to the C++ wrappers. When this is done, this function will also need to be slightly refactored to not use the C object.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98455

Files:
  polly/lib/CodeGen/IslAst.cpp


Index: polly/lib/CodeGen/IslAst.cpp
===================================================================
--- polly/lib/CodeGen/IslAst.cpp
+++ polly/lib/CodeGen/IslAst.cpp
@@ -203,50 +203,48 @@
 /// dependences connect all iterations with each other (thus they are cyclic)
 /// we can perform the parallelism check as we are only interested in a zero
 /// (or non-zero) dependence distance on the dimension in question.
-static bool astScheduleDimIsParallel(__isl_keep isl_ast_build *Build,
+static bool astScheduleDimIsParallel(const isl::ast_build &Build,
                                      const Dependences *D,
                                      IslAstUserPayload *NodeInfo) {
+
   if (!D->hasValidDependences())
     return false;
 
-  isl_union_map *Schedule = isl_ast_build_get_schedule(Build);
-  isl_union_map *Deps =
-      D->getDependences(Dependences::TYPE_RAW | Dependences::TYPE_WAW |
-                        Dependences::TYPE_WAR)
-          .release();
+  isl::union_map Schedule = Build.get_schedule();
+  isl::union_map Dep = D->getDependences(
+      Dependences::TYPE_RAW | Dependences::TYPE_WAW | Dependences::TYPE_WAR);
 
-  if (!D->isParallel(Schedule, Deps)) {
-    isl_union_map *DepsAll =
+  if (!D->isParallel(Schedule.get(), Dep.release())) {
+    isl::union_map DepsAll =
         D->getDependences(Dependences::TYPE_RAW | Dependences::TYPE_WAW |
-                          Dependences::TYPE_WAR | Dependences::TYPE_TC_RED)
-            .release();
-    isl_pw_aff *MinimalDependenceDistance = nullptr;
-    D->isParallel(Schedule, DepsAll, &MinimalDependenceDistance);
+                          Dependences::TYPE_WAR | Dependences::TYPE_TC_RED);
+    isl::pw_aff MinimalDependenceDistance;
+    // We will need to change isParallel to stop the unwrapping
+    isl_pw_aff *MinimalDependenceDistanceIsl =
+        MinimalDependenceDistance.release();
+    D->isParallel(Schedule.get(), DepsAll.release(),
+                  &MinimalDependenceDistanceIsl);
     NodeInfo->MinimalDependenceDistance =
-        isl::manage(MinimalDependenceDistance);
-    isl_union_map_free(Schedule);
+        isl::manage(MinimalDependenceDistanceIsl);
     return false;
   }
 
-  isl_union_map *RedDeps =
-      D->getDependences(Dependences::TYPE_TC_RED).release();
-  if (!D->isParallel(Schedule, RedDeps))
+  isl::union_map RedDeps = D->getDependences(Dependences::TYPE_TC_RED);
+  if (!D->isParallel(Schedule.get(), RedDeps.release())) {
     NodeInfo->IsReductionParallel = true;
+  }
 
-  if (!NodeInfo->IsReductionParallel && !isl_union_map_free(Schedule))
+  if (!NodeInfo->IsReductionParallel)
     return true;
 
-  // Annotate reduction parallel nodes with the memory accesses which caused the
-  // reduction dependences parallel execution of the node conflicts with.
   for (const auto &MaRedPair : D->getReductionDependences()) {
     if (!MaRedPair.second)
       continue;
-    RedDeps = isl_union_map_from_map(isl_map_copy(MaRedPair.second));
-    if (!D->isParallel(Schedule, RedDeps))
+    isl_union_map *RedDeps2 =
+        isl_union_map_from_map(isl_map_copy(MaRedPair.second));
+    if (!D->isParallel(Schedule.get(), RedDeps2))
       NodeInfo->BrokenReductions.insert(MaRedPair.first);
   }
-
-  isl_union_map_free(Schedule);
   return true;
 }
 
@@ -265,8 +263,8 @@
   Id = isl_id_set_free_user(Id, freeIslAstUserPayload);
   BuildInfo->LastForNodeId = Id;
 
-  Payload->IsParallel =
-      astScheduleDimIsParallel(Build, BuildInfo->Deps, Payload);
+  Payload->IsParallel = astScheduleDimIsParallel(isl::manage_copy(Build),
+                                                 BuildInfo->Deps, Payload);
 
   // Test for parallelism only if we are not already inside a parallel loop
   if (!BuildInfo->InParallelFor && !BuildInfo->InSIMD)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98455.330070.patch
Type: text/x-patch
Size: 3781 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210311/27201de6/attachment.bin>