[Mlir-commits] [mlir] [mlir][Index] Fix INT_MIN overflow in CeilDivSOp constant folding (PR #223127)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-index

Author: Cheng-Chieh Liu (jay87406)

<details>
<summary>Changes</summary>

Fix a silent overflow in `calculateCeilDivS` caused by negating `INT_MIN` via `-(-n).sdiv(m)`, which wraps back and yields incorrect signs/magnitudes (e.g., `ceildivs(INT_MIN, 2)` folding to `4611686018427387904` instead of `-4611686018427387904`).

This replaces the negation logic with `sdiv_ov` and `sadd_ov`, matching `arith::CeilDivSIOp::fold`. It now safely folds `INT_MIN` when representable and returns `std::nullopt` on true overflows (like `INT_MIN / -1`).

Updated `ceildivs_edge` since the previous test for 32-bit truncated `INT32_MIN` only passed due to identical overflow behavior. Added `ceildivs_intmin64` to explicitly cover the fixed scenario.

Verified locally with `ninja mlir-opt && llvm-lit mlir/test/Dialect/Index/`.

---
Full diff: https://github.com/llvm/llvm-project/pull/223127.diff


2 Files Affected:

- (modified) mlir/lib/Dialect/Index/IR/IndexOps.cpp (+16-13) 
- (modified) mlir/test/Dialect/Index/index-canonicalize.mlir (+20-2) 


``````````diff
diff --git a/mlir/lib/Dialect/Index/IR/IndexOps.cpp b/mlir/lib/Dialect/Index/IR/IndexOps.cpp
index 2b1baa8b44643..92f68bade1ee8 100644
--- a/mlir/lib/Dialect/Index/IR/IndexOps.cpp
+++ b/mlir/lib/Dialect/Index/IR/IndexOps.cpp
@@ -245,8 +245,9 @@ 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)` via `sdiv_ov`/`sadd_ov` so that neither operand
+/// is negated, letting `MININT` fold correctly instead of silently
+/// overflowing (mirrors `arith::CeilDivSIOp::fold`).
 static std::optional<APInt> calculateCeilDivS(const APInt &n, const APInt &m) {
   // Don't fold division by zero.
   if (m.isZero())
@@ -255,17 +256,19 @@ static std::optional<APInt> calculateCeilDivS(const APInt &n, const APInt &m) {
   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;
+  bool overflow = false;
+  APInt quotient = n.sdiv_ov(m, overflow);
+  if (overflow) // MININT / -1, not representable.
+    return std::nullopt;
+  // sdiv already rounds towards the ceiling for a negative quotient; a
+  // positive, inexact quotient needs a +1 correction.
+  if (n.isNegative() != m.isNegative() || quotient * m == n)
+    return quotient;
+
+  bool addOverflow = false;
+  APInt result = quotient.sadd_ov(APInt(n.getBitWidth(), 1, /*isSigned=*/true),
+                                   addOverflow);
+  return addOverflow ? std::optional<APInt>() : result;
 }
 
 OpFoldResult CeilDivSOp::fold(FoldAdaptor adaptor) {
diff --git a/mlir/test/Dialect/Index/index-canonicalize.mlir b/mlir/test/Dialect/Index/index-canonicalize.mlir
index 45da6ea57d796..f161969584495 100644
--- a/mlir/test/Dialect/Index/index-canonicalize.mlir
+++ b/mlir/test/Dialect/Index/index-canonicalize.mlir
@@ -165,8 +165,8 @@ func.func @ceildivs_edge() -> (index, index) {
   %cIntMin = index.constant -2147483648
   %cIntMax = index.constant 2147483647
 
-  // The result is 0 on 32-bit.
-  // CHECK-DAG: %[[A:.*]] = index.constant 2147483648
+  // `INT_MIN / -1` overflows on a 32-bit index, so this must not fold.
+  // CHECK-DAG: %[[A:.*]] = index.ceildivs
   %0 = index.ceildivs %cIntMin, %cn1
 
   // CHECK-DAG: %[[B:.*]] = index.constant -2147483647
@@ -176,6 +176,24 @@ func.func @ceildivs_edge() -> (index, index) {
   return %0, %1 : index, index
 }
 
+// CHECK-LABEL: @ceildivs_intmin64
+func.func @ceildivs_intmin64() -> (index, index) {
+  %cIntMin = index.constant -9223372036854775808
+  %cn1 = index.constant -1
+  %c2 = index.constant 2
+
+  // `INT64_MIN / -1` is not representable, so this must not fold.
+  // CHECK-DAG: %[[A:.*]] = index.ceildivs
+  %0 = index.ceildivs %cIntMin, %cn1
+
+  // Must fold to the correct negative result, not a sign-flipped positive one.
+  // CHECK-DAG: %[[B:.*]] = index.constant -4611686018427387904
+  %1 = index.ceildivs %cIntMin, %c2
+
+  // CHECK: return %[[A]], %[[B]]
+  return %0, %1 : index, index
+}
+
 // CHECK-LABEL: @ceildivu
 func.func @ceildivu() -> index {
   %0 = index.constant 0x200000001

``````````

</details>


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