Propagate Trip count Assumptions to runtime check This patch updates Polly to use the assumptions we add for trip count to simplify the conditions generated in the runtime check added for the Scop. We accomplish this by intersecting the Trip Count assumptions with the InvalidContext (used for alias checks) then gist these assumptions that are already included in the AssumedContext. This patch saves about 54Kb in Android build (less than 0.01%) and it simplifies the generated runtime check.
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.
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:
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.
Thanks for the catch. First,, this is no longer needed since the buildMinMaxAccess already uses the updated domains for the statements (the ones that got intersected and gisted with RestrictDomainParams). Also, I updated the test case to show the effect on the AliasGroup.
Thanks for the explanation, I agree and I did it using by modifying the first suggestion (in the parent patch). In this patch I just want the Invalid Context to look a bit more simplified.
I agree. I am just not sure it falls within the Scop of this patch. Maybe in a later patch since I think such a change will update lots of test cases as well.
I confirm. We build these two context separately then we generate an AST in:
That is why the AST we get is not the most simplified. My goal from the test case it to show that propagating the info to InvalidContext will make it a bit cleaner.
[type] first letter case: Avoid, Single, Iteration
[suggestion] This seem to be useful enough to be enabled by default and outside of enforceAssumptionsForAvoidsingleIter since it is unrelated.
-S has no effect with --disable-output.
[serious] Is -O3 should not be necessary either; if some transformation is required by -polly-ast, it should be baked-into the test case.