[PATCH] D63304: Ignore Singletons in statement domains

[Sameer AbuAsal via Phabricator via llvm-commits]

sabuasal marked 6 inline comments as done.
sabuasal added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:2183-2184
+  if (HandleDoWhile && !S.getRestrictDomainParams().plain_is_universe())
+    AssumptionContext =
+        AssumptionContext.intersect(S.getRestrictDomainParams());
   return AssumptionContext;
----------------
sabuasal wrote:
> Meinersbur wrote:
> > [serious] `simplifyAssumptionContext` should not logically modify the `AssumptionContext`. The intersection should have happened earlier.
> Actually, this has to be done after simplification.  The AssumptionContext is simplified (gisted-ed) with the domain parameters as you can see in line 2178.  If you look at line 3025 you'll see that we also gist the domain parameters with the RestrictDomainParameters we are building.  this can result in a situation where the parameters restriction we add can be completely removed from both the assumption, and hence the runtime check, and the materialized domain. 
> 
> the reason it is important to gist the domain parameters is that it we'd rather push all the complex params to the RTC and keep the materialized statement as simple as possible. 
> 
> Can you suggest a better place for the intersection that insures our restriction params are always present in the AssumptionContext?  I think intersectBBDomainParams() might  work?
> 
> 
> 
Here is an example from an internal test case that shows the problem:

Avoid Single Iter Assumptions: [p_0, p_1, p_2] -> {  : p_1 = 1 and p_2 > 0 and (p_0 >= 32 or p_0 <= 30) }

Assumed context Before gist with domain params: [p_0, p_1, p_2] -> {  : (p_1 = 1 and p_0 >= 32 and p_2 > 0) or (p_1 = 1 and p_0 <= 30 and p_2 > 0) }
Domain parameters: [p_0, p_1, p_2] -> {  : (p_0 >= 32 and p_2 > 0) or (p_0 <= 30 and p_2 > 0) }
Assumed context After simplifying with domain params: [p_0, p_1, p_2] -> {  : (p_1 = 1 and p_0 >= 32) or (p_1 = 1 and p_0 <= 30) }  *Note the loss of p_2 >0*




Repository:
  rPLO Polly

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

https://reviews.llvm.org/D63304