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

[Kevin Zhou via Phabricator via llvm-commits]

QwertycowMoo added a comment.

Yup! Please push for me I'm not sure whether I should be doing that yet so if you @Meinersbur could push it that would be great!



================
Comment at: polly/lib/CodeGen/IslAst.cpp:243-245
+    isl_union_map *RedDeps2 =
+        isl_union_map_from_map(isl_map_copy(MaRedPair.second));
+    if (!D->isParallel(Schedule.get(), RedDeps2))
----------------
Meinersbur wrote:
> I used `RedDeps2` in my examples, but its not a good variable name.
> 
> > 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.
> 
> [suggestion] Consider
> ```
> isl::union_map MaRedDeps = isl::manage_copy(MaRedPair.second);
> if (!D->isParallel(Schedule.get(), MaRedDeps.release()))
> ```
> I think it still looks better even without converting `IsParallel` as well.
> 
Does this mean that we can have implicit conversion from isl::map to isl::union_map?
It seems that MaRedPair.second is an `isl_map` and when we apply `isl::manage_copy` we get an isl::map.
Can we implicitly convert that to isl::union_map?



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98455/new/

https://reviews.llvm.org/D98455