[Mlir-commits] [mlir] [MLIR] Fix VectorEmulateNarrowType constant op mask bug (PR #116064)

[Han-Chung Wang]

================
@@ -129,6 +132,45 @@ static FailureOr<Operation *> getCompressedMaskOp(OpBuilder &rewriter,
       auto denseAttr = DenseElementsAttr::get(newMaskType, newMaskValues);
       newMask = rewriter.create<arith::ConstantOp>(loc, newMaskType, denseAttr);
     }
+  } else if (constantOp) {
+    assert(shape.size() == 1 && "expected 1-D mask");
+    // Rearrange the original mask values to cover the whole potential loading
+    // region. For example, in the case of using byte-size for emulation, given
+    // the following mask:
+    //
+    //   %mask = vector.constant_mask [0, 1, 0, 1, 0, 0] : vector<6xi2>
----------------
hanhanW wrote:

I'm confused about `vector.constant_mask` here. The comment looks off to me because of the definition of the vector.constant_mask op: https://github.com/llvm/llvm-project/blob/00f2989f98520c401f0ab544a3dc766ed83785c0/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td#L2347-L2383

Do you mean `arith.constant`? Also, should the type be i1? To simplify the comment, we can omit the op name and types, which looks cleaner to me. The idea of having the comment is to help people understand the logic. IMO, we don't need the concrete ops at this scope. So it could be something like `%mask = [1, 1, 0, 0, 0, 0]` (and it is how we document in the above function comment).

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