[Mlir-commits] [mlir] [mlir][vector] Only fold ext ops from the same type into vector.contract (PR #221298)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-vector

Author: Hung-Kuan Tseng (Tim096)

<details>
<summary>Changes</summary>

`FoldArithExtIntoContractionOp` checks that both contraction operands are produced by the same kind of extension, but not that the extensions start from the same element type. With `extsi` from i8 on one side and from i16 on the other it rewrites the contraction to take the two sources directly, and the result fails to verify:

```
'vector.contract' op failed to verify that lhs and rhs have same element type
```

The same happens with `extf` from f16 and bf16. Compare the source element types and leave such contractions alone.

Tests: one `extf` and one `extsi` case in `fold-arith-extf-into-vector-contract.mlir` that keep the extensions.


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


2 Files Affected:

- (modified) mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp (+8) 
- (modified) mlir/test/Dialect/Vector/fold-arith-extf-into-vector-contract.mlir (+47) 


``````````diff
diff --git a/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp b/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
index 123caf2c11b83..de34847fea3e4 100644
--- a/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
@@ -1915,6 +1915,14 @@ struct FoldArithExtIntoContractionOp
                                          "no defining op on contract operands");
     }
 
+    // The contraction requires lhs and rhs to have the same element type, so
+    // the two extensions must also start from the same one.
+    if (getElementTypeOrSelf(lhsDefOp.getIn().getType()) !=
+        getElementTypeOrSelf(rhsDefOp.getIn().getType())) {
+      return rewriter.notifyMatchFailure(
+          contractOp, "lhs and rhs are extended from different element types");
+    }
+
     rewriter.replaceOpWithNewOp<vector::ContractionOp>(
         contractOp, lhsDefOp->getOperand(0), rhsDefOp->getOperand(0),
         contractOp.getAcc(), contractOp.getIndexingMapsAttr(),
diff --git a/mlir/test/Dialect/Vector/fold-arith-extf-into-vector-contract.mlir b/mlir/test/Dialect/Vector/fold-arith-extf-into-vector-contract.mlir
index 6dbde7afbdd33..5266889647222 100644
--- a/mlir/test/Dialect/Vector/fold-arith-extf-into-vector-contract.mlir
+++ b/mlir/test/Dialect/Vector/fold-arith-extf-into-vector-contract.mlir
@@ -70,3 +70,50 @@ func.func @fold_arith_extsi_into_contract(
       %lhs_i32, %rhs_i32, %arg2 : vector<64x64xi32>, vector<64x64xi32> into vector<64x64xi32>
     return %result : vector<64x64xi32>
 }
+
+// -----
+
+// The contraction requires lhs and rhs to have the same element type, so the
+// extensions are only folded when they start from the same one.
+
+// CHECK-LABEL: func.func @no_fold_arith_extf_from_different_types
+//  CHECK-SAME: (%[[ARG0:.*]]: vector<64x64xf16>, %[[ARG1:.*]]: vector<64x64xbf16>, %[[ARG2:.*]]: vector<64x64xf32>)
+//       CHECK:   %[[LHS:.*]] = arith.extf %[[ARG0]] : vector<64x64xf16> to vector<64x64xf32>
+//       CHECK:   %[[RHS:.*]] = arith.extf %[[ARG1]] : vector<64x64xbf16> to vector<64x64xf32>
+//       CHECK:   vector.contract
+//  CHECK-SAME:   %[[LHS]], %[[RHS]], %[[ARG2]] : vector<64x64xf32>, vector<64x64xf32> into vector<64x64xf32>
+func.func @no_fold_arith_extf_from_different_types(
+  %arg0: vector<64x64xf16>,
+  %arg1: vector<64x64xbf16>,
+  %arg2: vector<64x64xf32>) -> vector<64x64xf32> {
+    %lhs_f32 = arith.extf %arg0 : vector<64x64xf16> to vector<64x64xf32>
+    %rhs_f32 = arith.extf %arg1 : vector<64x64xbf16> to vector<64x64xf32>
+    %result = vector.contract {
+      indexing_maps = [affine_map<(d0, d1, d2) -> (d0, d2)>, affine_map<(d0, d1, d2) -> (d2, d1)>, affine_map<(d0, d1, d2) -> (d0, d1)>],
+      iterator_types = ["parallel", "parallel", "reduction"],
+      kind = #vector.kind<add>}
+      %lhs_f32, %rhs_f32, %arg2 : vector<64x64xf32>, vector<64x64xf32> into vector<64x64xf32>
+    return %result : vector<64x64xf32>
+}
+
+// -----
+
+// CHECK-LABEL: func.func @no_fold_arith_extsi_from_different_types
+//  CHECK-SAME: (%[[ARG0:.*]]: vector<64x64xi8>, %[[ARG1:.*]]: vector<64x64xi16>, %[[ARG2:.*]]: vector<64x64xi32>)
+//       CHECK:   %[[LHS:.*]] = arith.extsi %[[ARG0]] : vector<64x64xi8> to vector<64x64xi32>
+//       CHECK:   %[[RHS:.*]] = arith.extsi %[[ARG1]] : vector<64x64xi16> to vector<64x64xi32>
+//       CHECK:   vector.contract
+//  CHECK-SAME:   %[[LHS]], %[[RHS]], %[[ARG2]] : vector<64x64xi32>, vector<64x64xi32> into vector<64x64xi32>
+func.func @no_fold_arith_extsi_from_different_types(
+  %arg0: vector<64x64xi8>,
+  %arg1: vector<64x64xi16>,
+  %arg2: vector<64x64xi32>) -> vector<64x64xi32> {
+    %lhs_i32 = arith.extsi %arg0 : vector<64x64xi8> to vector<64x64xi32>
+    %rhs_i32 = arith.extsi %arg1 : vector<64x64xi16> to vector<64x64xi32>
+    %result = vector.contract {
+      indexing_maps = [affine_map<(d0, d1, d2) -> (d0, d2)>, affine_map<(d0, d1, d2) -> (d2, d1)>, affine_map<(d0, d1, d2) -> (d0, d1)>],
+      iterator_types = ["parallel", "parallel", "reduction"],
+      kind = #vector.kind<add>}
+      %lhs_i32, %rhs_i32, %arg2 : vector<64x64xi32>, vector<64x64xi32> into vector<64x64xi32>
+    return %result : vector<64x64xi32>
+}

``````````

</details>


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