[Mlir-commits] [mlir] [mlir][Vector] Move `vector.mask` canonicalization to folder (PR #140324)

[Andrzej Warzyński]

================
@@ -1,10 +1,12 @@
 // RUN: mlir-opt %s --gpu-to-llvm | FileCheck %s
 
 module {
+  // CHECK-LABEL: func @func
+  // CHECK-SAME: %[[IN:.*]]: vector<11xf32>
   func.func @func(%arg: vector<11xf32>) -> vector<11xf32> {
     %cst_41 = arith.constant dense<true> : vector<11xi1>
-    // CHECK: vector.mask
-    // CHECK-SAME: vector.yield %arg0
+    // CHECK-NOT: vector.mask
+    // CHECK: return %[[IN]] : vector<11xf32>
----------------
banach-space wrote:

This is a _regression_ test originally added in https://github.com/llvm/llvm-project/commit/690499882973f5cac997e1074162b3fb31c53ef2 . It’s a great example of why "regression" on its own isn’t a helpful test description 😅

That commit fixed a bug in `ConvertVectorToLLVM.cpp` -  it’s unclear why the test landed under the "GPUCommon" subdirectory. Unfortunately, with no comments and uninformative function/variable names, it’s hard to tell what specific edge case this was meant to cover - which makes changes like this unnecessarily tricky to review.

>From the context of the original commit, it seems the intent was to test the lowering of empty vector.mask. If so, I think this would be better placed in: [mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir](https://github.com/llvm/llvm-project/blob/main/mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir)

WDYT? Would you mind moving it there instead?

Also - if I’ve understood the original intent correctly, your update here makes sense 🙂

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