[Mlir-commits] [mlir] [mlir] Refactor ConvertVectorToLLVMPass options (PR #128219)

[Andrzej Warzyński]

================
@@ -1410,10 +1410,32 @@ def ConvertVectorToLLVMPass : Pass<"convert-vector-to-llvm"> {
            "bool", /*default=*/"false",
            "Enables the use of X86Vector dialect while lowering the vector "
 	   "dialect.">,
-    Option<"vectorTransformsOptions", "vector-transform-options",
-           "vector::VectorTransformsOptions",
-           /*default=*/"vector::VectorTransformsOptions()",
-           "Options to lower some operations like contractions and transposes.">,
+    Option<"vectorContractLowering", "vector-contract-lowering",
+           "vector::VectorContractLowering",
+           /*default=*/"vector::VectorContractLowering::Dot",
+           VectorContractLoweringAttr.summary, [{::llvm::cl::values(
+           clEnumValN(::mlir::vector::VectorContractLowering::Dot, "dot", 
+            "Progressively lower to finer grained `vector.contract` and dot-products. (default)"),
+           clEnumValN(::mlir::vector::VectorContractLowering::Matmul, "matmul",
+            "Lower to `vector.matrix_multiply`, maps 1-1 to LLVM matrix intrinsics."),
+           clEnumValN(::mlir::vector::VectorContractLowering::OuterProduct, "outerproduct",
+            "Lower to `vector.outerproduct`."),
+           clEnumValN(::mlir::vector::VectorContractLowering::ParallelArith, "parallelarith",
+            "Lower contract with all reduction dimensions unrolled to 1 to a vector elementwise operations.")
+	        )}]>,
+    Option<"vectorTransposeLowering", "vector-transpose-lowering",
+           "vector::VectorTransposeLowering",
+           /*default=*/"vector::VectorTransposeLowering::EltWise",
+           VectorTransposeLoweringAttr.summary, [{::llvm::cl::values(
+           clEnumValN(::mlir::vector::VectorTransposeLowering::EltWise, "eltwise",
+            "Lower transpose into element-wise extract and inserts (default)"),
+           clEnumValN(::mlir::vector::VectorTransposeLowering::Flat, "flat",
+            "Lower 2-D transpose to `vector.flat_transpose`, maps 1-1 to LLVM matrix intrinsics"),
+           clEnumValN(::mlir::vector::VectorTransposeLowering::Shuffle1D, "shuffle1d",
+            "Lower 2-D transpose to `vector.shuffle` on 1-D vector."),
+           clEnumValN(::mlir::vector::VectorTransposeLowering::Shuffle16x16, "shuffle16x16",
+            "Lower 2-D transpose to `vector.shuffle` on 16x16 vector.")
+          )}]>,
----------------
banach-space wrote:

IMHO, this is becoming too verbose and too fine-grained - do we need this?

In general, there are three different ways to drive this pass (and the relevant transformations):

* C++ API
* [Transform Dialect](https://mlir.llvm.org/docs/Dialects/Transform/) (this is what we tend to use in tests)
* LLVM command-line (llvm::cl) flags (added in this PR)

>From my perspective, `llvm::cl` flags are mostly useful for testing and prototyping. However, in practice, we seem to favor Transform Dialect for _finer-grained control_. That said, this may be a matter of personal preference.

My bigger concern is that we are not testing these flags. If they duplicate existing Transform Dialect functionality, we should ensure that relevant RUN lines are duplicated, rather than adding tests only to check the flags themselves.

This is a broader issue in MLIR and somewhat tangential to this PR (which is attempting to fix the issue reported here: [MLIR: Specifying Structs as cl::Options for PassOptions](https://discourse.llvm.org/t/mlir-specifying-structs-as-cl-options-for-passoptions)).

Let’s wait for others to chime in. More generally, I hope we can establish clearer guidelines on the relationship between different ways of "driving" transformations and preferred testing approaches.

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