[PATCH] D157005: [mlir][ArmSME] Add vector to tile slice op and lowerings
Cullen Rhodes via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 22 06:50:32 PDT 2023
c-rhodes added inline comments.
================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:417
+def VectorToTileSliceOp : ArmSME_Op<"vector_to_tile_slice", [
+ AllTypesMatch<["tile", "result"]>
----------------
awarzynski wrote:
> c-rhodes wrote:
> > awarzynski wrote:
> > > Most Ops' names convey what the corresponding "action"/"function" is. Wondering whether this shouldn't be `MoveVectorToTileSliceOp` instead. Naming is hard.
> > > Most Ops' names convey what the corresponding "action"/"function" is. Wondering whether this shouldn't be `MoveVectorToTileSliceOp` instead. Naming is hard.
> >
> > are you suggesting to rename it both internally and externally (i.e. `move_vector_to_tile_slice`) or just the former?
> I was thinking both.
> I was thinking both.
Still considering, it's more descriptive but also it's quite a long name
================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:424
+ (2-D scalable vector) slice at the given index. The 1-D vector type must
+ match the inner vector type of the 2-D vector type that represents the
+ tile. The updated tile is returned as the result.
----------------
awarzynski wrote:
> c-rhodes wrote:
> > dcaballe wrote:
> > > -> element type?
> > > -> element type?
> >
> > For a 2-d scalable vector such as `vector<[4]x[4]xi32>` is the element type `vector<[4]xi32>`? That's what I want to express here but I do struggle with these descriptions. FWIW `getElementType()` returns the scalar `i32`. Perhaps `The 1-D vector type must match the vector type of the inner dimension of the 2-D vector type`?
> I also though that you meant the element type here, as in e.g. `i32` or `f32`.
>
> I think that we need to be more explicit here:
>
> > The type of the 1-d scalable vector to be moved must match the type of the tile slice (note that 1 slice is effectively 1 row in a virtual tile).
>
> WDYT?
> I also though that you meant the element type here, as in e.g. `i32` or `f32`.
>
> I think that we need to be more explicit here:
>
> > The type of the 1-d scalable vector to be moved must match the type of the tile slice (note that 1 slice is effectively 1 row in a virtual tile).
>
> WDYT?
I've updated the description along these lines, hope it makes more sense now, appreciate the input!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157005/new/
https://reviews.llvm.org/D157005
More information about the llvm-commits
mailing list