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

[Artemiy Bulavin]

================
@@ -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.")
+          )}]>,
----------------
abulavin wrote:

The [previous PR](https://github.com/llvm/llvm-project/pull/123491) added these options to the pass initially so I'm assuming that this implies the need for this kind of control?

I'm also unfamiliar with using the transform dialect for testing -- my understanding was that pass pipelines were generally tested using invocations of `mlir-opt`. Indeed, there are a large number of test passes registered [here](https://github.com/llvm/llvm-project/blob/main/mlir/tools/mlir-opt/mlir-opt.cpp#L34) that would suggest so, unless I've misunderstood. If yourself and the other reviewers would like me to write some tests as using the transform interpreter than I am happy to do so.

That being said, when I was writing this PR I attempted to write a test for these options. However, as someone who has limited knowledge of the `vector` to `llvm` lowering,  it's not obvious to me how to correctly isolate and test the effect of a single pattern option on the entirety of the `ConvertVectorToLLVMPass`. For example, it's hard for me to say (and seems like a brittle test) what the resulting LLVM IR looks like when `vector.contract` is lowered with the `dot` option versus the `matmul` option, when this is only one of many patterns in the pass. As a result, my hope is that the correctness of these specific patterns in this PR is covered by existing tests in the vector dialect.

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