[Mlir-commits] [mlir] [mlir][VectorToSCF] Decline transfer ops masked by vector.mask (PR #221307)
Hung-Kuan Tseng
llvmlistbot at llvm.org
Mon Sep 14 08:02:15 PDT 2026
https://github.com/Tim096 updated https://github.com/llvm/llvm-project/pull/221307
>From af0e5de00d82303d8a2cc7526a90f004e7404884 Mon Sep 17 00:00:00 2001
From: Hung-Kuan Tseng <tseng.tim096 at gmail.com>
Date: Sat, 5 Sep 2026 02:06:37 +0800
Subject: [PATCH 1/2] [mlir][VectorToSCF] Decline transfer ops masked by
vector.mask
The transfer op patterns in convert-vector-to-scf unpack a
`vector.transfer_read`/`vector.transfer_write` into a loop, a buffer and
lower-rank transfer ops, but never check whether the op sits inside a
`vector.mask`. In that case all of it lands in the mask region and the
pass fails with
'vector.mask' op expects only one operation to mask
The unrolled, 1-D and scalable-transpose paths do the same, and the
transfer ops they emit carry no mask at all. Decline masked transfer
ops in every pattern instead; `-lower-vector-mask` turns the
`vector.mask` into a mask operand, which the patterns already handle.
---
.../Conversion/VectorToSCF/VectorToSCF.cpp | 15 ++++
.../Conversion/VectorToSCF/vector-to-scf.mlir | 72 +++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index f1085e3c86b361..2a387236ecfdd9 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -553,6 +553,9 @@ static LogicalResult checkPrepareXferOp(OpTy xferOp, PatternRewriter &rewriter,
if (xferOp->hasAttr(kPassLabel))
return rewriter.notifyMatchFailure(
xferOp, "kPassLabel is present (vector-to-scf lowering in progress)");
+ if (xferOp.isMasked())
+ return rewriter.notifyMatchFailure(
+ xferOp, "masked by vector.mask, lower the mask first");
if (xferOp.getVectorType().getRank() <= options.targetRank)
return rewriter.notifyMatchFailure(
xferOp, "xferOp vector rank <= transformation target rank");
@@ -1078,6 +1081,9 @@ struct ScalableTransposeTransferWriteConversion
PatternRewriter &rewriter) const override {
if (failed(checkLowerTensors(writeOp, rewriter)))
return failure();
+ if (writeOp.isMasked())
+ return rewriter.notifyMatchFailure(
+ writeOp, "masked by vector.mask, lower the mask first");
VectorType vectorType = writeOp.getVectorType();
@@ -1304,6 +1310,9 @@ struct UnrollTransferReadConversion
xferOp, "vector rank is less or equal to target rank");
if (failed(checkLowerTensors(xferOp, rewriter)))
return failure();
+ if (xferOp.isMasked())
+ return rewriter.notifyMatchFailure(
+ xferOp, "masked by vector.mask, lower the mask first");
if (xferOp.getVectorType().getElementType() !=
xferOp.getShapedType().getElementType())
return rewriter.notifyMatchFailure(
@@ -1450,6 +1459,9 @@ struct UnrollTransferWriteConversion
if (failed(checkLowerTensors(xferOp, rewriter)))
return failure();
+ if (xferOp.isMasked())
+ return rewriter.notifyMatchFailure(
+ xferOp, "masked by vector.mask, lower the mask first");
// Transfer ops that modify the element type are not supported atm.
if (inputVectorTy.getElementType() !=
xferOp.getShapedType().getElementType())
@@ -1660,6 +1672,9 @@ struct TransferOp1dConversion : public VectorToSCFPattern<OpTy> {
// TODO: support 0-d corner case.
if (xferOp.getTransferRank() == 0)
return failure();
+ if (xferOp.isMasked())
+ return rewriter.notifyMatchFailure(
+ xferOp, "masked by vector.mask, lower the mask first");
auto map = xferOp.getPermutationMap();
auto memRefType = dyn_cast<MemRefType>(xferOp.getShapedType());
diff --git a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
index e3a7f57e144ed6..d47d2508549dda 100644
--- a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
+++ b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
@@ -923,3 +923,75 @@ func.func private @transfer_write_no_alloc_scope()
%cst = arith.constant dense<0.0> : vector<2x3xf32>
%m = memref.alloc() : memref<2x3xf32>
vector.transfer_write %cst, %m[%c0, %c0] : vector<2x3xf32>, memref<2x3xf32>
+
+// -----
+
+// Negative tests: a transfer op inside `vector.mask` is left alone, since the
+// mask region can only hold that one op. The mask has to be lowered first.
+
+// CHECK-LABEL: @masked_transfer_read
+// CHECK: vector.mask %{{.*}} { vector.transfer_read
+// CHECK-NOT: scf.for
+// FULL-UNROLL-LABEL: @masked_transfer_read
+// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_read
+// FULL-UNROLL-NOT: vector.extract
+// TARGET-RANK-ZERO-LABEL: @masked_transfer_read
+// TARGET-RANK-ZERO: vector.mask %{{.*}} { vector.transfer_read
+// TARGET-RANK-ZERO-NOT: vector.extract
+func.func @masked_transfer_read(%mem: memref<?x?xf32>, %mask: vector<4x8xi1>) -> vector<4x8xf32> {
+ %c0 = arith.constant 0 : index
+ %pad = arith.constant 0.0 : f32
+ %0 = vector.mask %mask { vector.transfer_read %mem[%c0, %c0], %pad : memref<?x?xf32>, vector<4x8xf32> } : vector<4x8xi1> -> vector<4x8xf32>
+ return %0 : vector<4x8xf32>
+}
+
+// -----
+
+// CHECK-LABEL: @masked_transfer_write
+// CHECK: vector.mask %{{.*}} { vector.transfer_write
+// CHECK-NOT: scf.for
+// FULL-UNROLL-LABEL: @masked_transfer_write
+// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_write
+// FULL-UNROLL-NOT: vector.extract
+// TARGET-RANK-ZERO-LABEL: @masked_transfer_write
+// TARGET-RANK-ZERO: vector.mask %{{.*}} { vector.transfer_write
+// TARGET-RANK-ZERO-NOT: vector.extract
+func.func @masked_transfer_write(%vec: vector<4x8xf32>, %mem: memref<?x?xf32>, %mask: vector<4x8xi1>) {
+ %c0 = arith.constant 0 : index
+ vector.mask %mask { vector.transfer_write %vec, %mem[%c0, %c0] : vector<4x8xf32>, memref<?x?xf32> } : vector<4x8xi1>
+ return
+}
+
+// -----
+
+// CHECK-LABEL: @masked_transfer_read_1d_strided
+// CHECK: vector.mask %{{.*}} { vector.transfer_read
+// CHECK-NOT: scf.for
+// FULL-UNROLL-LABEL: @masked_transfer_read_1d_strided
+// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_read
+// FULL-UNROLL-NOT: scf.for
+// TARGET-RANK-ZERO-LABEL: @masked_transfer_read_1d_strided
+// TARGET-RANK-ZERO: vector.mask %{{.*}} { vector.transfer_read
+// TARGET-RANK-ZERO-NOT: scf.for
+func.func @masked_transfer_read_1d_strided(%mem: memref<?x?xf32>, %mask: vector<8xi1>) -> vector<8xf32> {
+ %c0 = arith.constant 0 : index
+ %pad = arith.constant 0.0 : f32
+ %0 = vector.mask %mask { vector.transfer_read %mem[%c0, %c0], %pad {permutation_map = affine_map<(d0, d1) -> (d0)>} : memref<?x?xf32>, vector<8xf32> } : vector<8xi1> -> vector<8xf32>
+ return %0 : vector<8xf32>
+}
+
+// -----
+
+// CHECK-LABEL: @masked_scalable_transpose_store
+// CHECK: vector.mask %{{.*}} { vector.transfer_write
+// CHECK-NOT: scf.for
+// FULL-UNROLL-LABEL: @masked_scalable_transpose_store
+// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_write
+// FULL-UNROLL-NOT: scf.for
+func.func @masked_scalable_transpose_store(%vec: vector<[4]x4xf32>, %mem: memref<?x?xf32>, %a: index, %b: index) {
+ %c0 = arith.constant 0 : index
+ %transpose = vector.transpose %vec, [1, 0] : vector<[4]x4xf32> to vector<4x[4]xf32>
+ %mask = vector.create_mask %a, %b : vector<4x[4]xi1>
+ vector.mask %mask { vector.transfer_write %transpose, %mem[%c0, %c0] : vector<4x[4]xf32>, memref<?x?xf32> } : vector<4x[4]xi1>
+ return
+}
>From 16645262387e6620eb47ee6e9ec5bca5065e455a Mon Sep 17 00:00:00 2001
From: Hung-Kuan Tseng <tseng.tim096 at gmail.com>
Date: Mon, 14 Sep 2026 23:01:54 +0800
Subject: [PATCH 2/2] Name the negative tests after their pattern, and let the
scalable one reach it
---
.../Conversion/VectorToSCF/vector-to-scf.mlir | 53 +++++++++++--------
1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
index d47d2508549dda..998cb4d69e068d 100644
--- a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
+++ b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
@@ -926,19 +926,23 @@ vector.transfer_write %cst, %m[%c0, %c0] : vector<2x3xf32>, memref<2x3xf32>
// -----
+//-----------------------------------------------------------------------------
// Negative tests: a transfer op inside `vector.mask` is left alone, since the
// mask region can only hold that one op. The mask has to be lowered first.
+//-----------------------------------------------------------------------------
-// CHECK-LABEL: @masked_transfer_read
+// [Pattern: PrepareTransferReadConversion, UnrollTransferReadConversion]
+
+// CHECK-LABEL: @negative_masked_transfer_read
// CHECK: vector.mask %{{.*}} { vector.transfer_read
// CHECK-NOT: scf.for
-// FULL-UNROLL-LABEL: @masked_transfer_read
+// FULL-UNROLL-LABEL: @negative_masked_transfer_read
// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_read
// FULL-UNROLL-NOT: vector.extract
-// TARGET-RANK-ZERO-LABEL: @masked_transfer_read
+// TARGET-RANK-ZERO-LABEL: @negative_masked_transfer_read
// TARGET-RANK-ZERO: vector.mask %{{.*}} { vector.transfer_read
// TARGET-RANK-ZERO-NOT: vector.extract
-func.func @masked_transfer_read(%mem: memref<?x?xf32>, %mask: vector<4x8xi1>) -> vector<4x8xf32> {
+func.func @negative_masked_transfer_read(%mem: memref<?x?xf32>, %mask: vector<4x8xi1>) -> vector<4x8xf32> {
%c0 = arith.constant 0 : index
%pad = arith.constant 0.0 : f32
%0 = vector.mask %mask { vector.transfer_read %mem[%c0, %c0], %pad : memref<?x?xf32>, vector<4x8xf32> } : vector<4x8xi1> -> vector<4x8xf32>
@@ -947,16 +951,18 @@ func.func @masked_transfer_read(%mem: memref<?x?xf32>, %mask: vector<4x8xi1>) ->
// -----
-// CHECK-LABEL: @masked_transfer_write
+// [Pattern: PrepareTransferWriteConversion, UnrollTransferWriteConversion]
+
+// CHECK-LABEL: @negative_masked_transfer_write
// CHECK: vector.mask %{{.*}} { vector.transfer_write
// CHECK-NOT: scf.for
-// FULL-UNROLL-LABEL: @masked_transfer_write
+// FULL-UNROLL-LABEL: @negative_masked_transfer_write
// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_write
// FULL-UNROLL-NOT: vector.extract
-// TARGET-RANK-ZERO-LABEL: @masked_transfer_write
+// TARGET-RANK-ZERO-LABEL: @negative_masked_transfer_write
// TARGET-RANK-ZERO: vector.mask %{{.*}} { vector.transfer_write
// TARGET-RANK-ZERO-NOT: vector.extract
-func.func @masked_transfer_write(%vec: vector<4x8xf32>, %mem: memref<?x?xf32>, %mask: vector<4x8xi1>) {
+func.func @negative_masked_transfer_write(%vec: vector<4x8xf32>, %mem: memref<?x?xf32>, %mask: vector<4x8xi1>) {
%c0 = arith.constant 0 : index
vector.mask %mask { vector.transfer_write %vec, %mem[%c0, %c0] : vector<4x8xf32>, memref<?x?xf32> } : vector<4x8xi1>
return
@@ -964,16 +970,21 @@ func.func @masked_transfer_write(%vec: vector<4x8xf32>, %mem: memref<?x?xf32>, %
// -----
-// CHECK-LABEL: @masked_transfer_read_1d_strided
+// [Pattern: TransferOp1dConversion]
+// The permutation map makes this a strided 1-D read, which goes through
+// TransferOp1dConversion rather than the patterns above (TARGET-RANK-ZERO
+// unrolls it first).
+
+// CHECK-LABEL: @negative_masked_transfer_read_1d_strided
// CHECK: vector.mask %{{.*}} { vector.transfer_read
// CHECK-NOT: scf.for
-// FULL-UNROLL-LABEL: @masked_transfer_read_1d_strided
+// FULL-UNROLL-LABEL: @negative_masked_transfer_read_1d_strided
// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_read
// FULL-UNROLL-NOT: scf.for
-// TARGET-RANK-ZERO-LABEL: @masked_transfer_read_1d_strided
+// TARGET-RANK-ZERO-LABEL: @negative_masked_transfer_read_1d_strided
// TARGET-RANK-ZERO: vector.mask %{{.*}} { vector.transfer_read
// TARGET-RANK-ZERO-NOT: scf.for
-func.func @masked_transfer_read_1d_strided(%mem: memref<?x?xf32>, %mask: vector<8xi1>) -> vector<8xf32> {
+func.func @negative_masked_transfer_read_1d_strided(%mem: memref<?x?xf32>, %mask: vector<8xi1>) -> vector<8xf32> {
%c0 = arith.constant 0 : index
%pad = arith.constant 0.0 : f32
%0 = vector.mask %mask { vector.transfer_read %mem[%c0, %c0], %pad {permutation_map = affine_map<(d0, d1) -> (d0)>} : memref<?x?xf32>, vector<8xf32> } : vector<8xi1> -> vector<8xf32>
@@ -982,16 +993,16 @@ func.func @masked_transfer_read_1d_strided(%mem: memref<?x?xf32>, %mask: vector<
// -----
-// CHECK-LABEL: @masked_scalable_transpose_store
-// CHECK: vector.mask %{{.*}} { vector.transfer_write
-// CHECK-NOT: scf.for
-// FULL-UNROLL-LABEL: @masked_scalable_transpose_store
+// [Pattern: ScalableTransposeTransferWriteConversion]
+// @scalable_transpose_store_unmasked inside a `vector.mask`. Only the
+// FULL-UNROLL RUN line sets lower-scalable, so only it reaches this pattern.
+
+// FULL-UNROLL-LABEL: @negative_masked_scalable_transpose_store
// FULL-UNROLL: vector.mask %{{.*}} { vector.transfer_write
// FULL-UNROLL-NOT: scf.for
-func.func @masked_scalable_transpose_store(%vec: vector<[4]x4xf32>, %mem: memref<?x?xf32>, %a: index, %b: index) {
- %c0 = arith.constant 0 : index
- %transpose = vector.transpose %vec, [1, 0] : vector<[4]x4xf32> to vector<4x[4]xf32>
- %mask = vector.create_mask %a, %b : vector<4x[4]xi1>
- vector.mask %mask { vector.transfer_write %transpose, %mem[%c0, %c0] : vector<4x[4]xf32>, memref<?x?xf32> } : vector<4x[4]xi1>
+func.func @negative_masked_scalable_transpose_store(%vec: vector<4x[4]xf32>, %dest: memref<?x?xf32>, %i: index, %j: index, %a: index, %b: index) {
+ %transpose = vector.transpose %vec, [1, 0] : vector<4x[4]xf32> to vector<[4]x4xf32>
+ %mask = vector.create_mask %a, %b : vector<[4]x4xi1>
+ vector.mask %mask { vector.transfer_write %transpose, %dest[%i, %j] {in_bounds = [true, true]} : vector<[4]x4xf32>, memref<?x?xf32> } : vector<[4]x4xi1>
return
}
More information about the Mlir-commits
mailing list