[PATCH] D72933: Adds CastSliceOp to the vector ops dialect.

Mon Jan 20 19:35:56 PST 2020

nicolasvasilache requested changes to this revision.
nicolasvasilache added inline comments.
This revision now requires changes to proceed.

================
Comment at: mlir/include/mlir/Dialect/VectorOps/VectorOps.td:964
+    ```mlir
+     // Cast slices of 'vector<4x1x3x12xf32>' from shape 1x1x3x12 to 3x12
+     %1 = vector.cast_slices %0, [1, 1, 3, 12], [3, 12]
----------------
I find the semantics harder to grasp than it could, I think.
I think the issue is that the **usage** you have for "cast_slices" leaks into the semantics of the op and I don't think it is warranted.
```
 %1 = vector.cast_slices %0, [1, 1, 3, 12], [3, 12]
       : vector<4x1x3x12xf32> to vector<12x12xf32>
```

I can think of different ways of specifying this that I would find more natural but first I'd like to understand why you need these attributes in this form and where ambiguity appears that makes it impossible to derive them automatically.

In particular I view this as conflating 2 unrelated things: (1) a reshape that does not move and (2) a multiplicity.
See how (1) is represented in linalg.reshape with [affine maps](https://github.com/llvm/llvm-project/blob/master/mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td#L92).

It would be great if we could uniformize things (if is makes sense).
Note that this may be an indication that you need 2 ops to express what you want.

Marking as "request changes" until the discussion is resolved. 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72933/new/

https://reviews.llvm.org/D72933