[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 05:27:47 PST 2024


https://github.com/zero9178 created https://github.com/llvm/llvm-project/pull/118488

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, particular for beginners:
* 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.

>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] [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
 }



More information about the Mlir-commits mailing list