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

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: mlir/include/mlir/Dialect/ArmSME/IR/ArmSME.td:46-47
+      And<[IsVectorOfRankPred<[2]>,
+           CPred<[{::llvm::cast<::mlir::VectorType>($_self).allDimsScalable()}]>,
+           CPred<"::llvm::cast<::mlir::VectorType>($_self).getShape() == ArrayRef<int64_t>({" # !interleave(dims, ", ") # "})">]>,
+  description>;
----------------
c-rhodes wrote:
> awarzynski wrote:
> > c-rhodes wrote:
> > > awarzynski wrote:
> > > > [nit] Perhaps extract the preconditions to dedicated definition?
> > > > 
> > > > Also, when would this be triggered:
> > > > ```
> > > >  CPred<"::llvm::cast<::mlir::VectorType>($_self).getShape() == ArrayRef<int64_t>({" # !interleave(dims, ", ") # "})"
> > > > ```
> > > > It feels like a "VectorType verifier" that could be safely skipped (i.e. nothing SME specific).
> > > > Also, when would this be triggered:
> > > > ```
> > > >  CPred<"::llvm::cast<::mlir::VectorType>($_self).getShape() == ArrayRef<int64_t>({" # !interleave(dims, ", ") # "})"
> > > > ```
> > > > It feels like a "VectorType verifier" that could be safely skipped (i.e. nothing SME specific).
> > > 
> > > Please could you clarify, not sure what you mean? This verifies the shape, i.e. `vector<[16]x[16]xi8>` is (16, 16).
> > I am just thinking that every vector that you create like this will satisfy this condition and to me this check feels redundant. But I am probably just failing to understand the underlying rationale. No harm in keeping this.
> > I am just thinking that every vector that you create like this will satisfy this condition and to me this check feels redundant.
> 
> And this verifies that :)
> 
> 
> > I am just thinking that every vector that you create like this will satisfy this condition and to me this check feels redundant.
> 
> And this verifies that :)
> 
> 

To clarify, without this check:
```
%tile = arm_sme.cast_tile_to_vector %tile_id : i32 to vector<[4]x[8]xi32>
```

would be valid


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

https://reviews.llvm.org/D154941