[Mlir-commits] [mlir] [mlir][Vector] Infer mask and pass_thru types for maskedload/store (PR #131482)

[Andrzej Warzyński]

https://github.com/banach-space commented:

Hi Kunwar, thanks for looking at ways to improve the codebase!

To be perfectly honest, I actually prefer the longer and more explicit spelling that we have today 😅. The cognitive load here:
```mlir
%0 = vector.maskedload %arg0[%c0], %arg1, %arg2 : memref<?xindex>, vector<[16]xi1>, vector<[16]xindex> into vector<[16]xindex>
```

feels much lower than in:

```mlir
%0 = vector.maskedload %arg0[%c0], %arg1, %arg2 : memref<?xindex>, vector<[16]xindex> into vector<[16]xindex>
```

With an explicit type for every argument, it's immediately clear what each type corresponds to. By contrast, in the shorter version, it’s unclear which type belongs to which operand:

* `%arg0` + `%arg1`,
* `%arg0` + `%arg2`,
* `%arg1` + `%arg2`.

Sure, things take some getting used to, and perhaps over time, the proposed format would feel more natural. However, I’m particularly concerned about **newcomers** to `Vector`. IMHO, they would likely find this quite confusing - _is there a clear way to map each type to its corresponding argument_?

> The type format now exactly matches vector.load and vector.store, with the only difference being that one takes a mask and the other doesn’t.

But isn’t this what we want to avoid? If the distinction between `vector.load` and `vector.maskedload` becomes too blurred, it may lead to more confusion rather than clarity.

Additionally, I’ve noticed that the auto-generated error messages aren’t as helpful with the shorter format.

Some of these things are a matter or personal preference -  I've tried to explain my reasoning so that you know where I am coming from. I think that it would be good to hear from more people.

Thanks again for the effort - I really appreciate the time you put into this!

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