[PATCH] D45066: [Polly] [ScopInfo] Remove bail out condition in buildMinMaxAccess()

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

Thanks for the update. I don't understand why you changed `Set` to a reference and introduced `SimpleSet`?



================
Comment at: lib/Analysis/ScopInfo.cpp:2303
 /// Add the minimal/maximal access in @p Set to @p User.
-static isl::stat
-buildMinMaxAccess(isl::set Set, Scop::MinMaxVectorTy &MinMaxAccesses, Scop &S) {
+static isl::stat buildMinMaxAccess(isl::set &Set,
+                                   Scop::MinMaxVectorTy &MinMaxAccesses,
----------------
Is there a reason to change `Set` to by-reference?


================
Comment at: lib/Analysis/ScopInfo.cpp:2312
 
-  Set = Set.remove_divs();
+  polly::simplify(Set);
+
----------------
Calling `remove_divs` before `simplify` could still be useful. It should not affect the ranges we need to check for aliasing.


================
Comment at: lib/Analysis/ScopInfo.cpp:2317-2318
 
   if (isl_set_n_basic_set(Set.get()) >= MaxDisjunctsInDomain)
     return isl::stat::error;
 
----------------
IMHO this condition can be removed.


================
Comment at: lib/Analysis/ScopInfo.cpp:2393
   auto Lambda = [&MinMaxAccesses, &S](isl::set Set) -> isl::stat {
-    return buildMinMaxAccess(Set, MinMaxAccesses, S);
+    isl::set &SimpleSet = Set;
+    return buildMinMaxAccess(SimpleSet, MinMaxAccesses, S);
----------------
Why introducing an alias for `Set`?


https://reviews.llvm.org/D45066