[mlir][intrange] Fix inference of zero-trip loop bound

[ubfx]

When lower bound and exclusive upper bound of a loop are the same, and the zero-trip loop is not canonicalized away before the analysis, this leads to a meaningless range for the induction variable being inferred. This patch adds a check to make sure that the inferred range for the IV is meaningful before updating the analysis state.

Fix #94423

[llvmbot]

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-mlir

Author: Felix Schneider (ubfx)

Changes

When lower bound and exclusive upper bound of a loop are the same, and the zero-trip loop is not canonicalized away before the analysis, this leads to a meaningless range for the induction variable being inferred. This patch adds a check to make sure that the inferred range for the IV is meaningful before updating the analysis state.

Fix #94423

---

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

2 Files Affected:

• (modified) mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp (+8-3)
• (modified) mlir/test/Dialect/Arith/int-range-interface.mlir (+18)

diff --git a/mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp b/mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp
index 9721620807a0f..244ce8b9c2ac6 100644
--- a/mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp
+++ b/mlir/lib/Analysis/DataFlow/IntegerRangeAnalysis.cpp
@@ -195,9 +195,14 @@ void IntegerRangeAnalysis::visitNonControlFlowArguments(
       max -= 1;
     }
 
-    IntegerValueRangeLattice *ivEntry = getLatticeElement(*iv);
-    auto ivRange = ConstantIntRanges::fromSigned(min, max);
-    propagateIfChanged(ivEntry, ivEntry->join(IntegerValueRange{ivRange}));
+    // If we infer the lower bound to be larger than the upper bound, the
+    // resulting range is meaningless and should not be used in further
+    // inferences.
+    if (max.sge(min)) {
+      IntegerValueRangeLattice *ivEntry = getLatticeElement(*iv);
+      auto ivRange = ConstantIntRanges::fromSigned(min, max);
+      propagateIfChanged(ivEntry, ivEntry->join(IntegerValueRange{ivRange}));
+    }
     return;
   }
 
diff --git a/mlir/test/Dialect/Arith/int-range-interface.mlir b/mlir/test/Dialect/Arith/int-range-interface.mlir
index e00b7692fe396..48fdb1cdced4d 100644
--- a/mlir/test/Dialect/Arith/int-range-interface.mlir
+++ b/mlir/test/Dialect/Arith/int-range-interface.mlir
@@ -918,3 +918,21 @@ func.func @test_cmpf_propagates(%a: f32, %b: f32) -> index {
   func.return %2 : index
 }
 
+// CHECK-LABEL: func @zero_trip_loop
+func.func @zero_trip_loop() {
+  %idx1 = arith.constant 1 : index
+  scf.for %arg0 = %idx1 to %idx1 step %idx1 {
+    %138 = index.floordivs %arg0, %arg0
+  }
+  return
+}
+
+// CHECK-LABEL: func @zero_trip_loop2
+func.func @zero_trip_loop2() {
+  %idx1 = arith.constant 1 : index
+  %idxm1 = arith.constant -1 : index
+  scf.for %arg0 = %idx1 to %idx1 step %idxm1 {
+    %138 = index.floordivs %arg0, %arg0
+  }
+  return
+}

[krzysz00]

Approved, thanks!

Also, I'm now thinking we might want to move to a signed and unsigned ConstantRsnge from LLVM ...

[ubfx]

we might want to move to a signed and unsigned ConstantRsnge from LLVM

Thank you for the hint, I'll look it up