[Mlir-commits] [mlir] [mlir][vector] Add vector.transpose with unit-dim to vector.shape_cast pattern (PR #72105)

[llvmlistbot at llvm.org]

MaheshRavishankar wrote:

> > . The solution here as Lei suggested is to move it out of general Vector lowering of transpose and use a separate populate method to use for different back ends.
> 
> I read Lei's message 4 times, but I don't quite see what you're referring to here?

Sorry, conversation is happening in different places. I was refering to this comment https://github.com/llvm/llvm-project/pull/72493#issuecomment-1821906139

> 
> I don't know what the "use a separate populate method to use for different back ends", but in general I would be concerned that we can't add canonicalization to the vector dialect because SPIRV is lacking support for lowering. That is a canonicalization isn't supposed to "lose information", so I assume that any problem is just "incompleteness in support of the vector dialect by the SPIRV lowering", but you can't just prevent adding canonicalization to the vector dialect for this reason: instead SPIRV folks needs to 1) have better support for the vector dialect and 2) be reactive in fixing the lowering when such issues are found.

I would say this is the canonicalization cause of the following reason. Lets say I have an analysis that is look for `vector.transpose` and is general enough to work for dynamic cases.

```
%1 = vector.transpose %0 [1, 0] : vector<?x?xf32> to vector<?x?xf32>
```

Now the same analysis on static case can get

```
%1 = vector.transpose %0 [1, 0] : vector<1x4xf32> to vector<4x1xf32>
```

and then canonicalization would have then turned it into 

```
%1 = vector.shape_cast : vector<1x4xf32> to vector<4x1xf32>
```

breaking my analysis. To recover the information of the fact that the `shape_cast` is a `transpose` I would have to effectively undo what this canonicalization did in my analysis. So in my book this doesnt fit as a canonicalization.


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