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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:2306-2308
   if (isl_set_n_basic_set(Set.get()) >= MaxDisjunctsInDomain)
     return isl::stat::error;
 
----------------
cs15btech11044 wrote:
> I would like to have an opinion on this part of codebase here.
> 
> Since the operation of `simple_hull` tries to reduce the number of disjuncts present in the set. Can we conditionally call `simple_hull` only when `MaxDisjunctsInDomain` is triggered?
> In other words, I am proposing to replace
> 
> ```
> if (isl_set_n_basic_set(Set.get()) >= MaxDisjunctsInDomain)
>     return isl::stat::error;
> ```
> 
> with
> 
> 
> ```
> if (isl_set_n_basic_set(Set.get()) >= MaxDisjunctsInDomain)
>     Set = Set.simple_hull();
> if (isl_set_n_basic_set(Set.get()) >= MaxDisjunctsInDomain)
>     return isl::stat::error;
> ```
This resembles example 3. from by last comment, correct? Sounds like a good idea. Could you update this patch?

The error processing in the remainder of this function still has to updated:
1. Must handle an error that would otherwise trigger an assertion at line 2355.
2. Must never add `nullptr` values to `MinMaxAccesses`.

Do you know how to do this?


https://reviews.llvm.org/D45066