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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a subscriber: grosser.
Meinersbur added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:2308
+
+  if (isl_set_n_basic_set(simple_set.get()) >= MaxDisjunctsInDomain)
+    simple_set = simple_set.simple_hull();
----------------
cs15btech11044 wrote:
> Meinersbur wrote:
> > Consider using `RunTimeChecksMaxAccessDisjuncts` which the current trunk uses.
> I do not quite understand why `RunTimeChecksMaxAccessDisjuncts` needs to be used here.
> 
> If I remember correctly, the point of this patch was to remove max-disjuncts bailout in `MinMaxAccess` so that the computation limit will decide whether the memory access needs to be considered or not. 
> Then why is it being reintroduced here?
The idea was to remove a reason for a bail-out to happen (which occurred in a hot loop in a SPEC benchmark), if we can avoid it. Part of the reason it triggered was that the code does not coalesce() after remove_divs(). Instead of bailing-out we can apply a rougher approximation than remove_divs(): simple_hull() in this case.

>  remove max-disjuncts bailout in MinMaxAccess so that the computation limit will decide whether the memory access needs to be considered or not. 

This was my idea for it, but according to @grosser we should avoid the additional computation time in `lexmin_pw_multi_aff()` to wait for the computation limit to trigger when currently we have a bail-out before that computational overhead.

How would you handle the situation?


Repository:
  rPLO Polly

https://reviews.llvm.org/D45066