-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[mlir][InferIntRangeCommon] Fix Division by Zero Crash #151637
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
Conversation
@llvm/pr-subscribers-mlir-arith Author: Veera (veera-sivarajan) ChangesFixes #131273 Adds a check to avoid division when max value of denominator is zero. Full diff: https://github.com/llvm/llvm-project/pull/151637.diff 2 Files Affected:
diff --git a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
index 2f47939df5a02..af4ea5ac1cec8 100644
--- a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
+++ b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
@@ -290,8 +290,7 @@ static ConstantIntRanges inferDivURange(const ConstantIntRanges &lhs,
DivisionFixupFn fixup) {
const APInt &lhsMin = lhs.umin(), &lhsMax = lhs.umax(), &rhsMin = rhs.umin(),
&rhsMax = rhs.umax();
-
- if (!rhsMin.isZero()) {
+ if (!rhsMin.isZero() && !rhsMax.isZero()) {
auto udiv = [&fixup](const APInt &a,
const APInt &b) -> std::optional<APInt> {
return fixup(a, b, a.udiv(b));
diff --git a/mlir/test/Dialect/Arith/int-range-interface.mlir b/mlir/test/Dialect/Arith/int-range-interface.mlir
index 2128d36f1a28e..130782ba9f525 100644
--- a/mlir/test/Dialect/Arith/int-range-interface.mlir
+++ b/mlir/test/Dialect/Arith/int-range-interface.mlir
@@ -224,6 +224,15 @@ func.func @ceil_divui(%arg0 : index) -> i1 {
func.return %7 : i1
}
+// CHECK-LABEL: func @ceil_divui_by_zero_issue_131273
+// CHECK-NEXT: return
+func.func @ceil_divui_by_zero_issue_131273() {
+ %0 = test.with_bounds {smax = 0 : i32, smin = -1 : i32, umax = 0 : i32, umin = -1 : i32} : i32
+ %c7_i32 = arith.constant 7 : i32
+ %1 = arith.ceildivui %c7_i32, %0 : i32
+ return
+}
+
// CHECK-LABEL: func @ceil_divsi
// CHECK: %[[ret:.*]] = arith.cmpi eq
// CHECK: return %[[ret]]
|
@llvm/pr-subscribers-mlir Author: Veera (veera-sivarajan) ChangesFixes #131273 Adds a check to avoid division when max value of denominator is zero. Full diff: https://github.com/llvm/llvm-project/pull/151637.diff 2 Files Affected:
diff --git a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
index 2f47939df5a02..af4ea5ac1cec8 100644
--- a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
+++ b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
@@ -290,8 +290,7 @@ static ConstantIntRanges inferDivURange(const ConstantIntRanges &lhs,
DivisionFixupFn fixup) {
const APInt &lhsMin = lhs.umin(), &lhsMax = lhs.umax(), &rhsMin = rhs.umin(),
&rhsMax = rhs.umax();
-
- if (!rhsMin.isZero()) {
+ if (!rhsMin.isZero() && !rhsMax.isZero()) {
auto udiv = [&fixup](const APInt &a,
const APInt &b) -> std::optional<APInt> {
return fixup(a, b, a.udiv(b));
diff --git a/mlir/test/Dialect/Arith/int-range-interface.mlir b/mlir/test/Dialect/Arith/int-range-interface.mlir
index 2128d36f1a28e..130782ba9f525 100644
--- a/mlir/test/Dialect/Arith/int-range-interface.mlir
+++ b/mlir/test/Dialect/Arith/int-range-interface.mlir
@@ -224,6 +224,15 @@ func.func @ceil_divui(%arg0 : index) -> i1 {
func.return %7 : i1
}
+// CHECK-LABEL: func @ceil_divui_by_zero_issue_131273
+// CHECK-NEXT: return
+func.func @ceil_divui_by_zero_issue_131273() {
+ %0 = test.with_bounds {smax = 0 : i32, smin = -1 : i32, umax = 0 : i32, umin = -1 : i32} : i32
+ %c7_i32 = arith.constant 7 : i32
+ %1 = arith.ceildivui %c7_i32, %0 : i32
+ return
+}
+
// CHECK-LABEL: func @ceil_divsi
// CHECK: %[[ret:.*]] = arith.cmpi eq
// CHECK: return %[[ret]]
|
ping @krzysz00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm stocking a hold on this because the scenario you're guarding against is a can't happen
// CHECK-LABEL: func @ceil_divui_by_zero_issue_131273 | ||
// CHECK-NEXT: return | ||
func.func @ceil_divui_by_zero_issue_131273() { | ||
%0 = test.with_bounds {smax = 0 : i32, smin = -1 : i32, umax = 0 : i32, umin = -1 : i32} : i32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix this rest so that umin <=(unsigned) umax?
// CHECK-LABEL: func @ceil_divui_by_zero_issue_131273 | ||
// CHECK-NEXT: return | ||
func.func @ceil_divui_by_zero_issue_131273() { | ||
%0 = test.with_bounds {smax = 0 : i32, smin = -1 : i32, umax = 0 : i32, umin = -1 : i32} : i32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix this test so that umin <=(unsigned) umax?
&rhsMax = rhs.umax(); | ||
|
||
if (!rhsMin.isZero()) { | ||
if (!rhsMin.isZero() && !rhsMax.isZero()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition should be strictly redundant - can you debug why you have this case?
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/24/builds/11660 Here is the relevant piece of the build log for the reference
|
Fixes #131273
Adds a check to avoid division when max value of denominator is zero.