[PATCH] D154941: [mlir][ArmSME] Add custom get_tile_id and cast ops

[Cullen Rhodes via Phabricator via llvm-commits]

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


================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:44
+class SMETileType<Type datatype, list<int> dims, string description>
+  : ShapedContainerType<[datatype],
+      And<[IsVectorOfRankPred<[2]>, allDimsScalableVectorTypePred,
----------------
dcaballe wrote:
> Is there a construct to make this a VectorType instead of a ShapedType? I guess the subsequent predicates constraint the shaped type a bit more but it would be great if this could be an vector type directly.
> Is there a construct to make this a VectorType instead of a ShapedType? I guess the subsequent predicates constraint the shaped type a bit more but it would be great if this could be an vector type directly.

I don't believe there is an existing one, the vector ones in `mlir/include/mlir/IR/OpBase.td` I used for reference here also use this, but perhaps `ShapedContainerType` could be copied for a `VectorType`.


================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:131
+def CastVectorToTile : ArmSME_Op<"cast_vector_to_tile", [Pure, TileElementWidthMatchesTileID]> {
+  let summary = "Cast from 2-d scalable vector type to tile id";
+  let description = [{
----------------
dcaballe wrote:
> I guess you also considered introducing a single cast op that could cast both ways depending on the order of the operand/types. I think having two makes sense since this cast is kind of crossing two domains...
> I guess you also considered introducing a single cast op that could cast both ways depending on the order of the operand/types. I think having two makes sense since this cast is kind of crossing two domains...

That didn't cross my mind actually, it's a good point I think of these casts as being similar to `builtin.unrealized_conversion_cast` and that does similar but with a single cast op like you say, perhaps this could be a single cast as well.


================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:166
+def GetTileID : ArmSME_Op<"get_tile_id", [Pure]> {
+  let summary = "Returns an SME \"virtual tile\" id";
+  let description = [{
----------------
dcaballe wrote:
> nit: do the quotes imply anything on `virtual tiles`? I think I don't get what it is :)
> nit: do the quotes imply anything on `virtual tiles`? I think I don't get what it is :)

To be honest I would prefer we just use `tile`, but the rationale is that these are not real tiles but merely “views” into ZA.


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

https://reviews.llvm.org/D154941