Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][IntRangeInference] Fix arith.ceildivsi range inference when it includes INT_MIN #121062

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Hardcode84
Copy link
Contributor

There is a special case in arith.ceildivsi range inference for handling lhs.smin()==INT_MIN, but when lhs is not a single value, it can cause it to skip entire negative range. Add lhs.smin() + 1 check to handle it.

…it includes INT_MIN

There is a special case in `arith.ceildivsi` range inference for handling `lhs.smin()==INT_MIN`, but when
`lhs` is not a single value, it can cause it to skip entire negative range. Add `lhs.smin() + 1` check to handle it.
@llvmbot
Copy link
Member

llvmbot commented Dec 24, 2024

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-mlir

Author: Ivan Butygin (Hardcode84)

Changes

There is a special case in arith.ceildivsi range inference for handling lhs.smin()==INT_MIN, but when lhs is not a single value, it can cause it to skip entire negative range. Add lhs.smin() + 1 check to handle it.


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

2 Files Affected:

  • (modified) mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp (+9-1)
  • (modified) mlir/test/Dialect/Arith/int-range-interface.mlir (+12)
diff --git a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
index 7a73a94201f1d6..1eab4139488bdd 100644
--- a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
+++ b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
@@ -386,7 +386,15 @@ mlir::intrange::inferCeilDivS(ArrayRef<ConstantIntRanges> argRanges) {
     }
     return result;
   };
-  return inferDivSRange(lhs, rhs, ceilDivSIFix);
+  ConstantIntRanges result = inferDivSRange(lhs, rhs, ceilDivSIFix);
+  if (lhs.smin().isMinSignedValue() && lhs.smax().sgt(lhs.smin())) {
+    // If lhs range includes INT_MIN and lhs is not a single value, we can
+    // suddenly wrap to positive val, skipping entire negative range, add
+    // [INT_MIN + 1, smax()] range to the result to handle this.
+    auto newLhs = ConstantIntRanges::fromSigned(lhs.smin() + 1, lhs.smax());
+    result = result.rangeUnion(inferDivSRange(newLhs, rhs, ceilDivSIFix));
+  }
+  return result;
 }
 
 ConstantIntRanges
diff --git a/mlir/test/Dialect/Arith/int-range-interface.mlir b/mlir/test/Dialect/Arith/int-range-interface.mlir
index 48a3eb20eb7fb0..090af3e79f4a10 100644
--- a/mlir/test/Dialect/Arith/int-range-interface.mlir
+++ b/mlir/test/Dialect/Arith/int-range-interface.mlir
@@ -249,6 +249,18 @@ func.func @ceil_divsi(%arg0 : index) -> i1 {
     func.return %10 : i1
 }
 
+// There was a bug, which was causing this expr errorneously fold to constant
+// CHECK-LABEL: func @ceil_divsi_full_range
+// CHECK-SAME: (%[[arg:.*]]: index)
+// CHECK: %[[c64:.*]] = arith.constant 64 : index
+// CHECK: %[[ret:.*]] = arith.ceildivsi %[[arg]], %[[c64]] : index
+// CHECK: return %[[ret]]
+func.func @ceil_divsi_full_range(%6: index) -> index {
+  %c64 = arith.constant 64 : index
+  %55 = arith.ceildivsi %6, %c64 : index
+  return %55 : index
+}
+
 // CHECK-LABEL: func @ceil_divsi_intmin_bug_115293
 // CHECK: %[[ret:.*]] = arith.constant true
 // CHECK: return %[[ret]]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants