[PATCH] D14413: [Polly][WIP] Use parameter constraints provided via llvm.assume

[Tobias Grosser via llvm-commits]

grosser accepted this revision.
grosser added a comment.
This revision is now accepted and ready to land.

Hi Johannes,

this patch looks good.

Tobias
PS: Please add next time the tests right away. This would make the review easier and more complete.


================
Comment at: lib/Analysis/ScopInfo.cpp:1063
@@ -1059,1 +1062,3 @@
+    ConsequenceCondSet = isl_set_params(ConsequenceCondSet);
+
   assert(ConsequenceCondSet);
----------------
I think using a non-defined TI parameter to look for parameter constraints is rather unexpected. You should at least document this in the function header. Probably even better to understand would be to split this function into the part that creates the ConsequenceSet and the one that adds it
and then create two simple helpers, one for the parameter and one for the non parameter case.

================
Comment at: lib/Analysis/ScopInfo.cpp:1593
@@ +1592,3 @@
+                                     CI->getDebugLoc(),
+                                     "Non-affine user assumption.");
+      continue;
----------------
What about adding "ignored":

"Non-affine user assumption ignored"


http://reviews.llvm.org/D14413