[PATCH] D19526: Allow unsigned comparisons

[Michael Kruse via llvm-commits]

Meinersbur added inline comments.

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

I could understand why checking the Statement because of its domain, but I don't know why checking the accessed array and its size should be relevant.

================
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
----------------
jdoerfert wrote:
> 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?
This is a brand new test case, so who else could have added it?

The problem I see is that the commented C-code does not accurately represent the IR, hence does a disfavor when trying to understand the code. If you do not intend to keep the C-representation in-sync with the IR, you can just remove it.


http://reviews.llvm.org/D19526