[Mlir-commits] [mlir] [mlir] Compute ceildivs consistently for INT_MIN operands (PR #215696)

[Hung Kuan Tseng]

================
@@ -245,27 +245,29 @@ OpFoldResult DivUOp::fold(FoldAdaptor adaptor) {
 // CeilDivSOp
 //===----------------------------------------------------------------------===//
 
-/// Compute `ceildivs(n, m)` as `x = m > 0 ? -1 : 1` and then
-/// `n*m > 0 ? (n+x)/m + 1 : -(-n/m)`.
+/// Compute `ceildivs(n, m)` as `z = n / m` and then
+/// `z*m != n && (n < 0) == (m < 0) ? z + 1 : z`.
 static std::optional<APInt> calculateCeilDivS(const APInt &n, const APInt &m) {
   // Don't fold division by zero.
   if (m.isZero())
     return std::nullopt;
-  // Short-circuit the zero case.
-  if (n.isZero())
-    return n;
 
-  bool mGtZ = m.sgt(0);
-  if (n.sgt(0) != mGtZ) {
-    // If the operands have different signs, compute the negative result. Signed
-    // division overflow is not possible, since if `m == -1`, `n` can be at most
-    // `INT_MAX`, and `-INT_MAX != INT_MIN` in two's complement.
-    return -(-n).sdiv(m);
-  }
-  // Otherwise, compute the positive result. Signed division overflow is not
-  // possible since if `m == -1`, `x` will be `1`.
-  int64_t x = mGtZ ? -1 : 1;
-  return (n + x).sdiv(m) + 1;
+  // sdiv truncates towards zero, so it already rounds up whenever the exact
+  // quotient is negative. Neither operand is negated, so `INT_MIN` folds like
+  // any other dividend.
+  bool overflowed = false;
+  APInt quotient = n.sdiv_ov(m, overflowed);
----------------
Tim096 wrote:

Done. `RoundingSDiv` with `Rounding::UP` tests `Rem.isNegative() != B.isNegative()`, and `sdivrem` gives the remainder the sign of the dividend, so that is the same same-sign condition the hand-rolled version tested. Dropping my `sadd_ov` on the correction is fine for the reason `RoundingSDiv` leaves its own `Quo + 1` unguarded: a quotient of `INT_MAX` needs `m` to be `1` or `-1`, and both of those divide exactly, so the correction never fires on one.

One deviation from what you wrote, now called out in the commit message: I spelled the rejection as the bit test `sdiv_ov` itself performs rather than calling `sdiv_ov` and discarding the quotient, since `RoundingSDiv` already divides internally and the fold has no reason to divide twice. Say the word if you would rather the guard name the helper it came from.


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