[PATCH] D36873: [IRCE] Fix buggy behavior in Clamp

[Anna Thomas via Phabricator via llvm-commits]

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

LGTM w/comment.



================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1088
   auto Clamp = [this, Smallest, Greatest, IsSignedPredicate](const SCEV *S) {
-    return IsSignedPredicate
+    bool MaybeNegativeValues = IsSignedPredicate || !SE.isKnownNonNegative(S);
+    return MaybeNegativeValues
----------------
I think there is just one missed case of checking `isKnownNonNegative` right? 

Just looked through IRCE code and we correctly check it for choosing the right predicate: `ULT` or `SLT`. 


================
Comment at: test/Transforms/IRCE/clamp.ll:9
+define void @test() {
+bci_0:
+  %indvars.iv.next467 = add nuw nsw i64 2, 1
----------------
Please give canonical names to the basic blocks (i.e. entry, header etc).


https://reviews.llvm.org/D36873