Skip to content

Commit 377db1a

Browse files
author
Spenser Bauman
committed
[mlir][dataflow] Fix for integer range analysis propagation bug
Integer range analysis will not update the range of an operation when any of the inferred input lattices are uninitialized. In the current behavior, all lattice values for non integer types are uninitialized. For operations like arith.cmpf ```mlir %3 = arith.cmpf ugt, %arg0, %arg1 : f32 ``` that will result in the range of the output also being uninitialized, and so on for any consumer of the arith.cmpf result. When control-flow ops are involved, the lack of propagation results in incorrect ranges, as the back edges for loop carried values are not properly joined with the definitions from the body region. For example, an scf.while loop whose body region produces a value that is in a dataflow relationship with some floating-point values through an arith.cmpf operation: ```mlir func.func @test_bad_range(%arg0: f32, %arg1: f32) -> (index, index) { %c4 = arith.constant 4 : index %c1 = arith.constant 1 : index %c0 = arith.constant 0 : index %3 = arith.cmpf ugt, %arg0, %arg1 : f32 %1:2 = scf.while (%arg2 = %c0, %arg3 = %c0) : (index, index) -> (index, index) { %2 = arith.cmpi ult, %arg2, %c4 : index scf.condition(%2) %arg2, %arg3 : index, index } do { ^bb0(%arg2: index, %arg3: index): %4 = arith.select %3, %arg3, %arg3 : index %5 = arith.addi %arg2, %c1 : index scf.yield %5, %4 : index, index } return %1#0, %1#1 : index, index } ``` The existing behavior results in the control condition %2 being optimized to true, turning the while loop into an infinite loop. The update to %arg2 through the body region is never factored into the range calculation, as the ranges for the body ops all test as uninitialized. This change causes all values initialized with setToEntryState to be set to some initialized range, even if the values are not integers.
1 parent faef8b4 commit 377db1a

File tree

2 files changed

+18
-2
lines changed

2 files changed

+18
-2
lines changed

mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ using namespace mlir::dataflow;
3838

3939
IntegerValueRange IntegerValueRange::getMaxRange(Value value) {
4040
unsigned width = ConstantIntRanges::getStorageBitwidth(value.getType());
41-
if (width == 0)
42-
return {};
4341
APInt umin = APInt::getMinValue(width);
4442
APInt umax = APInt::getMaxValue(width);
4543
APInt smin = width != 0 ? APInt::getSignedMinValue(width) : umin;

mlir/test/Dialect/Arith/int-range-interface.mlir

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,3 +899,21 @@ func.func @test_shl_i8_nowrap() -> i8 {
899899
%2 = test.reflect_bounds %1 : i8
900900
return %2: i8
901901
}
902+
903+
/// A test case to ensure that the ranges for unsupported ops are initialized
904+
/// properly to maxRange, rather than left uninitialized.
905+
/// In this test case, the previous behavior would leave the ranges for %a and
906+
/// %b uninitialized, resulting in arith.cmpf's range not being updated, even
907+
/// though it has an integer valued result.
908+
909+
// CHECK-LABEL: func @test_cmpf_propagates
910+
// CHECK: test.reflect_bounds {smax = 2 : index, smin = 1 : index, umax = 2 : index, umin = 1 : index}
911+
func.func @test_cmpf_propagates(%a: f32, %b: f32) -> index {
912+
%c1 = arith.constant 1 : index
913+
%c2 = arith.constant 2 : index
914+
915+
%0 = arith.cmpf ueq, %a, %b : f32
916+
%1 = arith.select %0, %c1, %c2 : index
917+
%2 = test.reflect_bounds %1 : index
918+
func.return %2 : index
919+
}

0 commit comments

Comments
 (0)