[Mlir-commits] [mlir] [mlir][vector] Restrict narrow-type-emulation patterns (PR #115612)

[Andrzej Warzyński]

================
@@ -225,6 +225,10 @@ struct ConvertVectorStore final : OpConversionPattern<vector::StoreOp> {
   matchAndRewrite(vector::StoreOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
 
+    if (op.getValueToStore().getType().getRank() != 1)
----------------
banach-space wrote:

> This might be a bug.

This _is a bug_ :) In fact, one of many. Please see the summary ;-)

My PR is effectively a bug report. In fact, **I should've started with a bug report**. This is now reported here:
* https://github.com/llvm/llvm-project/issues/115653

> See comment below. It is explicitly written for multi-dimensional loads. The only general way to emulate sub-byte loads is to linearize the memrefs and do a linear store.

Yes, two things need to happen: _linearization_ + _bitcasting_. The former (_linearization_) seems to work fine only for source/destination `memref`(s). For vectors, it appears to be broken. For reference, see the reproduces that I added as tests.

> I am not opposed to having this, but seems too big a hammer. There is a bug here for multi-dimensional stores.

IIUC, we agree that there are multiple bugs here? This should be fixed, but in the meantime, lets document these "discoveries" through:
* a bug report #115653,
* reproducers added in this PR,
* code (explicit pattern failures for code-paths that are known to be broken).

How does it sound?

---- 
As a side note ...
>  So during the emulation the destination memref and the source vector get converted to 1D before the store.

>From what I can tell, dealing with n-D vectors is going to be tricky and might take some time (especially when masking is involved). I'd start by making sure 3 basic cases are covered:
* 1-D `memref` + 1-D `vector`,
* 2-D `memref` + 1-D `vector`,
* 2-D `memref` + 2-D `vector`.

Top 2 seem to be already supported. The bottom one is not. I haven't thought of n-D cases yet (n > 2), but perhaps that's trivial once 2-D is fully supported.

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