[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:2304
 
-  Set = Set.remove_divs();
-
+  Set = Set.simple_hull();
+  
----------------
cs15btech11044 wrote:
> Meinersbur wrote:
> > cs15btech11044 wrote:
> > > On some analysis of this particular line, it turns out that on the application of simple hull, a bounded set results in its unbounded approximation, which is likely the cause of MinPMA and MaxPMA sets computed as null values. Almost all of the failed regression tests are indirectly caused by this unexpected behaviour.
> > Do you have an example for this behavior? If this happens too often or in too important cases, we may need to look for another solution. I would have hoped that this happens less often, or not at all, with affine_hull.
> Yes, there are some examples for this behavior. 
> 
> For example, in the regression test `ScopInfo/invariant_load_zext_parameter-2.ll` before applying `simple_hull` in Line 2304 of ScopInfo.cpp, the set was `[p_0, p_1, p_2, p_3, p_4] -> { MemRef_I1[p_2] : p_1 > -p_0; MemRef_I1[p_3 + p_4] : p_1 > -p_0; MemRef_I1[0] }` , but after applying the hull operation, it got changed to `[p_0, p_1, p_2, p_3, p_4] -> { MemRef_I1[i0] }` , whereby the variable i0 does not have any bounds/conditions to it.
> 
and with affine_hull?

I'd rate all of the test cases as important cases. We should not regress that much. However, we can still improve on the previous mechanism. E.g.

1. call remove_divs (which overapproximates without changing boundaries)
2. call polly::simplify (this should coalesce disjuncts, and unlike how it is in trunk, should be done after remove_divs)
3. check for too many disjuncts. We either abort or try with affine/simple_hull.



https://reviews.llvm.org/D45066