[Mlir-commits] [mlir] [mlir] Improve error message when number of operands and types differ (PR #118488)
Markus Böck
llvmlistbot at llvm.org
Tue Dec 3 09:41:07 PST 2024
https://github.com/zero9178 updated https://github.com/llvm/llvm-project/pull/118488
>From cb606775216294da4300342ace91ed8f97e83b81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Tue, 3 Dec 2024 14:25:58 +0100
Subject: [PATCH 1/3] [mlir] Improve the error message when the number of
operands and types differ
If using a variadic operand, the error message given if the number of types and operands do not match would be along the lines of:
```
3 operands present, but expected 2
```
This error message is confusing for multiple reasons:
* If the intention is to have 3 operands, it does not point out why it expects 2. The user may actually just want to add a type to the type list
* It reads as if a verifier error rather than a parser error, giving the impression the Op only supports 2 operands.
This PR attempts to improve the error message by first noting the issue ("number of operands and types mismatch") and then give a helpful note as to how many operands and types it got.
---
mlir/include/mlir/IR/OpImplementation.h | 34 +++++++++++++++----
mlir/test/Dialect/LLVMIR/invalid.mlir | 6 ++--
.../Dialect/Linalg/transform-ops-invalid.mlir | 3 +-
mlir/test/Dialect/SCF/invalid.mlir | 6 ++--
mlir/test/Dialect/SPIRV/IR/memory-ops.mlir | 9 +++--
mlir/test/Dialect/Tensor/invalid.mlir | 3 +-
mlir/test/Dialect/Vector/invalid.mlir | 3 +-
7 files changed, 47 insertions(+), 17 deletions(-)
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index a7222794f320b2..4a5d5e27309f5d 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -1602,14 +1602,34 @@ class OpAsmParser : public AsmParser {
SmallVectorImpl<Value> &result) {
size_t operandSize = llvm::range_size(operands);
size_t typeSize = llvm::range_size(types);
- if (operandSize != typeSize)
- return emitError(loc)
- << operandSize << " operands present, but expected " << typeSize;
+ if (operandSize == typeSize) {
+ for (auto [operand, type] : llvm::zip_equal(operands, types))
+ if (resolveOperand(operand, type, result))
+ return failure();
+ return success();
+ }
- for (auto [operand, type] : llvm::zip_equal(operands, types))
- if (resolveOperand(operand, type, result))
- return failure();
- return success();
+ InFlightDiagnostic diag =
+ emitError(loc)
+ << "number of operands and types do not match";
+ std::string lesserQuantityText = "operand";
+ if (operandSize != 1)
+ lesserQuantityText += "s";
+ std::string higherQuantityText = "type";
+ if (typeSize != 1)
+ higherQuantityText += "s";
+
+ size_t lesserQuantity = operandSize;
+ size_t higherQuantity = typeSize;
+ if (operandSize > typeSize) {
+ std::swap(lesserQuantity, higherQuantity);
+ std::swap(lesserQuantityText, higherQuantityText);
+ }
+
+ diag.attachNote() << "got " << higherQuantity << " " << higherQuantityText
+ << " but only " << lesserQuantity << " "
+ << lesserQuantityText;
+ return diag;
}
/// Parses an affine map attribute where dims and symbols are SSA operands.
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index 5677d7ff41202f..b279b862bed2eb 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -91,14 +91,16 @@ func.func @alloca_non_integer_alignment() {
// -----
func.func @gep_missing_input_result_type(%pos : i64, %base : !llvm.ptr) {
- // expected-error at +1 {{2 operands present, but expected 0}}
+ // expected-error at +2 {{number of operands and types do not match}}
+ // expected-note at +1 {{got 2 operands but only 0 types}}
llvm.getelementptr %base[%pos] : () -> (), i64
}
// -----
func.func @gep_missing_input_type(%pos : i64, %base : !llvm.ptr) {
- // expected-error at +1 {{2 operands present, but expected 0}}
+ // expected-error at +2 {{number of operands and types do not match}}
+ // expected-note at +1 {{got 2 operands but only 0 types}}
llvm.getelementptr %base[%pos] : () -> (!llvm.ptr), i64
}
diff --git a/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir b/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir
index fbebb97a11983e..0c6ce11a6c8bf6 100644
--- a/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir
+++ b/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir
@@ -77,7 +77,8 @@ transform.sequence failures(propagate) {
transform.sequence failures(propagate) {
^bb0(%arg0: !transform.any_op):
%0 = transform.param.constant 2 : i64 -> !transform.param<i64>
- // expected-error at below {{custom op 'transform.structured.vectorize' 1 operands present, but expected 2}}
+ // expected-error at +2 {{custom op 'transform.structured.vectorize' number of operands and types do not match}}
+ // expected-note at below {{got 2 types but only 1 operand}}
transform.structured.vectorize %arg0 vector_sizes [%0, 2] : !transform.any_op, !transform.param<i64>, !transform.param<i64>
}
diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir
index 337eb9eeb8fa57..58a91413301ea2 100644
--- a/mlir/test/Dialect/SCF/invalid.mlir
+++ b/mlir/test/Dialect/SCF/invalid.mlir
@@ -247,7 +247,8 @@ func.func @parallel_more_results_than_reduces(
func.func @parallel_more_results_than_initial_values(
%arg0 : index, %arg1: index, %arg2: index) {
- // expected-error at +1 {{'scf.parallel' 0 operands present, but expected 1}}
+ // expected-error at +2 {{'scf.parallel' number of operands and types do not match}}
+ // expected-note at +1 {{got 1 type but only 0 operands}}
%res = scf.parallel (%i0) = (%arg0) to (%arg1) step (%arg2) -> f32 {
scf.reduce(%arg0 : index) {
^bb0(%lhs: index, %rhs: index):
@@ -609,7 +610,8 @@ func.func @wrong_num_results(%in: tensor<100xf32>, %out: tensor<100xf32>) {
%c1 = arith.constant 1 : index
%num_threads = arith.constant 100 : index
- // expected-error @+1 {{1 operands present, but expected 2}}
+ // expected-error at +2 {{number of operands and types do not match}}
+ // expected-note at +1 {{got 2 types but only 1 operand}}
%result:2 = scf.forall (%thread_idx) in (%num_threads) shared_outs(%o = %out) -> (tensor<100xf32>, tensor<100xf32>) {
%1 = tensor.extract_slice %in[%thread_idx][1][1] : tensor<100xf32> to tensor<1xf32>
scf.forall.in_parallel {
diff --git a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
index 5aef6135afd97e..76a3a32dbcf91c 100644
--- a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
@@ -57,7 +57,8 @@ func.func @access_chain_non_composite() -> () {
func.func @access_chain_no_indices(%index0 : i32) -> () {
%0 = spirv.Variable : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>
- // expected-error @+1 {{custom op 'spirv.AccessChain' 0 operands present, but expected 1}}
+ // expected-error @+2 {{custom op 'spirv.AccessChain' number of operands and types do not match}}
+ // expected-note @+1 {{got 1 type but only 0 operands}}
%1 = spirv.AccessChain %0[] : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>, i32 -> !spirv.ptr<f32, Function>
return
}
@@ -75,7 +76,8 @@ func.func @access_chain_missing_comma(%index0 : i32) -> () {
func.func @access_chain_invalid_indices_types_count(%index0 : i32) -> () {
%0 = spirv.Variable : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>
- // expected-error @+1 {{custom op 'spirv.AccessChain' 1 operands present, but expected 2}}
+ // expected-error @+2 {{custom op 'spirv.AccessChain' number of operands and types do not match}}
+ // expected-note @+1 {{got 2 types but only 1 operand}}
%1 = spirv.AccessChain %0[%index0] : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>, i32, i32 -> !spirv.ptr<!spirv.array<4xf32>, Function>
return
}
@@ -84,7 +86,8 @@ func.func @access_chain_invalid_indices_types_count(%index0 : i32) -> () {
func.func @access_chain_missing_indices_type(%index0 : i32) -> () {
%0 = spirv.Variable : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>
- // expected-error @+1 {{custom op 'spirv.AccessChain' 2 operands present, but expected 1}}
+ // expected-error @+2 {{custom op 'spirv.AccessChain' number of operands and types do not match}}
+ // expected-note @+1 {{got 2 operands but only 1 type}}
%1 = spirv.AccessChain %0[%index0, %index0] : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>, i32 -> !spirv.ptr<f32, Function>
return
}
diff --git a/mlir/test/Dialect/Tensor/invalid.mlir b/mlir/test/Dialect/Tensor/invalid.mlir
index 77cae1cc5f242d..0f858fbda9dcdd 100644
--- a/mlir/test/Dialect/Tensor/invalid.mlir
+++ b/mlir/test/Dialect/Tensor/invalid.mlir
@@ -90,7 +90,8 @@ func.func @tensor.from_elements_wrong_result_type() {
// -----
func.func @tensor.from_elements_wrong_elements_count() {
- // expected-error at +2 {{1 operands present, but expected 2}}
+ // expected-error at +3 {{number of operands and types do not match}}
+ // expected-note at +2 {{got 2 types but only 1 operand}}
%c0 = arith.constant 0 : index
%0 = tensor.from_elements %c0 : tensor<2xindex>
return
diff --git a/mlir/test/Dialect/Vector/invalid.mlir b/mlir/test/Dialect/Vector/invalid.mlir
index 9f7efa15ed5207..5f9345e75285b6 100644
--- a/mlir/test/Dialect/Vector/invalid.mlir
+++ b/mlir/test/Dialect/Vector/invalid.mlir
@@ -1803,7 +1803,8 @@ func.func @deinterleave_scalable_rank_fail(%vec : vector<2x[4]xf32>) {
// -----
func.func @invalid_from_elements(%a: f32) {
- // expected-error @+1 {{'vector.from_elements' 1 operands present, but expected 2}}
+ // expected-error @+2 {{'vector.from_elements' number of operands and types do not match}}
+ // expected-note @+1 {{got 2 types but only 1 operand}}
vector.from_elements %a : vector<2xf32>
return
}
>From 689249d62df71ae6e9c5bb0fffad3e35c29517b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Tue, 3 Dec 2024 14:38:55 +0100
Subject: [PATCH 2/3] clang-format
---
mlir/include/mlir/IR/OpImplementation.h | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 4a5d5e27309f5d..874f05499da928 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -1609,9 +1609,8 @@ class OpAsmParser : public AsmParser {
return success();
}
- InFlightDiagnostic diag =
- emitError(loc)
- << "number of operands and types do not match";
+ InFlightDiagnostic diag = emitError(loc)
+ << "number of operands and types do not match";
std::string lesserQuantityText = "operand";
if (operandSize != 1)
lesserQuantityText += "s";
>From b2fd306b7d2d837253d8b9bac080b87afcb630c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Tue, 3 Dec 2024 18:40:37 +0100
Subject: [PATCH 3/3] address review comments
---
mlir/include/mlir/IR/OpImplementation.h | 34 +++++--------------
mlir/test/Dialect/LLVMIR/invalid.mlir | 6 ++--
.../Dialect/Linalg/transform-ops-invalid.mlir | 3 +-
mlir/test/Dialect/SCF/invalid.mlir | 6 ++--
mlir/test/Dialect/SPIRV/IR/memory-ops.mlir | 9 ++---
mlir/test/Dialect/Tensor/invalid.mlir | 3 +-
mlir/test/Dialect/Vector/invalid.mlir | 3 +-
7 files changed, 18 insertions(+), 46 deletions(-)
diff --git a/mlir/include/mlir/IR/OpImplementation.h b/mlir/include/mlir/IR/OpImplementation.h
index 874f05499da928..6c1ff4d0e5e6b9 100644
--- a/mlir/include/mlir/IR/OpImplementation.h
+++ b/mlir/include/mlir/IR/OpImplementation.h
@@ -1602,33 +1602,15 @@ class OpAsmParser : public AsmParser {
SmallVectorImpl<Value> &result) {
size_t operandSize = llvm::range_size(operands);
size_t typeSize = llvm::range_size(types);
- if (operandSize == typeSize) {
- for (auto [operand, type] : llvm::zip_equal(operands, types))
- if (resolveOperand(operand, type, result))
- return failure();
- return success();
- }
-
- InFlightDiagnostic diag = emitError(loc)
- << "number of operands and types do not match";
- std::string lesserQuantityText = "operand";
- if (operandSize != 1)
- lesserQuantityText += "s";
- std::string higherQuantityText = "type";
- if (typeSize != 1)
- higherQuantityText += "s";
-
- size_t lesserQuantity = operandSize;
- size_t higherQuantity = typeSize;
- if (operandSize > typeSize) {
- std::swap(lesserQuantity, higherQuantity);
- std::swap(lesserQuantityText, higherQuantityText);
- }
+ if (operandSize != typeSize)
+ return emitError(loc)
+ << "number of operands and types do not match: got " << operandSize
+ << " operands and " << typeSize << " types";
- diag.attachNote() << "got " << higherQuantity << " " << higherQuantityText
- << " but only " << lesserQuantity << " "
- << lesserQuantityText;
- return diag;
+ for (auto [operand, type] : llvm::zip_equal(operands, types))
+ if (resolveOperand(operand, type, result))
+ return failure();
+ return success();
}
/// Parses an affine map attribute where dims and symbols are SSA operands.
diff --git a/mlir/test/Dialect/LLVMIR/invalid.mlir b/mlir/test/Dialect/LLVMIR/invalid.mlir
index b279b862bed2eb..25806d9d0edd72 100644
--- a/mlir/test/Dialect/LLVMIR/invalid.mlir
+++ b/mlir/test/Dialect/LLVMIR/invalid.mlir
@@ -91,16 +91,14 @@ func.func @alloca_non_integer_alignment() {
// -----
func.func @gep_missing_input_result_type(%pos : i64, %base : !llvm.ptr) {
- // expected-error at +2 {{number of operands and types do not match}}
- // expected-note at +1 {{got 2 operands but only 0 types}}
+ // expected-error at +1 {{number of operands and types do not match: got 2 operands and 0 types}}
llvm.getelementptr %base[%pos] : () -> (), i64
}
// -----
func.func @gep_missing_input_type(%pos : i64, %base : !llvm.ptr) {
- // expected-error at +2 {{number of operands and types do not match}}
- // expected-note at +1 {{got 2 operands but only 0 types}}
+ // expected-error at +1 {{number of operands and types do not match: got 2 operands and 0 types}}
llvm.getelementptr %base[%pos] : () -> (!llvm.ptr), i64
}
diff --git a/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir b/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir
index 0c6ce11a6c8bf6..6584596cdfdb21 100644
--- a/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir
+++ b/mlir/test/Dialect/Linalg/transform-ops-invalid.mlir
@@ -77,8 +77,7 @@ transform.sequence failures(propagate) {
transform.sequence failures(propagate) {
^bb0(%arg0: !transform.any_op):
%0 = transform.param.constant 2 : i64 -> !transform.param<i64>
- // expected-error at +2 {{custom op 'transform.structured.vectorize' number of operands and types do not match}}
- // expected-note at below {{got 2 types but only 1 operand}}
+ // expected-error at +1 {{custom op 'transform.structured.vectorize' number of operands and types do not match: got 1 operands and 2 types}}
transform.structured.vectorize %arg0 vector_sizes [%0, 2] : !transform.any_op, !transform.param<i64>, !transform.param<i64>
}
diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir
index 58a91413301ea2..80576be8801276 100644
--- a/mlir/test/Dialect/SCF/invalid.mlir
+++ b/mlir/test/Dialect/SCF/invalid.mlir
@@ -247,8 +247,7 @@ func.func @parallel_more_results_than_reduces(
func.func @parallel_more_results_than_initial_values(
%arg0 : index, %arg1: index, %arg2: index) {
- // expected-error at +2 {{'scf.parallel' number of operands and types do not match}}
- // expected-note at +1 {{got 1 type but only 0 operands}}
+ // expected-error at +1 {{'scf.parallel' number of operands and types do not match: got 0 operands and 1 types}}
%res = scf.parallel (%i0) = (%arg0) to (%arg1) step (%arg2) -> f32 {
scf.reduce(%arg0 : index) {
^bb0(%lhs: index, %rhs: index):
@@ -610,8 +609,7 @@ func.func @wrong_num_results(%in: tensor<100xf32>, %out: tensor<100xf32>) {
%c1 = arith.constant 1 : index
%num_threads = arith.constant 100 : index
- // expected-error at +2 {{number of operands and types do not match}}
- // expected-note at +1 {{got 2 types but only 1 operand}}
+ // expected-error at +1 {{number of operands and types do not match: got 1 operands and 2 types}}
%result:2 = scf.forall (%thread_idx) in (%num_threads) shared_outs(%o = %out) -> (tensor<100xf32>, tensor<100xf32>) {
%1 = tensor.extract_slice %in[%thread_idx][1][1] : tensor<100xf32> to tensor<1xf32>
scf.forall.in_parallel {
diff --git a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
index 76a3a32dbcf91c..57ff94762ff68c 100644
--- a/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/memory-ops.mlir
@@ -57,8 +57,7 @@ func.func @access_chain_non_composite() -> () {
func.func @access_chain_no_indices(%index0 : i32) -> () {
%0 = spirv.Variable : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>
- // expected-error @+2 {{custom op 'spirv.AccessChain' number of operands and types do not match}}
- // expected-note @+1 {{got 1 type but only 0 operands}}
+ // expected-error @+1 {{custom op 'spirv.AccessChain' number of operands and types do not match: got 0 operands and 1 types}}
%1 = spirv.AccessChain %0[] : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>, i32 -> !spirv.ptr<f32, Function>
return
}
@@ -76,8 +75,7 @@ func.func @access_chain_missing_comma(%index0 : i32) -> () {
func.func @access_chain_invalid_indices_types_count(%index0 : i32) -> () {
%0 = spirv.Variable : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>
- // expected-error @+2 {{custom op 'spirv.AccessChain' number of operands and types do not match}}
- // expected-note @+1 {{got 2 types but only 1 operand}}
+ // expected-error @+1 {{custom op 'spirv.AccessChain' number of operands and types do not match: got 1 operands and 2 types}}
%1 = spirv.AccessChain %0[%index0] : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>, i32, i32 -> !spirv.ptr<!spirv.array<4xf32>, Function>
return
}
@@ -86,8 +84,7 @@ func.func @access_chain_invalid_indices_types_count(%index0 : i32) -> () {
func.func @access_chain_missing_indices_type(%index0 : i32) -> () {
%0 = spirv.Variable : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>
- // expected-error @+2 {{custom op 'spirv.AccessChain' number of operands and types do not match}}
- // expected-note @+1 {{got 2 operands but only 1 type}}
+ // expected-error @+1 {{custom op 'spirv.AccessChain' number of operands and types do not match: got 2 operands and 1 types}}
%1 = spirv.AccessChain %0[%index0, %index0] : !spirv.ptr<!spirv.array<4x!spirv.array<4xf32>>, Function>, i32 -> !spirv.ptr<f32, Function>
return
}
diff --git a/mlir/test/Dialect/Tensor/invalid.mlir b/mlir/test/Dialect/Tensor/invalid.mlir
index 0f858fbda9dcdd..83cb4b9d4ab247 100644
--- a/mlir/test/Dialect/Tensor/invalid.mlir
+++ b/mlir/test/Dialect/Tensor/invalid.mlir
@@ -90,8 +90,7 @@ func.func @tensor.from_elements_wrong_result_type() {
// -----
func.func @tensor.from_elements_wrong_elements_count() {
- // expected-error at +3 {{number of operands and types do not match}}
- // expected-note at +2 {{got 2 types but only 1 operand}}
+ // expected-error at +2 {{number of operands and types do not match: got 1 operands and 2 types}}
%c0 = arith.constant 0 : index
%0 = tensor.from_elements %c0 : tensor<2xindex>
return
diff --git a/mlir/test/Dialect/Vector/invalid.mlir b/mlir/test/Dialect/Vector/invalid.mlir
index 5f9345e75285b6..1a70791fae1257 100644
--- a/mlir/test/Dialect/Vector/invalid.mlir
+++ b/mlir/test/Dialect/Vector/invalid.mlir
@@ -1803,8 +1803,7 @@ func.func @deinterleave_scalable_rank_fail(%vec : vector<2x[4]xf32>) {
// -----
func.func @invalid_from_elements(%a: f32) {
- // expected-error @+2 {{'vector.from_elements' number of operands and types do not match}}
- // expected-note @+1 {{got 2 types but only 1 operand}}
+ // expected-error @+1 {{'vector.from_elements' number of operands and types do not match: got 1 operands and 2 types}}
vector.from_elements %a : vector<2xf32>
return
}
More information about the Mlir-commits
mailing list