[PATCH] D20036: [InstCombine] Fold icmp eq/ne (udiv i32 CI2, A), 0 -> icmp ugt/ule A, CI2

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1543-1550
@@ +1542,10 @@
+                                              ConstantInt *CI2, Value *A) {
+  // (icmp eq/ne (udiv i32 CI2, A), 0) -> (icmp ugt/ule A, CI2)
+  if (I.isEquality() && CI1->isZero()) {
+    ICmpInst::Predicate Pred = I.getPredicate() == ICmpInst::ICMP_EQ
+                                   ? ICmpInst::ICMP_UGT
+                                   : ICmpInst::ICMP_ULE;
+    return new ICmpInst(Pred, A,
+                        ConstantInt::get(A->getType(), CI2->getValue()));
+  }
+  // FIXME: Handle (icmp ugt (udiv i32 CI2, A), CI1) and
----------------
This transform is valid regardless of what value `CI2` holds.  We should require it to be a `ConstantInt`.

================
Comment at: test/Transforms/InstCombine/compare-udiv.ll:3-9
@@ +2,9 @@
+
+; CHECK-LABEL: @test1
+; CHECK: %cmp1 = icmp ugt i32 %a, 64
+define i1 @test1(i32 %a) {
+  %div = udiv i32 64, %a
+  %cmp1 = icmp eq i32 %div, 0
+  ret i1 %cmp1
+}
+
----------------
This transform is valid, regardless of what value the numerator is.  We shouldn't require a constant here.

================
Comment at: test/Transforms/InstCombine/compare-udiv.ll:11-17
@@ +10,8 @@
+
+; CHECK-LABEL: @test2
+; CHECK: %cmp1 = icmp ult i32 %a, 65
+define i1 @test2(i32 %a) {
+  %div = udiv i32 64, %a
+  %cmp1 = icmp ne i32 %div, 0
+  ret i1 %cmp1
+}
----------------
It'd be good to have a version of this test where the numerator is -1.


http://reviews.llvm.org/D20036