Skip to content

[mlir][affine] Verify map of affineMaxMinOp has at least one result #108699

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

CoTinker
Copy link
Contributor

This PR fixes a bug in verifyAffineMinMaxOp that allowed map of affine.max and affine.min to have an empty result. Fixes #108368.

This PR fixes a bug in `verifyAffineMinMaxOp` that allowed `map` of
`affine.max` and `affine.min` to have an empty result.
@llvmbot
Copy link
Member

llvmbot commented Sep 14, 2024

@llvm/pr-subscribers-mlir

Author: Longsheng Mou (CoTinker)

Changes

This PR fixes a bug in verifyAffineMinMaxOp that allowed map of affine.max and affine.min to have an empty result. Fixes #108368.


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

2 Files Affected:

  • (modified) mlir/lib/Dialect/Affine/IR/AffineOps.cpp (+3)
  • (modified) mlir/test/Dialect/Affine/invalid.mlir (+32)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 11b6b7cf5fd5a7..b89888e6aa83f7 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -3214,6 +3214,9 @@ static LogicalResult verifyAffineMinMaxOp(T op) {
       op.getMap().getNumDims() + op.getMap().getNumSymbols())
     return op.emitOpError(
         "operand count and affine map dimension and symbol count must match");
+
+  if (op.getMap().getNumResults() == 0)
+    return op.emitOpError("affine map expect at least one result");
   return success();
 }
 
diff --git a/mlir/test/Dialect/Affine/invalid.mlir b/mlir/test/Dialect/Affine/invalid.mlir
index 60f13102f55156..869ea712bb3690 100644
--- a/mlir/test/Dialect/Affine/invalid.mlir
+++ b/mlir/test/Dialect/Affine/invalid.mlir
@@ -178,6 +178,22 @@ func.func @affine_min(%arg0 : index, %arg1 : index, %arg2 : index) {
 
 // -----
 
+func.func @affine_min() {
+  // expected-error@+1 {{'affine.min' op affine map expect at least one result}}
+  %0 = affine.min affine_map<() -> ()> ()
+  return
+}
+
+// -----
+
+func.func @affine_min(%arg0 : index) {
+  // expected-error@+1 {{'affine.min' op affine map expect at least one result}}
+  %0 = affine.min affine_map<(d0) -> ()> (%arg0)
+  return
+}
+
+// -----
+
 func.func @affine_max(%arg0 : index, %arg1 : index, %arg2 : index) {
   // expected-error@+1 {{operand count and affine map dimension and symbol count must match}}
   %0 = affine.max affine_map<(d0) -> (d0)> (%arg0, %arg1)
@@ -205,6 +221,22 @@ func.func @affine_max(%arg0 : index, %arg1 : index, %arg2 : index) {
 
 // -----
 
+func.func @affine_max() {
+  // expected-error@+1 {{'affine.max' op affine map expect at least one result}}
+  %0 = affine.max affine_map<() -> ()> ()
+  return
+}
+
+// -----
+
+func.func @affine_max(%arg0 : index) {
+  // expected-error@+1 {{'affine.max' op affine map expect at least one result}}
+  %0 = affine.max affine_map<(d0) -> ()> (%arg0)
+  return
+}
+
+// -----
+
 func.func @affine_parallel(%arg0 : index, %arg1 : index, %arg2 : index) {
   // expected-error@+1 {{the number of region arguments (1) and the number of map groups for lower (2) and upper bound (2), and the number of steps (2) must all match}}
   affine.parallel (%i) = (0, 0) to (100, 100) step (10, 10) {

@llvmbot
Copy link
Member

llvmbot commented Sep 14, 2024

@llvm/pr-subscribers-mlir-affine

Author: Longsheng Mou (CoTinker)

Changes

This PR fixes a bug in verifyAffineMinMaxOp that allowed map of affine.max and affine.min to have an empty result. Fixes #108368.


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

2 Files Affected:

  • (modified) mlir/lib/Dialect/Affine/IR/AffineOps.cpp (+3)
  • (modified) mlir/test/Dialect/Affine/invalid.mlir (+32)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 11b6b7cf5fd5a7..b89888e6aa83f7 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -3214,6 +3214,9 @@ static LogicalResult verifyAffineMinMaxOp(T op) {
       op.getMap().getNumDims() + op.getMap().getNumSymbols())
     return op.emitOpError(
         "operand count and affine map dimension and symbol count must match");
+
+  if (op.getMap().getNumResults() == 0)
+    return op.emitOpError("affine map expect at least one result");
   return success();
 }
 
diff --git a/mlir/test/Dialect/Affine/invalid.mlir b/mlir/test/Dialect/Affine/invalid.mlir
index 60f13102f55156..869ea712bb3690 100644
--- a/mlir/test/Dialect/Affine/invalid.mlir
+++ b/mlir/test/Dialect/Affine/invalid.mlir
@@ -178,6 +178,22 @@ func.func @affine_min(%arg0 : index, %arg1 : index, %arg2 : index) {
 
 // -----
 
+func.func @affine_min() {
+  // expected-error@+1 {{'affine.min' op affine map expect at least one result}}
+  %0 = affine.min affine_map<() -> ()> ()
+  return
+}
+
+// -----
+
+func.func @affine_min(%arg0 : index) {
+  // expected-error@+1 {{'affine.min' op affine map expect at least one result}}
+  %0 = affine.min affine_map<(d0) -> ()> (%arg0)
+  return
+}
+
+// -----
+
 func.func @affine_max(%arg0 : index, %arg1 : index, %arg2 : index) {
   // expected-error@+1 {{operand count and affine map dimension and symbol count must match}}
   %0 = affine.max affine_map<(d0) -> (d0)> (%arg0, %arg1)
@@ -205,6 +221,22 @@ func.func @affine_max(%arg0 : index, %arg1 : index, %arg2 : index) {
 
 // -----
 
+func.func @affine_max() {
+  // expected-error@+1 {{'affine.max' op affine map expect at least one result}}
+  %0 = affine.max affine_map<() -> ()> ()
+  return
+}
+
+// -----
+
+func.func @affine_max(%arg0 : index) {
+  // expected-error@+1 {{'affine.max' op affine map expect at least one result}}
+  %0 = affine.max affine_map<(d0) -> ()> (%arg0)
+  return
+}
+
+// -----
+
 func.func @affine_parallel(%arg0 : index, %arg1 : index, %arg2 : index) {
   // expected-error@+1 {{the number of region arguments (1) and the number of map groups for lower (2) and upper bound (2), and the number of steps (2) must all match}}
   affine.parallel (%i) = (0, 0) to (100, 100) step (10, 10) {

Copy link
Contributor

@qedawkins qedawkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@CoTinker
Copy link
Contributor Author

Thanks.

@CoTinker CoTinker merged commit af4dcbb into llvm:main Sep 20, 2024
11 checks passed
@CoTinker CoTinker deleted the verify_maxmin_map branch September 20, 2024 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants