[Mlir-commits] [mlir] 2a0c335 - [mlir][arith] Fold ceildivsi with MININT operands (#214637)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Aug 12 06:39:39 PDT 2026
Author: Hung Kuan Tseng
Date: 2026-08-12T09:39:34-04:00
New Revision: 2a0c335d4538ed8a2739c9b5e006b47652a6a8b0
URL: https://github.com/llvm/llvm-project/commit/2a0c335d4538ed8a2739c9b5e006b47652a6a8b0
DIFF: https://github.com/llvm/llvm-project/commit/2a0c335d4538ed8a2739c9b5e006b47652a6a8b0.diff
LOG: [mlir][arith] Fold ceildivsi with MININT operands (#214637)
`CeilDivSIOp::fold` computes the ceiling by negating operands so that
the
division runs on two non-negative values. Negating `MININT` overflows,
so the
folder gives up on any `MININT` operand, even when the result is
perfectly
representable:
```mlir
// i8, MININT = -128. ceil(-128 / 7) = -18, which fits, but does not fold.
%0 = arith.constant 7 : i8
%min = arith.constant -128 : i8
%1 = arith.ceildivsi %min, %0 : i8
```
The existing TODO on the folder mentions only a `MININT` dividend, but a
`MININT` divisor is affected the same way, since that operand gets
negated too.
Over all 65280 `i8` operand pairs, 507 do not fold today: 253 with a
`MININT`
dividend and **254 with a `MININT` divisor**. The divisor half has not
been
recorded anywhere so far.
Note this is a missed fold, not a miscompile. The current code is
correct, just
conservative; it bails rather than producing a wrong constant.
## Change
Compute the ceiling without negating anything. `sdiv` truncates towards
zero,
which already rounds up whenever the exact quotient is negative, so the
only
correction needed is `+1` when the operands share a sign and the
division is
inexact:
```c++
APInt quotient = a.sdiv_ov(b, overflowDiv);
if (overflowDiv) // MININT / -1
bail;
if (a.srem(b).isZero() || a.isNegative() != b.isNegative())
return quotient;
return quotient.sadd_ov(one, overflowOrDiv0);
```
This is the same shape `ExpandOps.cpp` already uses to expand the op, so
the
folder and the expansion now agree by construction rather than by
coincidence.
After the change the folder bails on exactly two things: division by
zero, as
before, and `MININT / -1`, whose result `-MININT` is not representable.
Every
other operand pair folds.
`signedCeilNonnegInputs` has no callers left and is removed. The four
quadrant-specific branches and their four overflow flags collapse into
one
path, for a net reduction of about 50 lines.
## Correctness
**Alive2** — the new algorithm against the expansion `arith-expand`
already
uses, both lowered from MLIR via `--convert-to-llvm | mlir-translate
--mlir-to-llvmir`: https://alive2.llvm.org/ce/z/Chnon4 ✓ *Transformation
seems
to be correct!*
The two differ only in how inexactness is tested, `srem != 0` versus
`a != (a sdiv b) * b`; the rest is identical. Alive2 treats division by
zero and
`MININT / -1` as poison, so the cases the folder bails on are covered.
**Exhaustive check against an oracle** — every operand pair, compared
against
exact ceiling division computed independently of any LLVM code:
| Width | Pairs | Result |
|---|---|---|
| `i4` | 240 | all agree; 1 unfolded (`-8 / -1`) |
| `i8` | 65280 | all agree; 1 unfolded (`-128 / -1`) |
Before and after, over the same `i8` sweep:
| | Unfolded | Folded to a wrong value |
|---|---|---|
| Before | 507 | 0 |
| After | **1** | 0 |
This pair of checks is deliberate: Alive2 proves the algorithm
symbolically but
against upstream's own expansion, while the oracle sweep compares
against
mathematical ceiling division, so a mistake shared by both
implementations would
still be caught.
## Range inference
`inferCeilDivS` in `InferIntRangeCommon.cpp` negates the quotient of
`MININT / [positive number]`, which is not signed ceiling division.
#116284 added
it so that the analysis agreed with what the op computed: the expansion
at the
time was `-(-a / b)`, and negating `MININT` is a noop, so that
expression
returned a positive value. #121062 then unioned in the range of
`[MININT + 1, smax]` whenever the dividend range starts at `MININT`, to
cover the
discontinuity the first workaround introduces.
#133774 replaced that expansion with the mathematical ceiling in April
2025, and
both workarounds were left behind. On `main`, with none of this PR
applied, the
two passes answer differently for the same op:
```
$ mlir-opt ceildiv.mlir -arith-expand -canonicalize
%c-7754212542_i64 = arith.constant -7754212542 : i64
$ mlir-opt ranges.mlir -int-range-optimizations
// cmpi eq, ceildivsi(INT64_MIN, 1189465982), 7754212542 -> true
// cmpi eq, ceildivsi(INT64_MIN, 1189465982), -7754212542 -> false
```
Folding the `MININT` cases adds a third reading of the same op, so the
third
commit drops both workarounds. What is left has the same shape as
`inferFloorDivS`: `inferDivSRange` only evaluates at the range
endpoints, which
bounds `ceildivsi` because it is monotonic in each operand once the
divisor
cannot change sign -- and `inferDivSRange` already gives up unless it
cannot.
With the negation gone there is no discontinuity for the second
workaround to
patch up, which `@ceil_divsi_full_range`, the regression test added with
it,
pins: it still does not fold.
Afterwards the three implementations agree on that op:
`-test-single-fold` and
`-arith-expand -canonicalize` both fold it to `-7754212542`, and
`-int-range-optimizations` proves it equal to `-7754212542` and not
equal to
`7754212542`.
## Tests
The first commit pre-commits the missing `MININT` divisor cases, plus
`MININT / -1`, with check lines showing current behaviour. The second
commit
contains the functional change and the resulting check-line diffs.
This **updates an existing test**: `@simple_arith.ceildivsi_overflow`
asserted
that `MININT` dividends do not fold, and they now fold to `-18`, `-4681`
and
`-306783378`. It is renamed to `@simple_arith.ceildivsi_minint_dividend`
since
it no longer describes a bail-out, and the TODO it carried, "The folder
should
be able to fold the following by avoiding intermediate operations that
overflow", is resolved and removed.
The third commit renames `@ceil_divsi_intmin_bug_115293` in
`int-range-interface.mlir`, which expected `7754212542`, to
`@ceil_divsi_intmin`
expecting `-7754212542`, and adds `@ceil_divsi_intmin_dividend_range`
for a
dividend range that starts at `MININT`.
The fourth commit adds `@simple_arith.ceildivsi_vector` to
`constant-fold.mlir`.
The folder uses one overflow flag for the whole fold, so a single
element whose
result is not representable discards the result for every element, and
nothing
pinned that -- the `MININT / -1` tests are all scalar. It pairs a vector
that
folds with two that must not, differing only in whether the `MININT /
-1` element
comes first or last.
`ninja check-mlir` is 3848 passed / 0 failed, and `git clang-format` is
clean.
## Prior art
PR #90855 attempted the mixed-sign part of this in 2024 but stalled on a
request
for exactly this kind of analysis; it has been inactive since and its
base has
drifted considerably. cc @bviyer in case there is interest.
Added:
Modified:
mlir/lib/Dialect/Arith/IR/ArithOps.cpp
mlir/test/Transforms/constant-fold.mlir
Removed:
################################################################################
diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 479c4737f3214..1b7ef01ec64ac 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -913,18 +913,6 @@ Speculation::Speculatability arith::DivSIOp::getSpeculatability() {
return getDivSISpeculatability(getRhs());
}
-//===----------------------------------------------------------------------===//
-// Ceil and floor division folding helpers
-//===----------------------------------------------------------------------===//
-
-static APInt signedCeilNonnegInputs(const APInt &a, const APInt &b,
- bool &overflow) {
- // Returns (a-1)/b + 1
- APInt one(a.getBitWidth(), 1, true); // Signed value 1.
- APInt val = a.ssub_ov(one, overflow).sdiv_ov(b, overflow);
- return val.sadd_ov(one, overflow);
-}
-
//===----------------------------------------------------------------------===//
// CeilDivUIOp
//===----------------------------------------------------------------------===//
@@ -989,56 +977,36 @@ OpFoldResult arith::CeilDivSIOp::fold(FoldAdaptor adaptor) {
return getIntegerAttrOfType(getType(), 1);
// Don't fold if it would overflow or if it requires a division by zero.
- // TODO: This hook won't fold operations where a = MININT, because
- // negating MININT overflows. This can be improved.
bool overflowOrDiv0 = false;
auto result = constFoldBinaryOp<IntegerAttr>(
- adaptor.getOperands(), [&](APInt a, const APInt &b) {
+ adaptor.getOperands(), [&](const APInt &a, const APInt &b) {
if (overflowOrDiv0 || !b) {
overflowOrDiv0 = true;
return a;
}
- if (!a)
- return a;
- // After this point we know that neither a or b are zero.
- unsigned bits = a.getBitWidth();
- APInt zero = APInt::getZero(bits);
- bool aGtZero = a.sgt(zero);
- bool bGtZero = b.sgt(zero);
- if (aGtZero && bGtZero) {
- // Both positive, return ceil(a, b).
- return signedCeilNonnegInputs(a, b, overflowOrDiv0);
- }
-
- // No folding happens if any of the intermediate arithmetic operations
- // overflows.
- bool overflowNegA = false;
- bool overflowNegB = false;
+ // Compute the ceiling without negating either operand, so that MININT
+ // operands still fold whenever the result is representable.
+ //
+ // sdiv truncates towards zero, so it already rounds up whenever the
+ // exact quotient is negative. When the exact quotient is positive, i.e.
+ // when the operands have the same sign, an inexact division has to be
+ // corrected by one. This mirrors the expansion in ExpandOps.cpp.
bool overflowDiv = false;
- bool overflowNegRes = false;
- if (!aGtZero && !bGtZero) {
- // Both negative, return ceil(-a, -b).
- APInt posA = zero.ssub_ov(a, overflowNegA);
- APInt posB = zero.ssub_ov(b, overflowNegB);
- APInt res = signedCeilNonnegInputs(posA, posB, overflowDiv);
- overflowOrDiv0 = (overflowNegA || overflowNegB || overflowDiv);
- return res;
- }
- if (!aGtZero && bGtZero) {
- // A is negative, b is positive, return - ( -a / b).
- APInt posA = zero.ssub_ov(a, overflowNegA);
- APInt div = posA.sdiv_ov(b, overflowDiv);
- APInt res = zero.ssub_ov(div, overflowNegRes);
- overflowOrDiv0 = (overflowNegA || overflowDiv || overflowNegRes);
- return res;
+ APInt quotient = a.sdiv_ov(b, overflowDiv);
+ if (overflowDiv) {
+ // MININT / -1. The exact result is -MININT, which is not
+ // representable.
+ overflowOrDiv0 = true;
+ return a;
}
- // A is positive, b is negative, return - (a / -b).
- APInt posB = zero.ssub_ov(b, overflowNegB);
- APInt div = a.sdiv_ov(posB, overflowDiv);
- APInt res = zero.ssub_ov(div, overflowNegRes);
+ if (a.isNegative() != b.isNegative() || quotient * b == a)
+ return quotient;
- overflowOrDiv0 = (overflowNegB || overflowDiv || overflowNegRes);
- return res;
+ // The correction cannot overflow: it only applies when the exact
+ // quotient is positive and the division is inexact, which bounds the
+ // quotient well below the maximum. Check anyway, at no cost.
+ APInt one(a.getBitWidth(), 1, /*isSigned=*/true);
+ return quotient.sadd_ov(one, overflowOrDiv0);
});
return overflowOrDiv0 ? Attribute() : result;
diff --git a/mlir/test/Transforms/constant-fold.mlir b/mlir/test/Transforms/constant-fold.mlir
index 0b393bf0556b9..589a7cedb8a8f 100644
--- a/mlir/test/Transforms/constant-fold.mlir
+++ b/mlir/test/Transforms/constant-fold.mlir
@@ -478,35 +478,26 @@ func.func @simple_arith.ceildivsi() -> (i32, i32, i32, i32, i32) {
// -----
-// CHECK-LABEL: func @simple_arith.ceildivsi_overflow
-func.func @simple_arith.ceildivsi_overflow() -> (i8, i16, i32) {
- // The negative values below are MININTs for the corresponding bit-width. The
- // folder will try to negate them (so that the division operates on two
- // positive numbers), but that would cause overflow (negating MININT
- // overflows). Hence folding should not happen and the original ceildivsi is
- // preserved.
-
- // TODO: The folder should be able to fold the following by avoiding
- // intermediate operations that overflow.
-
- // CHECK-DAG: %[[C_1:.*]] = arith.constant 7 : i8
- // CHECK-DAG: %[[MIN_I8:.*]] = arith.constant -128 : i8
- // CHECK-DAG: %[[C_2:.*]] = arith.constant 7 : i16
- // CHECK-DAG: %[[MIN_I16:.*]] = arith.constant -32768 : i16
- // CHECK-DAG: %[[C_3:.*]] = arith.constant 7 : i32
- // CHECK-DAG: %[[MIN_I32:.*]] = arith.constant -2147483648 : i32
-
- // CHECK-NEXT: %[[CEILDIV_1:.*]] = arith.ceildivsi %[[MIN_I8]], %[[C_1]] : i8
+// The dividends below are MININTs for the corresponding bit-width. Every
+// result is representable, so all of them fold.
+
+// CHECK-LABEL: func @simple_arith.ceildivsi_minint_dividend
+// CHECK-DAG: %[[CEILDIV_1:.*]] = arith.constant -18 : i8
+// CHECK-DAG: %[[CEILDIV_2:.*]] = arith.constant -4681 : i16
+// CHECK-DAG: %[[CEILDIV_3:.*]] = arith.constant -306783378 : i32
+// CHECK: return %[[CEILDIV_1]], %[[CEILDIV_2]], %[[CEILDIV_3]]
+func.func @simple_arith.ceildivsi_minint_dividend() -> (i8, i16, i32) {
+ // ceil(-128 / 7) = -18
%0 = arith.constant 7 : i8
%min_int_i8 = arith.constant -128 : i8
%2 = arith.ceildivsi %min_int_i8, %0 : i8
- // CHECK-NEXT: %[[CEILDIV_2:.*]] = arith.ceildivsi %[[MIN_I16]], %[[C_2]] : i16
+ // ceil(-32768 / 7) = -4681
%3 = arith.constant 7 : i16
%min_int_i16 = arith.constant -32768 : i16
%5 = arith.ceildivsi %min_int_i16, %3 : i16
- // CHECK-NEXT: %[[CEILDIV_2:.*]] = arith.ceildivsi %[[MIN_I32]], %[[C_3]] : i32
+ // ceil(-2147483648 / 7) = -306783378
%6 = arith.constant 7 : i32
%min_int_i32 = arith.constant -2147483648 : i32
%8 = arith.ceildivsi %min_int_i32, %6 : i32
@@ -516,6 +507,84 @@ func.func @simple_arith.ceildivsi_overflow() -> (i8, i16, i32) {
// -----
+// The divisor, rather than the dividend, is MININT here.
+
+// CHECK-LABEL: func @simple_arith.ceildivsi_minint_divisor
+// CHECK-DAG: %[[C_0:.*]] = arith.constant 0 : i8
+// CHECK-DAG: %[[C_1:.*]] = arith.constant 1 : i8
+// CHECK: return %[[C_0]], %[[C_1]], %[[C_1]]
+func.func @simple_arith.ceildivsi_minint_divisor() -> (i8, i8, i8) {
+ %min_int_i8 = arith.constant -128 : i8
+ %0 = arith.constant 7 : i8
+ %1 = arith.constant -9 : i8
+
+ // ceil(7 / -128) = 0
+ %2 = arith.ceildivsi %0, %min_int_i8 : i8
+ // ceil(-9 / -128) = 1
+ %3 = arith.ceildivsi %1, %min_int_i8 : i8
+ // ceil(-128 / -128) = 1, already folded by the ceildivsi(x, x) -> 1 pattern.
+ %4 = arith.ceildivsi %min_int_i8, %min_int_i8 : i8
+
+ return %2, %3, %4 : i8, i8, i8
+}
+
+// -----
+
+// ceil(MININT / -1) is -MININT, which is not representable. Unlike the cases
+// above, these must never fold.
+
+// CHECK-LABEL: func @simple_arith.ceildivsi_minint_div_minus_one
+// CHECK: arith.ceildivsi
+// CHECK-NEXT: arith.ceildivsi
+// CHECK-NEXT: arith.ceildivsi
+func.func @simple_arith.ceildivsi_minint_div_minus_one() -> (i8, i16, i32) {
+ %min_int_i8 = arith.constant -128 : i8
+ %0 = arith.constant -1 : i8
+ %1 = arith.ceildivsi %min_int_i8, %0 : i8
+
+ %min_int_i16 = arith.constant -32768 : i16
+ %2 = arith.constant -1 : i16
+ %3 = arith.ceildivsi %min_int_i16, %2 : i16
+
+ %min_int_i32 = arith.constant -2147483648 : i32
+ %4 = arith.constant -1 : i32
+ %5 = arith.ceildivsi %min_int_i32, %4 : i32
+
+ return %1, %3, %5 : i8, i16, i32
+}
+
+// -----
+
+// One overflow flag is shared by every element of a vector fold, so a single
+// element whose result is not representable discards the whole fold, wherever
+// in the vector it sits.
+
+// CHECK-LABEL: func @simple_arith.ceildivsi_vector
+// CHECK-DAG: %[[FOLDED:.*]] = arith.constant dense<[-18, 1, -1]> : vector<3xi8>
+// CHECK: %[[LAST:.*]] = arith.ceildivsi
+// CHECK-NEXT: %[[FIRST:.*]] = arith.ceildivsi
+// CHECK-NEXT: return %[[FOLDED]], %[[LAST]], %[[FIRST]]
+func.func @simple_arith.ceildivsi_vector() -> (vector<3xi8>, vector<3xi8>, vector<3xi8>) {
+ // ceil(-128 / 7) = -18, ceil(-9 / -128) = 1, ceil(5 / -3) = -1
+ %0 = arith.constant dense<[-128, -9, 5]> : vector<3xi8>
+ %1 = arith.constant dense<[7, -128, -3]> : vector<3xi8>
+
+ // MININT / -1 as the last element, then as the first, so that the flag is
+ // set both after and before the representable elements are visited.
+ %2 = arith.constant dense<[-128, -9, -128]> : vector<3xi8>
+ %3 = arith.constant dense<[7, -128, -1]> : vector<3xi8>
+ %4 = arith.constant dense<[-128, -128, -9]> : vector<3xi8>
+ %5 = arith.constant dense<[-1, 7, -128]> : vector<3xi8>
+
+ %6 = arith.ceildivsi %0, %1 : vector<3xi8>
+ %7 = arith.ceildivsi %2, %3 : vector<3xi8>
+ %8 = arith.ceildivsi %4, %5 : vector<3xi8>
+
+ return %6, %7, %8 : vector<3xi8>, vector<3xi8>, vector<3xi8>
+}
+
+// -----
+
// CHECK-LABEL: func @simple_arith.ceildivui
func.func @simple_arith.ceildivui() -> (i32, i32, i32, i32, i32) {
// CHECK-DAG: [[C0:%.+]] = arith.constant 0
More information about the Mlir-commits
mailing list