[Mlir-commits] [mlir] [mlir][affine] Add Check for 'affine.for' Bound Map Results (PR #127105)

Ayokunle Amodu llvmlistbot at llvm.org
Wed Feb 19 11:13:01 PST 2025


https://github.com/ayokunle321 updated https://github.com/llvm/llvm-project/pull/127105

>From e8894d1e7f9f06e604363de3d34833b21a54620d Mon Sep 17 00:00:00 2001
From: Uday Bondhugula <uday at polymagelabs.com>
Date: Wed, 19 Feb 2025 20:48:38 +0530
Subject: [PATCH 1/7] [MLIR][Affine] Add missing check in affine data copy nest
 generation

Handle case where no lower or upper bound could be found for a
dimension. Invalid IR was being generated silently for a test case.

Fixes: https://github.com/llvm/llvm-project/issues/127808
---
 mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp    |  9 ++++++++-
 mlir/test/Dialect/Affine/affine-data-copy.mlir | 11 ++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp b/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp
index 4e02559a08949..82b96e9876a6f 100644
--- a/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp
+++ b/mlir/lib/Dialect/Affine/Utils/LoopUtils.cpp
@@ -2001,8 +2001,15 @@ static LogicalResult generateCopy(
   }
 
   SmallVector<AffineMap, 4> lbMaps(rank), ubMaps(rank);
-  for (unsigned i = 0; i < rank; ++i)
+  for (unsigned i = 0; i < rank; ++i) {
     region.getLowerAndUpperBound(i, lbMaps[i], ubMaps[i]);
+    if (lbMaps[i].getNumResults() == 0 || ubMaps[i].getNumResults() == 0) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "Missing lower or upper bound for region along dimension: "
+                 << i << '\n');
+      return failure();
+    }
+  }
 
   const FlatAffineValueConstraints *cst = region.getConstraints();
   // 'regionSymbols' hold values that this memory region is symbolic/parametric
diff --git a/mlir/test/Dialect/Affine/affine-data-copy.mlir b/mlir/test/Dialect/Affine/affine-data-copy.mlir
index 330cf92bafba4..5615acae5ecc4 100644
--- a/mlir/test/Dialect/Affine/affine-data-copy.mlir
+++ b/mlir/test/Dialect/Affine/affine-data-copy.mlir
@@ -300,14 +300,15 @@ func.func @affine_parallel(%85:memref<2x5x4x2xi64>) {
       }
     }
   }
-  // CHECK:     affine.for
-  // CHECK-NEXT:  affine.for %{{.*}} = 0 to 5
-  // CHECK-NEXT:    affine.for %{{.*}} = 0 to 4
-  // CHECK-NEXT:      affine.for %{{.*}} = 0 to 2
-
+  // Lower and upper bounds for the region can't be determined for the outermost
+  // dimension. No fast buffer generation.
   // CHECK:     affine.for
   // CHECK-NEXT:  affine.parallel
   // CHECK-NEXT:    affine.parallel
+  // CHECK-NEXT:      affine.for
+  // CHECK-NOT:      affine.for
+
+
   return
 }
 

>From 4aae9feff081e0b7bdc9892946c8c0a5aad6febf Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle321 at users.noreply.github.com>
Date: Thu, 13 Feb 2025 11:23:53 -0700
Subject: [PATCH 2/7] added check for affine.for bounds

---
 mlir/lib/Dialect/Affine/IR/AffineOps.cpp |  6 ++++++
 mlir/test/Dialect/Affine/invalid.mlir    | 22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index cfc51ad2a1524..b79cc6850b6e0 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1903,6 +1903,12 @@ LogicalResult AffineForOp::verifyRegions() {
                                              getUpperBoundMap().getNumDims())))
       return failure();
 
+  if (getLowerBoundMap().getNumResults() < 1)
+    return emitOpError("expected lower bound map to have at least one result");
+
+  if (getUpperBoundMap().getNumResults() < 1)
+    return emitOpError("expected upper bound map to have at least one result");
+    
   unsigned opNumResults = getNumResults();
   if (opNumResults == 0)
     return success();
diff --git a/mlir/test/Dialect/Affine/invalid.mlir b/mlir/test/Dialect/Affine/invalid.mlir
index da2913e3fec28..69d58ce1b5265 100644
--- a/mlir/test/Dialect/Affine/invalid.mlir
+++ b/mlir/test/Dialect/Affine/invalid.mlir
@@ -541,3 +541,25 @@ func.func @dynamic_dimension_index() {
   }) : () -> ()
   return
 }
+
+// -----
+
+#map = affine_map<() -> ()>
+#map1 = affine_map<() -> (1)>
+func.func @no_lower_bound() {
+  // expected-error at +1 {{'affine.for' op expected lower bound map to have at least one result}}
+  affine.for %i = max #map() to min #map1() {
+  }
+  return
+}
+
+// -----
+
+#map = affine_map<() -> ()>
+#map1 = affine_map<() -> (1)>
+func.func @no_upper_bound() {
+  // expected-error at +1 {{'affine.for' op expected upper bound map to have at least one result}}
+  affine.for %i = max #map1() to min #map() {
+  }
+  return
+}

>From f7cef5a613d1c1272aac46a15be4d79d2487819f Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle321 at users.noreply.github.com>
Date: Thu, 13 Feb 2025 11:52:48 -0700
Subject: [PATCH 3/7] added comments

---
 mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index b79cc6850b6e0..e044470cb451c 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1902,13 +1902,14 @@ LogicalResult AffineForOp::verifyRegions() {
     if (failed(verifyDimAndSymbolIdentifiers(*this, getUpperBoundOperands(),
                                              getUpperBoundMap().getNumDims())))
       return failure();
-
+  // Verify that the bound maps produce at least one result.
+  /// Lower bound.
   if (getLowerBoundMap().getNumResults() < 1)
     return emitOpError("expected lower bound map to have at least one result");
-
+  /// Upper bound.
   if (getUpperBoundMap().getNumResults() < 1)
     return emitOpError("expected upper bound map to have at least one result");
-    
+
   unsigned opNumResults = getNumResults();
   if (opNumResults == 0)
     return success();

>From 4592e597332d4196d6ea0d3e14fe631f92e9703d Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle321 at users.noreply.github.com>
Date: Thu, 13 Feb 2025 11:59:08 -0700
Subject: [PATCH 4/7] fixed code style

---
 mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index e044470cb451c..ee7b1ac385304 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1902,6 +1902,7 @@ LogicalResult AffineForOp::verifyRegions() {
     if (failed(verifyDimAndSymbolIdentifiers(*this, getUpperBoundOperands(),
                                              getUpperBoundMap().getNumDims())))
       return failure();
+      
   // Verify that the bound maps produce at least one result.
   /// Lower bound.
   if (getLowerBoundMap().getNumResults() < 1)

>From aa76624209cee244618a0cf8075cc44d13449e03 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle321 at users.noreply.github.com>
Date: Thu, 13 Feb 2025 12:03:34 -0700
Subject: [PATCH 5/7] fix style

---
 mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index ee7b1ac385304..aa1d97dbbd23b 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1901,8 +1901,7 @@ LogicalResult AffineForOp::verifyRegions() {
   if (getUpperBoundMap().getNumInputs() > 0)
     if (failed(verifyDimAndSymbolIdentifiers(*this, getUpperBoundOperands(),
                                              getUpperBoundMap().getNumDims())))
-      return failure();
-      
+      return failure();    
   // Verify that the bound maps produce at least one result.
   /// Lower bound.
   if (getLowerBoundMap().getNumResults() < 1)

>From 851fb07ff9265d54fbacba640dcfe92c8e298b15 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle321 at users.noreply.github.com>
Date: Thu, 13 Feb 2025 12:11:59 -0700
Subject: [PATCH 6/7] format code properly

---
 mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index aa1d97dbbd23b..e044470cb451c 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1901,7 +1901,7 @@ LogicalResult AffineForOp::verifyRegions() {
   if (getUpperBoundMap().getNumInputs() > 0)
     if (failed(verifyDimAndSymbolIdentifiers(*this, getUpperBoundOperands(),
                                              getUpperBoundMap().getNumDims())))
-      return failure();    
+      return failure();
   // Verify that the bound maps produce at least one result.
   /// Lower bound.
   if (getLowerBoundMap().getNumResults() < 1)

>From b07e11215016b7990b2c581a72daaa3ffdfdc7fc Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle321 at users.noreply.github.com>
Date: Thu, 13 Feb 2025 12:52:40 -0700
Subject: [PATCH 7/7] removed unnecessary comments

---
 mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index e044470cb451c..c4891614662a1 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1902,11 +1902,8 @@ LogicalResult AffineForOp::verifyRegions() {
     if (failed(verifyDimAndSymbolIdentifiers(*this, getUpperBoundOperands(),
                                              getUpperBoundMap().getNumDims())))
       return failure();
-  // Verify that the bound maps produce at least one result.
-  /// Lower bound.
   if (getLowerBoundMap().getNumResults() < 1)
     return emitOpError("expected lower bound map to have at least one result");
-  /// Upper bound.
   if (getUpperBoundMap().getNumResults() < 1)
     return emitOpError("expected upper bound map to have at least one result");
 



More information about the Mlir-commits mailing list