[Mlir-commits] [mlir] [MLIR] support dynamic indexing of `vector.maskedload` in `VectorEmulateNarrowTypes` (PR #115070)

[Andrzej Warzyński]

================
@@ -53,6 +53,7 @@ static FailureOr<Operation *> getCompressedMaskOp(OpBuilder &rewriter,
                                                   Location loc, Value mask,
                                                   int origElements, int scale,
                                                   int intraDataOffset = 0) {
+  assert(intraDataOffset < scale && "intraDataOffset must be less than scale");
----------------
banach-space wrote:

Here's my attempt to improve the comments and input variable names:
* https://github.com/llvm/llvm-project/pull/115663

Please let me know whether that makes sense to you, and any feedback is welcome.

Note, I've also created these two:
* https://github.com/llvm/llvm-project/pull/115612
* https://github.com/llvm/llvm-project/issues/115653

(again, your feedback would be appreciated). Last, but not least, this [example](https://github.com/llvm/llvm-project/blob/58a17e1bbc54357385d0b89cfc5635e402c31ef6/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp#L39-L51) seems off. In particular:

```cpp
/// [Comment from Andrzej] 6 elements
///  %mask = [1, 1, 0, 0, 0, 0]
///
/// will first be padded with number of `intraDataOffset` zeros:
/// [Comment from Andrzej] 8 elements != 6 + 1
///   %mask = [0, 1, 1, 0, 0, 0, 0, 0]
```

Shouldn't the padded mask be: `%mask = [0, 1, 1, 0, 0, 0, 0]` (7 elements)?

Btw, thanks so much for working on this - your efforts are truly appreciated! Please don’t let my comments (and appetite to improve things overall) give you any other impression 😅.

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