[PATCH] D156467: [mlir][ArmSME] Add conversion from ArmSME to SCF to materialize loops

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes marked 6 inline comments as done.
c-rhodes added inline comments.


================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:311
 
+def TileSliceLoadAndUpdateOp : ArmSME_Op<"tile_slice_load_and_update", [
+    TypesMatchWith<"expected result type to match tile type",
----------------
WanderAway wrote:
> I feel like the name is a bit confusing, although I'm not sure I can come up with something better...
> `LoadTileSlice`? `LoadVectorToTile`?
> 
> Also I wonder if it makes sense for this to have a horizontal/vertical load variant/argument, so that it would be possible to do in-flight transpose as well?
> I feel like the name is a bit confusing, although I'm not sure I can come up with something better...
> `LoadTileSlice`? `LoadVectorToTile`?
> 
> Also I wonder if it makes sense for this to have a horizontal/vertical load variant/argument, so that it would be possible to do in-flight transpose as well?

It was rather verbose but until Diego pointed out the passthru on other vectors ops I wasn't aware of any ops that worked similarly, so wanted to be explicit about what this is doing. I've renamed it to 'load_tile_slice' as suggested, I think that reads better that 'tile_slice_load'.

> Also I wonder if it makes sense for this to have a horizontal/vertical load variant/argument, so that it would be possible to do in-flight transpose as well?

I think at some point it probably will but I've given little consideration to that so far and have gone with the assumption that everything is horizontal. I personally dont want to add it prematurely without having an understanding of how it fits into the big picture. Have you given much thought to how it would be used? 


================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:317-318
+  let description = [{
+    Loads a 1D tile slice from memory into a 2D SME "virtual tile". The tile
+    slice is defined by the 2D scalable vector type of the input tile. A tile
+    slice index describes where in the input tile the tile slice is loaded to.
----------------
dcaballe wrote:
> I don't follow here. The tile slice (1D) is defined by the 2D type? Do you mean by the dimension of the 2D type pointed by the index?
> I don't follow here. The tile slice (1D) is defined by the 2D type? Do you mean by the dimension of the 2D type pointed by the index?

Apologies it's quite difficult writing generic descriptions for these ops. I think what I wanted to capture was e.g. for a tile of type `vector<[4]x[4]xi32>` the tile slice is `vector<[4]xi32>`, but I suppose the examples provide clarity there. I would like at some point to update the types in the asm to something like `vector<[4]xi32> from memref<?x?xi8> into vector<[4]x[4]xi32>`. Anyhow, I've updated the description, hopefully makes more sense.


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

https://reviews.llvm.org/D156467