[Mlir-commits] [mlir] [mlir][linalg] Consolidate tests for scalable vectorization (PR #141469)

[Andrzej Warzyński]

================
@@ -36,6 +40,42 @@ module attributes {transform.with_named_sequence} {
 
 // -----
 
+func.func @vectorize_dynamic_identity_scalable(%arg0: tensor<?xf32>,
+                                               %arg1: tensor<?xf32>,
+                                               %arg2: tensor<?xf32>) -> tensor<?xf32> {
+  %0 = linalg.generic { indexing_maps = [affine_map<(d0) -> (d0)>,
+                                         affine_map<(d0) -> (d0)>,
+                                         affine_map<(d0) -> (d0)>],
+                   iterator_types = ["parallel"] }
+    ins(%arg0, %arg1 : tensor<?xf32>, tensor<?xf32>)
+    outs(%arg2 : tensor<?xf32>) {
+    ^bb(%in0: f32, %in1: f32, %out: f32) :
+      %0 = arith.addf %in0, %in1 : f32
+      linalg.yield %0 : f32
+    } -> tensor<?xf32>
+  return %0 : tensor<?xf32>
+}
+
+// CHECK-LABEL:   @vectorize_dynamic_identity_scalable
----------------
banach-space wrote:

Thanks for sharing your perspective! I always appreciate when people explain what works for them - it's genuinely helpful. In this case, I’m just following the existing convention in the file, so I’d prefer not to diverge 😅

Regarding empty lines more generally, I find them useful for separating high-level logic. My reading of the Google C++ Style Guide supports that too:
> Use whitespace purposefully to provide separation in that flow.

That said, what's considered “good density” of empty lines can be pretty subjective - definitely feels like personal preference territory. My main point is just that there are reasonable arguments on both sides.

> Note: it already happens on my laptop monitor. E.g., the last line is cut in my laptop monitor in the below vectorize_dynamic_reduction_2d_scalable test. 

That’s fair. But I also think there are broader factors affecting readability on smaller screens:
* The `CHECK` lines are super wide - manageable on big screens but noisy on laptops, especially with auto-wrap. Perhaps wide-lines should be discouraged?
* There’s a lot of repetition in the output (e.g., 3x `arith.constant 0 : index` and 3x `arith.constant 0.000000e+00 : f32 lines`). That’s something we’re trying to improve via constant caching: https://github.com/llvm/llvm-project/issues/138265 (see my bottom comments there).

> I'll be able to see the full checks if the blank line is removed.

True! Removing the blank line could help fit in one more line of code, which is definitely a positive. Maybe something like this could be added to the [MLIR Testing Guide](https://mlir.llvm.org/getting_started/TestingGuide/#test-formatting-best-practices) as a tip? (*)

(*) That page doesn’t render well - I’ve been meaning to tweak the layout when I get a moment.

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