[PATCH] D63305: Propagate Trip count Assumptions to runtime check

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: test/ScopDetect/model-dowhile-update-rtc.ll:33
+; NOFIXRTC: [p_0] -> {  : -2147483648 <= p_0 <= 2147483647 }
+; NOFIXRTC: if (p_0 >= 5 && 0 == (p_0 <= -1 || p_0 == 2147483647) && (&MemRef0[p_0 + 1] <= &MemRef1[5] || &MemRef1[p_0 + 1] <= &MemRef0[5]))
+
----------------
sabuasal wrote:
> Meinersbur wrote:
> > `p_0 >= 5 && !(p_0 <= -1 || p_0 == 2147483647)` is equivalent to `p_0 >= 5 && p_0 > -1 && p_0 != 2147483647` which could be simplified to `p_0 >= 5 && p_0 != 2147483647`, ie the FIXRTC case. I guess this is a consequence of having split `InvalidContext` and `AssumedContext` to avoid a `complement()`.
> > 
> > I'd prefer some more intelligent treatment of the `InvalidContext` and `AssumedContext` handling over special handling of `UPPERBOUND` assumptions.
> > 
> > The `buildMinMaxAccess` change is not reflected in this test case.
> > 
> > I'd prefer some more intelligent treatment of the InvalidContext and AssumedContext handling over special handling of UPPERBOUND assumptions.
> > 
> 
> Can you please clarify, are you asking for a different way to propagate the assumptions that don't involve simply intersecting them?
> 
> 
One could determine whether a `complement()` would actually be expensive. E.g. for `p_0 >= 5` we have have choices:
1. Add `p_0 >= 5` to `AssumedContext`
2. Add `p_0 < 5` to `InvalidContext`

For this case, they are equally complex, so we could prefer one over the other.

Another idea is to apply `InvalidContext = isl_set_gist(InvalidContext, AssumedContext)` that theoretically would remove unnecessary conditions from `InvalidContext`.

Can you confirm that the separation of InvalidContext/AssumedContext is the issue? I am currently just assuming that.


Repository:
  rPLO Polly

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

https://reviews.llvm.org/D63305