[llvm] [InstCombine] fold unsigned predicates on srem result (PR #122520)

[Jacob Young via llvm-commits]

================
@@ -2674,10 +2674,41 @@ Instruction *InstCombinerImpl::foldICmpShrConstant(ICmpInst &Cmp,
 Instruction *InstCombinerImpl::foldICmpSRemConstant(ICmpInst &Cmp,
                                                     BinaryOperator *SRem,
                                                     const APInt &C) {
+  const ICmpInst::Predicate Pred = Cmp.getPredicate();
+  if (Pred == ICmpInst::ICMP_UGT || Pred == ICmpInst::ICMP_ULT) {
+    // Canonicalize unsigned predicates to signed:
+    // (X % DivisorC) ugt C -> (X % DivisorC) slt 0
+    //   iff (C slt 0 ? ~C : C) uge abs(DivisorC)-1
+    // (X % DivisorC) ult C+1 -> (X % DivisorC) sgt -1
+    //   iff (C+1 slt 0 ? ~C : C) uge abs(DivisorC)->
+
+    const APInt *DivisorC;
+    if (!match(SRem->getOperand(1), m_APInt(DivisorC)))
+      return nullptr;
+
+    APInt NormalizedC = C;
+    if (Pred == ICmpInst::ICMP_ULT) {
+      assert(!NormalizedC.isZero() &&
+             "ult X, 0 should have been simplified already.");
+      --NormalizedC;
+    }
+    if (C.isNegative())
+      NormalizedC.flipAllBits();
+    assert(!DivisorC->isZero() &&
+           "srem X, 0 should have been simplified already.");
+    if (!NormalizedC.uge(DivisorC->abs() - 1))
----------------
jacobly0 wrote:

Unfortunately, that's the wrong direction (the equivalent expression is `NormalizedC.ugt(DivisorC->abs() - 2)`), so applying that diff causes the fold to no longer happen in cases where it is still valid.  As much as I tried to make the math look nicer that was already the best I could manage while still having the correct bounds.
```diff
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index c107a12fcf43..8d99b5f1022a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -2696,7 +2696,7 @@ Instruction *InstCombinerImpl::foldICmpSRemConstant(ICmpInst &Cmp,
       NormalizedC.flipAllBits();
     assert(!DivisorC->isZero() &&
            "srem X, 0 should have been simplified already.");
-    if (!NormalizedC.uge(DivisorC->abs() - 1))
+    if (!NormalizedC.ugt(DivisorC->abs()))
       return nullptr;
 
     Type *Ty = SRem->getType();
diff --git a/llvm/test/Transforms/InstCombine/icmp-srem.ll b/llvm/test/Transforms/InstCombine/icmp-srem.ll
index 47429dbad74c..3c9a65e57fd0 100644
--- a/llvm/test/Transforms/InstCombine/icmp-srem.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-srem.ll
@@ -17,7 +17,7 @@ define i1 @icmp_ugt_srem5_m5(i32 %x) {
 ; CHECK-LABEL: define i1 @icmp_ugt_srem5_m5(
 ; CHECK-SAME: i32 [[X:%.*]]) {
 ; CHECK-NEXT:    [[R:%.*]] = srem i32 [[X]], 5
-; CHECK-NEXT:    [[C:%.*]] = icmp slt i32 [[R]], 0
+; CHECK-NEXT:    [[C:%.*]] = icmp ugt i32 [[R]], -5
 ; CHECK-NEXT:    ret i1 [[C]]
 ;
   %r = srem i32 %x, 5
@@ -53,7 +53,7 @@ define i1 @icmp_ugt_srem5_4(i32 %x) {
 ; CHECK-LABEL: define i1 @icmp_ugt_srem5_4(
 ; CHECK-SAME: i32 [[X:%.*]]) {
 ; CHECK-NEXT:    [[R:%.*]] = srem i32 [[X]], 5
-; CHECK-NEXT:    [[C:%.*]] = icmp slt i32 [[R]], 0
+; CHECK-NEXT:    [[C:%.*]] = icmp ugt i32 [[R]], 4
 ; CHECK-NEXT:    ret i1 [[C]]
 ;
   %r = srem i32 %x, 5
@@ -89,7 +89,7 @@ define i1 @icmp_ult_srem5_m4(i32 %x) {
 ; CHECK-LABEL: define i1 @icmp_ult_srem5_m4(
 ; CHECK-SAME: i32 [[X:%.*]]) {
 ; CHECK-NEXT:    [[R:%.*]] = srem i32 [[X]], 5
-; CHECK-NEXT:    [[C:%.*]] = icmp sgt i32 [[R]], -1
+; CHECK-NEXT:    [[C:%.*]] = icmp ult i32 [[R]], -4
 ; CHECK-NEXT:    ret i1 [[C]]
 ;
   %r = srem i32 %x, 5
@@ -125,7 +125,7 @@ define i1 @icmp_ult_srem5_5(i32 %x) {
 ; CHECK-LABEL: define i1 @icmp_ult_srem5_5(
 ; CHECK-SAME: i32 [[X:%.*]]) {
 ; CHECK-NEXT:    [[R:%.*]] = srem i32 [[X]], 5
-; CHECK-NEXT:    [[C:%.*]] = icmp sgt i32 [[R]], -1
+; CHECK-NEXT:    [[C:%.*]] = icmp ult i32 [[R]], 5
 ; CHECK-NEXT:    ret i1 [[C]]
 ;
   %r = srem i32 %x, 5

```
https://alive2.llvm.org/ce/z/naNgve

https://github.com/llvm/llvm-project/pull/122520