[PATCH] D17247: [Polly] Track assumptions and restrictions separately

[Johannes Doerfert via llvm-commits]

jdoerfert added inline comments.

================
Comment at: lib/Analysis/ScopInfo.cpp:591
@@ -590,2 +590,3 @@
   Outside = isl_set_remove_divs(Outside);
   Outside = isl_set_complement(Outside);
+  auto &Loc = getAccessInstruction() ? getAccessInstruction()->getDebugLoc()
----------------
grosser wrote:
> Is there a specific reason we do not drop the complement and use the other assumption context?
This way all INBOUNDS assumption are "positve" thus assumptions. We could make it an restriction here easily but the current implementation of INBOUNDS further down will always need a complement and the current choice is probably more effiecent.

Tl;dr
No "real" reason. It just looks weird to have INBOUNDS assumptions as well as restrictions...

================
Comment at: lib/Analysis/ScopInfo.cpp:1913
@@ -1935,3 +1912,3 @@
   AssumedContext = simplifyAssumptionContext(AssumedContext, *this);
-  BoundaryContext = simplifyAssumptionContext(BoundaryContext, *this);
+  InvalidContext = simplifyAssumptionContext(InvalidContext, *this);
 }
----------------
grosser wrote:
> Did you check that the very same simplication logic also works for invalid context.
> 
> Assuming you have code
> 
> ```
>    for (i = 0; i < N; i++)
> S:     ...
> ```
> 
> here we have N > 0 when we project the domain on the parameter set.
> 
> Now, assuming the code is invalid for N > 1000. If we gist_simplify the invalid set, we loose this constraint, no?
> 
> 
> 
Mh,... I removed the secon call for now. I have to think about this.


http://reviews.llvm.org/D17247