Useful error when input dim is unused by LHS/RHS.

[Benoit]

better message

[Benoit]

Generates this message:

benoitjacob@cloud:~/mlir-build$ cmake --build . --target mlir-opt && bin/mlir-opt /tmp/x.mlir
[6/6] Linking CXX executable bin/mlir-opt
/tmp/x.mlir:2:13: error: 'vector.contract' op expected all input dimensions to be used by either the LHS or the RHS.
  %result = vector.contract {
            ^
/tmp/x.mlir:2:13: note: see current operation: %0 = "vector.contract"(%arg0, %arg1, %arg2) {indexing_maps = [affine_map<(d0, d1, d2) -> (d0, d2)>, affine_map<(d0, d1, d2) -> (d2)>, affine_map<(d0, d1, d2) -> (d1)>], iterator_types = ["reduction", "parallel", "reduction"], kind = #vector.kind<add>} : (vector<1x2xi32>, vector<2xi32>, vector<1xi32>) -> vector<1xi32>

On this testcase:

benoitjacob@cloud:~/mlir-build$ cat /tmp/x.mlir
func.func @bad_broadcast(%arg0 : vector<1x2xi32>, %arg1 : vector<2xi32>, %arg2 : vector<1xi32>) -> vector<1xi32> {
  %result = vector.contract {
    indexing_maps = [
      affine_map<(d0, d1, d2) -> (d0, d2)>,
      affine_map<(d0, d1, d2) -> (d2)>,
      affine_map<(d0, d1, d2) -> (d1)>
    ],
    iterator_types = ["reduction", "parallel", "reduction"],
    kind = #vector.kind<add>} %arg0, %arg1, %arg2 : vector<1x2xi32>, vector<2xi32> into vector<1xi32>
  return  %result : vector<1xi32>
}

[jpienaar]

mlir/lib/Dialect/Vector/IR/VectorOps.cpp

702

Convention for error message are sentence fragments (no capitalization at start and no punctuation at end), https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

Is it helpful to say which input dimension are unused?

[nicolasvasilache]

thanks! please also add a test to invalid.mlir to test we report this.

[Benoit]

review comments, test, lint

[Benoit]

mlir/lib/Dialect/Vector/IR/VectorOps.cpp

702

The unused input dimension is plain to see in the op dump which follows this error message, see example in this review's comment thread.

mlir/lib/Dialect/Vector/IR/VectorOps.cpp

for (auto pair :
       VectorType v = pair.first;
       auto map = pair.second;
       for (unsigned idx = 0, e = v.getRank(); idx < e; ++idx) {
         unsigned pos = map.getDimPosition(idx);
         if (!extents[pos])
           extents[pos] = getAffineConstantExpr(v.getShape()[idx], ctx);
       }
     }
-    assert(llvm::all_of(extents, [](AffineExpr e) { return e; }) &&
-           "expected extent along all dimensions.");
+    if (!llvm::all_of(extents, [](AffineExpr e) { return e; }))
+      return op.emitOpError("expected all input dimensions to be used by "
+                            "either the LHS or the RHS");
 
     AffineMap resMap = op.getIndexingMaps()[2];
     auto extentsMap = AffineMap::get(/*dimCount=*/extents.size(),
                                      /*symCount=*/0, extents, ctx);
     // Compose the resMap with the extentsMap, which is a constant map.
     AffineMap expectedMap = simplifyAffineMap(resMap.compose(extentsMap));
     assert(llvm::all_of(
                expectedMap.getResults(),

mlir/test/Dialect/Vector/invalid.mlir

 // expected-error@+1 {{invalid accumulator/result vector shape, expected: 'vector<3x2xf32>'}}
   %0 = vector.contract #contraction_trait %arg0, %arg1, %arg2
     : vector<2x1xf32>, vector<1x3xf32> into vector<2x3xf32>
   return %0 : vector<2x3xf32>
 }
 
 // -----
 
+func.func @contract_with_dim_unused_by_lhs_and_rhs(%arg0 : vector<1x2xi32>, %arg1 : vector<2xi32>, %arg2 : vector<1xi32>) -> vector<1xi32> {
+// expected-error@+1 {{'vector.contract' op expected all input dimensions to be used by either the LHS or the RHS}}
+  %result = vector.contract {
+    indexing_maps = [
+      affine_map<(d0, d1, d2) -> (d0, d2)>,
+      affine_map<(d0, d1, d2) -> (d2)>,
+      affine_map<(d0, d1, d2) -> (d1)>
+    ],
+    iterator_types = ["reduction", "parallel", "reduction"],
+    kind = #vector.kind<add>} %arg0, %arg1, %arg2 : vector<1x2xi32>, vector<2xi32> into vector<1xi32>
+  return  %result : vector<1xi32>
+}
+
+// -----
+
 func.func @create_mask_0d_no_operands() {
   %c1 = arith.constant 1 : index
   // expected-error@+1 {{must specify exactly one operand for 0-D create_mask}}
   %0 = vector.create_mask : vector<i1>
 }
 
 // -----