[Mlir-commits] [mlir] [mlir][Vector] Fix out-of-bounds crash unrolling create_mask/constant_mask with rank-mismatched native shape (PR #217514)

[Alessandro Potenza]

https://github.com/alepot55 commented:

Two things from reading the branch. I have not built it, so nothing below is an execution result.

**The description no longer describes the patch.** It still says the fix is to bail out with a match failure when the ranks do not match, and the `Verified` paragraph is about that behaviour. The patch pads the target shape and unrolls instead, which is the opposite outcome for the user. LLVM squashes the PR description into the commit message, so as it stands this lands a commit documenting a patch that is not there. The `constant_mask` empty-tile fix is missing from both the description and the title as well, and per the thread above it is a second, independent bug rather than a consequence of the rank mismatch.

**The asymmetry between the two patterns is right, but nothing in the diff says why.** I went looking for whether `UnrollCreateMaskPattern` needs the same zero normalization. It does not, for reasons that are not visible locally:

- `vector.create_mask` carries its dim sizes as operands and its verifier has no conjunction rule, so a tile with a zero in one dimension and non-zero in another is a valid op.
- `CreateMaskOp` has `hasCanonicalizer` but no folder, so the `createOrFold<vector::CreateMaskOp>` in the pattern cannot rewrite it into a `constant_mask` on the spot, even when every operand has folded to a constant.
- When the canonicalizer does run later, `CreateMaskFolder` in `VectorOps.cpp` already applies exactly the rule you added:

```cpp
// If one of dim sizes is zero, set all dims to zero.
if (llvm::is_contained(constantDims, 0))
  constantDims.assign(constantDims.size(), 0);
```

So the new block in `UnrollConstantMaskPattern` follows an existing convention rather than inventing one, and the `create_mask` path reaches the same normalization by another route. Worth a one-line comment pointing at `CreateMaskFolder`; it answers the "is this complete?" question that a reviewer otherwise has to re-derive.

On the assert in `padTargetShapeToRank`: it holds. `computeShapeRatio` returns `std::nullopt` when `shape.size() < subShape.size()`, and `getTargetShape` bails on that, so a target shape with a higher rank than the op never reaches these patterns.

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