Add a pattern to take a rank-reducing subview and drop inner most
contiguous unit dim.
This is useful when lowering vector to backends with 1d vector types.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| mlir/include/mlir/Dialect/Vector/VectorTransforms.h | ||
|---|---|---|
| 104 | I wonder if we should have a more specific name. There is a different pattern called TransferOpReduceRank that does significantly different kind of transformation. | |
| mlir/lib/Dialect/Vector/VectorTransforms.cpp | ||
| 3826 | That sounds too restrictive, we should allow minor identity. (and ideally add a test for it) | |
| 3829–3830 | nit: srcType.getAffineMaps().size() <= 1? | |
| 3834 | nit: use getVectorType() | |
| 3843–3848 | Can we just iterate reverse through the strides and break whenever it is != 1? | |
| 3846 | nit: should be break in the else case? | |
| 3866–3868 | Should that be outside the loop? | |
| mlir/include/mlir/Dialect/Vector/VectorTransforms.h | ||
|---|---|---|
| 104 | about about reduceTransferOpsOperandRank ? | |
| mlir/lib/Dialect/Vector/VectorTransforms.cpp | ||
|---|---|---|
| 3826 | minor identity transfer read is the canonical form as far as I know. | |
| 3843–3848 | Can we just iterate reverse? just a suggestion though, feel free to ignore. for(int64_t stride : llvm::reverse(scrStrides) {
  if(stride != 1)
    break;
   dimsToDrop++;
} | |
| 3870–3876 | nit: can those two be merged? I realize this requires to change a bit how map is handled, just wondering if this makes the code a bit simpler, feel free to ignore. | |
| mlir/include/mlir/Dialect/Vector/VectorTransforms.h | ||
|---|---|---|
| 104 | 
 | |
| mlir/include/mlir/Dialect/Vector/VectorTransforms.h | ||
|---|---|---|
| 104 | If this is meant to only drop inner unit dimension I would include it in the name. What do you think? Or is the idea to collapse all the contiguous inner dimension? I wonder if we can make the goal a bit more explicit in the name. | |
| mlir/include/mlir/Dialect/Vector/VectorTransforms.h | ||
|---|---|---|
| 104 | I agree, collapseInnerMostContiguousDims is the goal of these patterns. I will go with this naming. Thanks! | |
| mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir | ||
|---|---|---|
| 7 ↗ | (On Diff #378781) | Can you make the dest vector<1x8x1xf32> to show how this works when vector rank < memref rank | 
I wonder if we should have a more specific name. There is a different pattern called TransferOpReduceRank that does significantly different kind of transformation.