[Mlir-commits] [mlir] [mlir][intrange] Fix inference of zero-trip loop bound (PR #96429)

[Felix Schneider]

https://github.com/ubfx created https://github.com/llvm/llvm-project/pull/96429

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 https://github.com/llvm/llvm-project/issues/94423

>From f8fcb23df022798e0f9f4affeceee2d605c3ac2e Mon Sep 17 00:00:00 2001
From: Felix Schneider <fx.schn at gmail.com>
Date: Sun, 23 Jun 2024 13:33:03 +0200
Subject: [PATCH] [mlir][intrange] Fix inference of zero-trip loop bound

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 https://github.com/llvm/llvm-project/issues/94423
---
 .../Analysis/DataFlow/IntegerRangeAnalysis.cpp | 11 ++++++++---
 .../Dialect/Arith/int-range-interface.mlir     | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 3 deletions(-)

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
+}