[Mlir-commits] [mlir] [mlir][ArmSVE] Add convert_to/from_svbool ops (PR #68586)

[Andrzej Warzyński]

================
@@ -37,6 +37,12 @@ def IsFixedVectorTypePred : CPred<[{::llvm::isa<::mlir::VectorType>($_self) &&
 def IsScalableVectorTypePred : CPred<[{::llvm::isa<::mlir::VectorType>($_self) &&
                                    ::llvm::cast<VectorType>($_self).isScalable()}]>;
 
+// Whether a type is a scalable VectorType, with a single trailing scalable dimension.
+def IsTrailingScalableVectorTypePred : And<[CPred<"::llvm::isa<::mlir::VectorType>($_self)">,
----------------
banach-space wrote:

Shouldn't this inherit from `IsScalableVectorTypePred`? Just looking at the comments:

> // Whether a type is a scalable VectorType.

vs

>  // Whether a type is a scalable VectorType, with a single trailing scalable dimension.

So it sounds like `IsTrailingScalableVectorTypePred` is a refined version of `IsScalableVectorTypePred`? In fact, given how complex this is - wouldn't it make sense to simply have a dedicated predicate to check that only the trailing dim is scalable? I am also thinking about the name of this predicate and IMHO it doesn't reveal what the predicate represents. I would try something like: 

* `IsOnlyTrailingDImScalablePred`

That would be shorter, but would capture the key aspect of this pred. Also, we know that only vectors can be "scalable", so can skip "scalable" from name. I would, perhaps, also add an example of what would be valid and what would not under this predicate.

https://github.com/llvm/llvm-project/pull/68586