[llvm] Simplify `(a % b) lt/ge (b-1)` into `(a % b) eq/ne (b-1)` (PR #72504)

[Nikita Popov via llvm-commits]

================
@@ -2519,6 +2519,30 @@ Instruction *InstCombinerImpl::foldICmpShrConstant(ICmpInst &Cmp,
 Instruction *InstCombinerImpl::foldICmpSRemConstant(ICmpInst &Cmp,
                                                     BinaryOperator *SRem,
                                                     const APInt &C) {
+  {
+    const APInt *C1;
+    ICmpInst::Predicate Pred = Cmp.getPredicate();
+    if ((match(SRem->getOperand(1), m_NonNegative(C1)) &&
+         ((Pred == ICmpInst::ICMP_SLT && C == *C1 - 1) ||
+          (Pred == ICmpInst::ICMP_SGT && C == *C1 - 2))) ||
+        (match(SRem->getOperand(1), m_Negative(C1)) &&
+         ((Pred == ICmpInst::ICMP_SGT && C == *C1 + 1) ||
+          (Pred == ICmpInst::ICMP_SLT && C == *C1 + 2)))) {
----------------
nikic wrote:

Sorry to bring this up only now, but playing around with this a bit, I don't think the way negative numbers are handled here makes sense. The thing is that `srem %x, -C` will be canonicalized to `srem %x, C` (the sign of the srem result is determined by the first operand, not the second one!) so we will never actually hit the `m_Negative()` branch.

We should still keep the `m_NonNegative(C1)` due to worklist order considerations, but the `m_Negative(C1)` branch can be dropped.

Instead, we'd want to add more cases to the positive case, such as `(srem %x, C) sgt (-C + 1)`. This would be one of the new cases: https://alive2.llvm.org/ce/z/MeiohD Same for slt and +2.

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