[Mlir-commits] [mlir] [mlir][IntRangeInference] Handle ceildivsi(INT_MIN, x > 1) as expected (PR #116284)

[Krzysztof Drewniak]

https://github.com/krzysz00 updated https://github.com/llvm/llvm-project/pull/116284

>From 0ec5b1cdae54404868b487263f866dc72944d9d8 Mon Sep 17 00:00:00 2001
From: Krzysztof Drewniak <Krzysztof.Drewniak at amd.com>
Date: Thu, 14 Nov 2024 21:46:51 +0000
Subject: [PATCH 1/2] [mlir][IntRangeInference] Handle ceildivsi(INT_MIN, x >
 1) as expected

Fixes #115293

While the definition of ceildivsi is integer division, rounding up,
most implementations will use `-(-a / b)` for dividing `a ceildiv b`
with `a` negative and `b` positive.

Mathematically, and for most integers, these two definitions are
equivalent. However, with `a == INT_MIN`, the initial negation is a
noop, which means that, while divinding and rounding up would give a
negative result, `-((- INT_MIN) / b)` is `-(INT_MIN / b)`, which is
positive.

This commit adds a special case to ceilDivSI inference to handle this
case and bring it in line with the operational instead of the
mathematical semantics of ceiling division.
---
 mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp |  9 +++++++++
 mlir/test/Dialect/Arith/int-range-interface.mlir  | 13 +++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
index c5610ba5d3c0bc..0b085b10b2b337 100644
--- a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
+++ b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
@@ -375,6 +375,15 @@ mlir::intrange::inferCeilDivS(ArrayRef<ConstantIntRanges> argRanges) {
           result.sadd_ov(APInt(result.getBitWidth(), 1), overflowed);
       return overflowed ? std::optional<APInt>() : corrected;
     }
+    // Special case where the usual implementation of ceilDiv causes
+    // INT_MIN / [positive number] to be positive. This doesn't match the
+    // definition of signed ceiling division mathematically, but it prevents
+    // inconsistent constant-folding results. This arises because (-int_min) is
+    // still negative, so -(-int_min / b) is -(int_min / b), which is
+    // positive See #115293.
+    if (lhs.isMinSignedValue() && rhs.sgt(1)) {
+      return -result;
+    }
     return result;
   };
   return inferDivSRange(lhs, rhs, ceilDivSIFix);
diff --git a/mlir/test/Dialect/Arith/int-range-interface.mlir b/mlir/test/Dialect/Arith/int-range-interface.mlir
index 6d66da2fc1eb35..235d61f2f042ac 100644
--- a/mlir/test/Dialect/Arith/int-range-interface.mlir
+++ b/mlir/test/Dialect/Arith/int-range-interface.mlir
@@ -249,6 +249,19 @@ func.func @ceil_divsi(%arg0 : index) -> i1 {
     func.return %10 : i1
 }
 
+// CHECK-LABEL: func @ceil_divsi_intmin_bug_115293
+// CHECK: %[[ret:.*]] = arith.constant true
+// CHECK: return %[[ret]]
+func.func @ceil_divsi_intmin_bug_115293() -> i1 {
+    %cIntMin_i64 = arith.constant -9223372036854775808 : i64
+    %cDenom_i64 = arith.constant 1189465982 : i64
+    %cRes_i64 = arith.constant 7754212542 : i64
+
+    %0 = arith.ceildivsi %cIntMin_i64, %cDenom_i64 : i64
+    %1 = arith.cmpi eq, %0, %cRes_i64 : i64
+    func.return %1 : i1
+}
+
 // CHECK-LABEL: func @floor_divsi
 // CHECK: %[[true:.*]] = arith.constant true
 // CHECK: return %[[true]]

>From 62639ebd5047acb954a1eb3b2138f60e91720ad5 Mon Sep 17 00:00:00 2001
From: Krzysztof Drewniak <Krzysztof.Drewniak at amd.com>
Date: Thu, 14 Nov 2024 23:12:27 +0000
Subject: [PATCH 2/2] Make one of the constants a with_bounds to prevent
 folding

---
 mlir/test/Dialect/Arith/int-range-interface.mlir | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mlir/test/Dialect/Arith/int-range-interface.mlir b/mlir/test/Dialect/Arith/int-range-interface.mlir
index 235d61f2f042ac..5189d7a3a92245 100644
--- a/mlir/test/Dialect/Arith/int-range-interface.mlir
+++ b/mlir/test/Dialect/Arith/int-range-interface.mlir
@@ -253,6 +253,8 @@ func.func @ceil_divsi(%arg0 : index) -> i1 {
 // CHECK: %[[ret:.*]] = arith.constant true
 // CHECK: return %[[ret]]
 func.func @ceil_divsi_intmin_bug_115293() -> i1 {
+    %intMin_i64 = test.with_bounds { smin = -9223372036854775808 : si64, smax = -9223372036854775808 : si64, umin = 9223372036854775808 : ui64, umax = 9223372036854775808 : ui64 } : i64
+
     %cIntMin_i64 = arith.constant -9223372036854775808 : i64
     %cDenom_i64 = arith.constant 1189465982 : i64
     %cRes_i64 = arith.constant 7754212542 : i64