[Mlir-commits] [mlir] [mlir][Vector] Fix scalable InsertSlice/ExtractSlice lowering (PR #124861)

[Andrzej Warzyński]

banach-space wrote:

Sorry for being a bit slow, reading this on a tablet and the GitHub app is far from ideal.

> I'm not sure why this wasn't caught by the verifier

Have you checked the debug dump? Sometimes, an intermediate IR might be invalid while the overall result remains valid. In such cases, `mlir-opt` would return success. This could help in designing a dedicated test for the issue.

> The bug is triggered by existing tests

If the bug is triggered by existing tests, shouldn't they be updated as part of this PR? I suspect this isn’t feasible because the affected patterns aren’t tested in isolation—highlighting a gap in Vector dialect testing. The fact that only `mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir` is updated supports this point (as I also alluded to in #124308).

> As mentioned, we generated valid IR but going through an intermediate invalid vector shuffle.

By "IR," do you mean LLVM IR or the intermediate representation being checked?

> Happy to add any extra test you think makes sense

Yes, that would be great. The vector-to-llvm pipeline feels like a jackhammer when used to test such a nuance in a very specific pattern.

Sadly, the only relevant-looking test file that I could find is this:
* https://github.com/llvm/llvm-project/blob/main/mlir/test/Dialect/Vector/vector-extract-strided-slice-lowering.mlir

But I'm on a tablet using GitHub app, so I'm pretty limited with my search.

---

**Tl;Dr** The fix is correct and much appreciated—I’ll approve this to unblock you. Let’s add a TODO for a dedicated test for this pattern. I can take care of it when adding tests for scalable vectors.


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