[Mlir-commits] [mlir] [mlir][vector] Improve flattening vector.transfer_write ops. (PR #94051)

[Han-Chung Wang]

================
@@ -471,25 +471,27 @@ func.func @regression_non_contiguous_dim_read(%subview : memref<1x3x3x2xf32, str
 }
 
 //       CHECK:  #[[$MAP:.+]] = affine_map<()[s0] -> (s0 * 2)>
-// CHECK-LABEL:    func.func @regression_non_contiguous_dim_read(
-//       CHECK:      %[[COLLAPSE:.+]] = memref.collapse_shape %{{.*}} {{\[}}[0], [1], [2, 3]] : memref<1x3x3x2xf32, strided<[40, 10, 2, 1], offset: ?>> into memref<1x3x6xf32, strided<[40, 10, 1], offset: ?>>
-//       CHECK:     %[[APPLY:.*]] = affine.apply #[[$MAP]]()
+// CHECK-LABEL:  func.func @regression_non_contiguous_dim_read(
+//       CHECK:    %[[COLLAPSE:.+]] = memref.collapse_shape %{{.*}} {{\[}}[0], [1], [2, 3]] : memref<1x3x3x2xf32, strided<[40, 10, 2, 1], offset: ?>> into memref<1x3x6xf32, strided<[40, 10, 1], offset: ?>>
+//       CHECK:    %[[APPLY:.*]] = affine.apply #[[$MAP]]()
 
 // CHECK-128B-LABEL: func @regression_non_contiguous_dim_read(
 //       CHECK-128B:   memref.collapse_shape
 
 // -----
 
-func.func @unsupported_non_contiguous_dim_write(%value : vector<2x2xf32>,
+func.func @regression_non_contiguous_dim_write(%value : vector<2x2xf32>,
----------------
hanhanW wrote:

>> Fundamentally, we can not collapse three inner dimensions because the strides is [x, 10, 2, 1] but not [x, 6, 2, 1]

> Just to double check - this is referring to regression_non_contiguous_dim_write? In which collapsing does happen?

Yes, it is referring to the test. However, I was wrong on the statement because it is 2D vector. I will add a 3D vector case, and check that it is not collapsed.

> "Regression" doesn't really help differentiate this test from other tests. It's a missed opportunity to document what makes this test case unique - i.e. non-zero indices :) (*) So, perhaps:
>     @transfer_write_non_contigous_dims_mismatch_non_zero_idx
> Next, you could move it near @transfer_write_dims_mismatch_non_contiguous - both tests seem to check sth very similar, two edge cases for vector.transfer_write. I find it very helpful when such edge cases are clustered together.

I also found "regression" is weird, but I'd like to follow the naming style like the above xfer_read test. The new name is great, and I'll send a follow-up about naming and structure of the file.

> I've been actually discussing this with someone recently :) We don't need it for this PR, but it would be good to add a TODO somewhere (I can look into this).

Wait, I thought that I added the tests! Then I found that it is on my local file.. my bad. adding now

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