[Mlir-commits] [mlir] [mlir][linalg} Add check for affine.for map bounds (PR #127105)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-affine

Author: Ayokunle Amodu (ayokunle321)

<details>
<summary>Changes</summary>

Related to issue #<!-- -->120001.

I learnt that we shouldn’t assert on IR that can be constructed and passes the verifier. In this case, it appears that the verifier is insufficiently strict and should be tightened. Since the maps used in the affine.for op below do not have results, the IR verification should fail however it does not. 

Minimal example:

#map = affine_map<() -> ()>
module {
  func.func @<!-- -->main() {
    affine.for %arg0 = max #map() to min #map() {
    }
    return
  }
}

This gets past the verifier, but I added a check to see that it causes an error if an affine.for has a bound map with no result.

Open to any comments or clarifications :)



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


2 Files Affected:

- (modified) mlir/lib/Dialect/Affine/IR/AffineOps.cpp (+6) 
- (modified) mlir/test/Dialect/Affine/invalid.mlir (+22) 


``````````diff
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 147f5dd7a24b6..06e26e887b050 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 44e484b9ba598..5a3243cb074c1 100644
--- a/mlir/test/Dialect/Affine/invalid.mlir
+++ b/mlir/test/Dialect/Affine/invalid.mlir
@@ -523,3 +523,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
+}

``````````

</details>


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