[PATCH] D19526: Allow unsigned comparisons

[Johannes Doerfert via llvm-commits]

jdoerfert added inline comments.

================
Comment at: lib/Analysis/ScopInfo.cpp:1323-1326
@@ +1322,6 @@
+      auto *BB = Stmt.getEntryBlock();
+      S.recordAssumption(UNSIGNED, isl_pw_aff_nonneg_set(isl_pw_aff_copy(LHS)),
+                         TI->getDebugLoc(), AS_ASSUMPTION, BB);
+      S.recordAssumption(UNSIGNED, isl_pw_aff_nonneg_set(isl_pw_aff_copy(RHS)),
+                         TI->getDebugLoc(), AS_ASSUMPTION, BB);
+    }
----------------
Meinersbur wrote:
> I assume that RTCs checks will always be signed therefor this will check whether the MSB is not 1. Could you add a comment for that?
Will do.

================
Comment at: test/ScopInfo/simple_loop_unsigned.ll:3-7
@@ +2,7 @@
+
+; void f(int a[], int N) {
+;   int i;
+;   for (i = 0; i < N; ++i)
+;     a[i] = i;
+; }
+
----------------
Meinersbur wrote:
> This has no unsigned comparison. Can you make it represent the IR below? Eg.
> 
>     for (i = 0; (uint64_t)i < (uint64_t)N; ++i)
> 
> %i is of type i64, so I suggest to declare `i` as `int64_t`
Ok.

================
Comment at: test/ScopInfo/simple_loop_unsigned.ll:13
@@ +12,3 @@
+; CHECK:      Arrays {
+; CHECK-NEXT:     i64 MemRef_a[*]; // Element size 8
+; CHECK-NEXT: }
----------------
Meinersbur wrote:
> Why is this significant for the test case?
Idk. Most of the check lines are not. Is this a problem now?

================
Comment at: test/ScopInfo/simple_loop_unsigned_2.ll:36
@@ +35,3 @@
+  store i64 %i, i64* %scevgep
+  %i.dec = add nsw i64 %i, -1
+  %exitcond = icmp ugt i64 %i.dec, 0
----------------
Meinersbur wrote:
> Since `i` is unsigned, why does it have a nsw flag?
that wasn't me (I think). Is it really important if it has this flag or not?


http://reviews.llvm.org/D19526