[PATCH] [Unfinished] Use modulo semantic to generate non-wrap assumptions

[Johannes Doerfert]

Some comments, new version is coming soon.


================
Comment at: lib/Analysis/ScopInfo.cpp:94
@@ -91,2 +93,3 @@
 
-  SCEVAffinator(const ScopStmt *Stmt);
+  /// @brief Flag to inidicate that modulo semantic should be used.
+  const bool useModulo;
----------------
grosser wrote:
> indicate
Done.

================
Comment at: lib/Analysis/ScopInfo.cpp:108
@@ -95,1 +107,3 @@
+                                        SCEV::NoWrapFlags Flags);
+
   __isl_give isl_pw_aff *visit(const SCEV *Expr);
----------------
grosser wrote:
> Why do you call this function 'NonWrap'? Is the point of this function not to add the integer wrapping? Could a name like 'addIntegerWrapping()' be a better fit?
is
  addModuloSemantic
fine?

================
Comment at: lib/Analysis/ScopInfo.cpp:131
@@ -118,1 +130,3 @@
+__isl_give isl_pw_aff *SCEVAffinator::getPwAff(ScopStmt *Stmt, const SCEV *Scev,
+                                               bool useModulo) {
   Scop *S = Stmt->getParent();
----------------
grosser wrote:
> UseModulo (start with uppercase)
Done.

================
Comment at: lib/Analysis/ScopInfo.cpp:143
@@ +142,3 @@
+                                                     const SCEV *Expr,
+                                                     SCEV::NoWrapFlags Flags) {
+  Expr->dump();
----------------
grosser wrote:
> A comment explaining how we implement the modulo of signed types might be helpful.
> 
> res = ((res + 2^7) mod (2 ^ 8)) - 2^7
Done.

================
Comment at: lib/Analysis/ScopInfo.cpp:144
@@ +143,3 @@
+                                                     SCEV::NoWrapFlags Flags) {
+  Expr->dump();
+  if (Flags & SCEV::FlagNSW)
----------------
grosser wrote:
> Leftover.
Done.

================
Comment at: lib/Analysis/ScopInfo.cpp:147
@@ +146,3 @@
+    return PWA;
+  errs() << " No nsw!\n";
+
----------------
grosser wrote:
> Leftover.
Done.

================
Comment at: lib/Analysis/ScopInfo.cpp:268
@@ -221,1 +267,3 @@
+  assert(S->getRegion().contains(Expr->getLoop()) &&
+         "Scop does not contain the loop referenced in this AddRec");
 
----------------
grosser wrote:
> This assert now fires for a couple of test cases, e.g. :
> test/ScopInfo/NonAffine/non-affine-loop-condition-dependent-access_2.ll
I haven't looked at all the testcases yet but I am aware that some fail (hence the [Unfinished] part).
I'll check what happens here.

================
Comment at: lib/Analysis/ScopInfo.cpp:281
@@ -232,5 +280,3 @@
 
-    // TODO: Do we need to check for NSW and NUW?
-    return isl_pw_aff_add(Start, isl_pw_aff_mul(Step, LPwAff));
-  }
+  isl_pw_aff *AddRecPWA = isl_pw_aff_add(Start, isl_pw_aff_mul(Step, LPwAff));
 
----------------
grosser wrote:
> It seems you used this change remove the special case of isZero(). This special-case-removal seems partially unrelated. Maybe you want to commit this cleanup ahead of time (no review needed).
I will split it into a separate patch. It is needed as the new SCEV we introduced doesn't have to original flags and I don't see how we can keep them with the transformation we apply.

================
Comment at: test/ScopInfo/wraping_signed_expr_1.ll:31
@@ +30,3 @@
+bb4:                                              ; preds = %bb2
+  %tmp5 = add nsw nuw i64 %indvars.iv, 1
+  %tmp6 = getelementptr i32, i32* %A, i64 %tmp5
----------------
grosser wrote:
> Would it make sense to add a version that lacks the 'nsw' flag here?
I have to check.

http://reviews.llvm.org/D9099

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/