[mlir][vector] Update tests for collapse 1/n (nfc) #94490

banach-space · 2024-06-05T15:48:16Z

The main goal of this PR (and subsequent PRs), is to add more tests with
scalable vectors to:

vector-transfer-collapse-inner-most-dims.mlir

There's quite a few cases to consider, hence this is split into multiple
PRs. In this PR, the very first test is complemented with all the possible
combinations:

scalable (rather than fixed) unit trailing dim,
dynamic (rather than static) trailing dim in the source memref.

Also,

@leading_scalable_dimension_transfer_read and
@trailing_scalable_one_dim_transfer_read,

are replaced with:

@contiguous_inner_most_scalable_inner_dim and
@negative_scalable_unit_dim,

respectively, and added to the list above (i.e. alongside other
variations for the very first test).

In addition:

"_view" is removed from function names (it's not clear to me what it
was meant to signify)
extra comments are added to separate tests for vector.transfer_read
and vector.transfer_write

NOTE: This PR is limited to tests for vector.transfer_read.

llvmbot · 2024-06-05T15:48:45Z

@llvm/pr-subscribers-mlir-vector

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

The main goal of this PR (and subsequent PRs), is to add more tests with
scalable vectors to:

vector-transfer-collapse-inner-most-dims.mlir

There's quite a few cases to consider, hence this is split into multiple
PRs. In this PR, the very first test is complemented with all the possible
combinations:

scalable (rather than fixed) unit trailing dim,
dynamic (rather than static) trailing dim in the source
memref.
Also,
@leading_scalable_dimension_transfer_read,
is replaced with
@contiguous_inner_most_scalable_inner_dim,
and added to the list above (i.e. alongside other variations for the
very first test).

In addition:

"_view" is removed from function names (it's not clear to me what it
was meant to signify)
extra comments are added to separate tests for
vector.transfer_read and vector.transfer_write

Full diff: https://github.com/llvm/llvm-project/pull/94490.diff

1 Files Affected:

(modified) mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir (+59-19)

diff --git a/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir b/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir
index b4cb640108bae..42e255c2152d7 100644
--- a/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir
+++ b/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir
@@ -1,12 +1,17 @@
 // RUN: mlir-opt %s -test-vector-transfer-collapse-inner-most-dims -split-input-file | FileCheck %s
 
-func.func @contiguous_inner_most_view(%in: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x8x1xf32>{
+//-----------------------------------------------------------------------------
+// 1. vector.transfer_read
+//-----------------------------------------------------------------------------
+
+func.func @contiguous_inner_most(%in: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x8x1xf32>{
   %c0 = arith.constant 0 : index
   %cst = arith.constant 0.0 : f32
   %0 = vector.transfer_read %in[%c0, %c0, %c0, %c0], %cst {in_bounds = [true, true, true]} : memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>, vector<1x8x1xf32>
   return %0 : vector<1x8x1xf32>
 }
-//      CHECK: func @contiguous_inner_most_view(%[[SRC:.+]]: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>
+
+//      CHECK: func @contiguous_inner_most(%[[SRC:.+]]: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>
 //      CHECK:   %[[SRC_0:.+]] = memref.subview %[[SRC]]
 // CHECK-SAME:    memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>> to memref<1x1x8xf32, strided<[3072, 8, 1], offset: ?>>
 //      CHECK:   %[[VEC:.+]] = vector.transfer_read %[[SRC_0]]
@@ -14,15 +19,61 @@ func.func @contiguous_inner_most_view(%in: memref<1x1x8x1xf32, strided<[3072, 8,
 //      CHECK:   %[[RESULT:.+]] = vector.shape_cast %[[VEC]]
 //      CHECK:   return %[[RESULT]]
 
+// Same as the top example within this split, but with the inner vector
+// dim scalable. Note that this example only makes sense when "8 = [8]" (i.e.
+// vscale = 1). This is assumed (impliciely) via the `in_bounds` attribute.
+
+func.func @contiguous_inner_most_scalable_inner_dim(%in: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x[8]x1xf32>{
+  %c0 = arith.constant 0 : index
+  %cst = arith.constant 0.0 : f32
+  %0 = vector.transfer_read %in[%c0, %c0, %c0, %c0], %cst {in_bounds = [true, true, true]} : memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>, vector<1x[8]x1xf32>
+  return %0 : vector<1x[8]x1xf32>
+}
+
+//      CHECK: func @contiguous_inner_most_scalable_inner_dim(%[[SRC:.+]]: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>
+//      CHECK:   %[[SRC_0:.+]] = memref.subview %[[SRC]]
+// CHECK-SAME:    memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>> to memref<1x1x8xf32, strided<[3072, 8, 1], offset: ?>>
+//      CHECK:   %[[VEC:.+]] = vector.transfer_read %[[SRC_0]]
+// CHECK-SAME:    memref<1x1x8xf32, strided<[3072, 8, 1], offset: ?>>, vector<1x[8]xf32>
+//      CHECK:   %[[RESULT:.+]] = vector.shape_cast %[[VEC]]
+//      CHECK:   return %[[RESULT]]
+
+// Same as the top example within this split, but the trailing unit dim was
+// replaced with a dyn dim - not supported
+
+func.func @non_unit_trailing_dim(%in: memref<1x1x8x?xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x8x1xf32>{
+  %c0 = arith.constant 0 : index
+  %cst = arith.constant 0.0 : f32
+  %0 = vector.transfer_read %in[%c0, %c0, %c0, %c0], %cst {in_bounds = [true, true, true]} : memref<1x1x8x?xf32, strided<[3072, 8, 1, 1], offset: ?>>, vector<1x8x1xf32>
+  return %0 : vector<1x8x1xf32>
+}
+
+//  CHECK-LABEL: func @non_unit_trailing_dim
+//    CHECK-NOT: memref.subview
+//    CHECK-NOT: vector.shape_cast
+
+// Same as the top example within this split, but with a scalable unit dim in
+// the output vector - not supported
+
+func.func @scalable_unit_dim(%in: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x8x[1]xf32>{
+  %c0 = arith.constant 0 : index
+  %cst = arith.constant 0.0 : f32
+  %0 = vector.transfer_read %in[%c0, %c0, %c0, %c0], %cst {in_bounds = [true, true, true]} : memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>, vector<1x8x[1]xf32>
+  return %0 : vector<1x8x[1]xf32>
+}
+//  CHECK-LABEL: func @scalable_unit_dim
+//    CHECK-NOT: memref.subview
+//    CHECK-NOT: vector.shape_cast
+
 // -----
 
-func.func @contiguous_outer_dyn_inner_most_view(%a: index, %b: index, %memref: memref<?x?x8x1xf32>) -> vector<8x1xf32> {
+func.func @contiguous_outer_dyn_inner_most(%a: index, %b: index, %memref: memref<?x?x8x1xf32>) -> vector<8x1xf32> {
   %c0 = arith.constant 0 : index
   %pad = arith.constant 0.0 : f32
   %v = vector.transfer_read %memref[%a, %b, %c0, %c0], %pad {in_bounds = [true, true]} : memref<?x?x8x1xf32>, vector<8x1xf32>
   return %v : vector<8x1xf32>
 }
-// CHECK: func.func @contiguous_outer_dyn_inner_most_view(
+// CHECK: func.func @contiguous_outer_dyn_inner_most(
 // CHECK-SAME:   %[[IDX0:[a-zA-Z0-9]+]]
 // CHECK-SAME:   %[[IDX1:[a-zA-Z0-9]+]]
 // CHECK-SAME:   %[[SRC:[a-zA-Z0-9]+]]
@@ -103,6 +154,10 @@ func.func @contiguous_inner_most_dim_out_of_bounds_2d(%arg0: memref<1x1xf32>) ->
 
 // -----
 
+//-----------------------------------------------------------------------------
+// 2. vector.transfer_write
+//-----------------------------------------------------------------------------
+
 func.func @drop_two_inner_most_dim_for_transfer_write(%arg0: memref<1x512x16x1x1xf32>, %arg1: vector<1x16x16x1x1xf32>, %arg2: index) {
   %c0 = arith.constant 0 : index
   vector.transfer_write %arg1, %arg0[%c0, %arg2, %c0, %c0, %c0]
@@ -177,21 +232,6 @@ func.func @non_unit_strides(%arg0: memref<512x16x1xf32, strided<[8192, 16, 4], o
 
 // -----
 
-func.func @leading_scalable_dimension_transfer_read(%dest : memref<24x1xf32>) -> vector<[4]x1xf32> {
-  %c0 = arith.constant 0 : index
-  %pad = arith.constant 0.0 : f32
-  %0 = vector.transfer_read %dest[%c0, %c0], %pad {in_bounds = [true, true]} : memref<24x1xf32>, vector<[4]x1xf32>
-  return %0 : vector<[4]x1xf32>
-}
-// CHECK:      func.func @leading_scalable_dimension_transfer_read
-// CHECK-SAME:   %[[DEST:[a-zA-Z0-9]+]]
-// CHECK:        %[[SUBVIEW:.+]] = memref.subview %[[DEST]][0, 0] [24, 1] [1, 1] : memref<24x1xf32> to memref<24xf32, strided<[1]>>
-// CHECK:        %[[READ:.+]] = vector.transfer_read %[[SUBVIEW]]{{.*}} {in_bounds = [true]} : memref<24xf32, strided<[1]>>, vector<[4]xf32>
-// CHECK:        %[[CAST:.+]] = vector.shape_cast %[[READ]] : vector<[4]xf32> to vector<[4]x1xf32>
-// CHECK:        return %[[CAST]]
-
-// -----
-
 // Negative test: [1] (scalable 1) is _not_ a unit dimension.
 func.func @trailing_scalable_one_dim_transfer_read(%dest : memref<24x1xf32>) -> vector<4x[1]xf32> {
   %c0 = arith.constant 0 : index

banach-space · 2024-06-05T15:52:25Z

@MacDue This updates the test that you added in #92402. I don't see this change affecting test coverage, but please let me know if you feel differently 😅

I mostly wanted to make sure that it's clear what edge cases are tested - to me it really helps if "similar" examples is re-used (hence why I moved your test).

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR (only `vector.transfer_read` tests are updated): 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. There are other tests (higher up) that also verify that the `in_bounds` attribute is correctly preserved. What's unique about this test is that it contains `memref.subview`. 3. Update test names to make them more consistent with the other tests and highlights what's unique in every test. 2. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. Follow-up for: llvm#94490

MacDue · 2024-06-06T13:46:58Z

CI seems to be failing:

FAIL: MLIR :: Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir (1068 of 2630)
******************** TEST 'MLIR :: Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir' FAILED ********************
Exit Code: 1
Command Output (stdout):
--
# RUN: at line 1
/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/build/bin/mlir-opt /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir -test-vector-transfer-collapse-inner-most-dims -split-input-file | /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/build/bin/FileCheck /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir
# executed command: /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/build/bin/mlir-opt /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir -test-vector-transfer-collapse-inner-most-dims -split-input-file
# note: command had no output on stdout or stderr
# executed command: /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/build/bin/FileCheck /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir
# .---command stderr------------
# | /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-rshzq-1/llvm-project/github-pull-requests/mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir:64:17: error: CHECK-LABEL: expected string not found in input
# | // CHECK-LABEL: func @scalable_unit_dim
# |                 ^
# | <stdin>:18:34: note: scanning from here
# |  func.func @non_unit_trailing_dim(%arg0: memref<1x1x8x?xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x8x1xf32> {
# |                                  ^
# | <stdin>:24:16: note: possible intended match here
# |  func.func @negative_scalable_unit_dim(%arg0: memref<1x1x8x1xf32, strided<[3072, 8, 1, 1], offset: ?>>) -> vector<1x8x[1]xf32> {

MacDue · 2024-06-06T13:49:53Z

mlir/test/Dialect/Vector/vector-transfer-collapse-inner-most-dims.mlir

-// Negative test: [1] (scalable 1) is _not_ a unit dimension.
-func.func @trailing_scalable_one_dim_transfer_write(%dest : memref<24x1xf32>, %vec: vector<4x[1]xf32>, %index: index) {
-  %c0 = arith.constant 0 : index
-  vector.transfer_write %vec, %dest[%index, %c0] {in_bounds = [true, true]} : vector<4x[1]xf32>,  memref<24x1xf32>
-  return
-}


There seems to be no replacement for this transfer_write negative test?

Also,

@leading_scalable_dimension_transfer_read,
@trailing_scalable_one_dim_transfer_write,
are replaced with:

@contiguous_inner_most_scalable_inner_dim,
@negative_scalable_unit_dim,

Check @negative_scalable_unit_dim :) The idea was to make it a variation of @negative_scalable_unit_dim (so that it's easy to spot the difference).

But @negative_scalable_unit_dim is checking transfer_read, while this test checks transfer_write (a different pattern).

Argh, sorry, I meant @trailing_scalable_one_dim_transfer_read rather than @trailing_scalable_one_dim_transfer_write. This test should stay. Thank you Ben! (I've updated the summary)

EDIT Should be fixed. Please note that I've updated the latest commit rather than creating a new one (Git failure on my part, sorry)

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR (only `vector.transfer_read` tests are updated): 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. There are other tests (higher up) that also verify that the `in_bounds` attribute is correctly preserved. What's unique about this test is that it contains `memref.subview`. 3. Update test names to make them more consistent with the other tests and highlights what's unique in every test. 2. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. Follow-up for: llvm#94490

banach-space · 2024-06-06T17:08:46Z

Thanks for taking a loon!

CI seems to be failing:

Sorry about that, should be fixed.

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, the very first test is complemented with all the possible combinations: * scalable (rather than fixed) unit trailing dim, * dynamic (rather than static) trailing dim in the source memref. Also, * @leading_scalable_dimension_transfer_read and @trailing_scalable_one_dim_transfer_read, are replaced with: * @contiguous_inner_most_scalable_inner_dim and @negative_scalable_unit_dim, respectively, and added to the list above (i.e. alongside other variations for the very first test). In addition: * "_view" is removed from function names (it's not clear to me what it was meant to signify) * extra comments are added to separate tests for vector.transfer_read and vector.transfer_write NOTE: This PR is limited to tests for `vector.transfer_read`.

Fix failing test

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR: 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. This test was introduced to make sure that the `in_bounds` attribute is correctly preserved, but that's already verified by some earlier tests. The updated name highlights the differentiating factor of this test when compared to the other tests currently present in the file (i.e. the presence of `memref.subview` in the input IR). 3. Updated test names to make them more consistent with the other tests and to highlight the unique features that are being tested (e.g. `@contiguous_inner_most_dim` -> `@contiguous_inner_most_dim_non_zero_idxs` for a test 2. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. NOTE: This PR is limited to tests for `vector.transfer_read`. Follow-up for: llvm#94490

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR: 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. This test was introduced to make sure that the `in_bounds` attribute is correctly preserved, but that's already verified by some earlier tests. The updated name highlights the differentiating factor of this test when compared to the other tests _currently_ present in the file, i.e. the presence of `memref.subview` in the input IR. 2. Renamed `@contiguous_inner_most_dim_out_of_bounds_2d` as `@negative_non_unit_inner_vec_dim`. While this test does contain an out-of-bounds access, the actual reason for the tested pattern to fail is the fact that the inner dim in the output vector is not "1". A complimentary test was added to verify that the pattern also fails when the source memref has non-unit trailing dim (`@negative_non_unit_inner_memref_dim`). 3. Renamed `@contiguous_inner_most_dim` as `@contiguous_inner_most_dim_non_zero_idxs` - this test verifies that the pattern works in the presence of non-zero idxs. 4. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. NOTE: This PR is limited to tests for `vector.transfer_read`. Follow-up for: llvm#94490

Restrict `DropInnerMostUnitDimsTransferRead` so that it fails when one of the indices to be dropped could be != 0, e.g. ``` func.func @negative_example(%A: memref<16x1xf32>, %i:index, %j:index) -> (vector<8x1xf32>) { %f0 = arith.constant 0.0 : f32 %1 = vector.transfer_read %A[%i, %j], %f0 : memref<16x1xf32>, vector<8x1xf32> return %1 : vector<8x1xf32> } ``` This is an edge case that could represent an out-of-bounds access, though that will depend on the actual value of `%j`. NOTE: This PR is limited to tests for `vector.transfer_read`. Depends on: llvm#94490, llvm#94604

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR: 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. This test was introduced to make sure that the `in_bounds` attribute is correctly preserved, but that's already verified by some earlier tests. The updated name highlights the differentiating factor of this test when compared to the other tests _currently_ present in the file, i.e. the presence of `memref.subview` in the input IR. 2. Renamed `@contiguous_inner_most_dim_out_of_bounds_2d` as `@negative_non_unit_inner_vec_dim`. While this test does contain an out-of-bounds access, the actual reason for the tested pattern to fail is the fact that the inner dim in the output vector is not "1". A complimentary test was added to verify that the pattern also fails when the source memref has non-unit trailing dim (`@negative_non_unit_inner_memref_dim`). 3. Renamed `@contiguous_inner_most_dim` as `@contiguous_inner_most_dim_non_zero_idxs` - this test verifies that the pattern works in the presence of non-zero idxs. 4. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. NOTE: This PR is limited to tests for `vector.transfer_read`. Follow-up for: llvm#94490

Restrict `DropInnerMostUnitDimsTransferRead` so that it fails when one of the indices to be dropped could be != 0, e.g. ``` func.func @negative_example(%A: memref<16x1xf32>, %i:index, %j:index) -> (vector<8x1xf32>) { %f0 = arith.constant 0.0 : f32 %1 = vector.transfer_read %A[%i, %j], %f0 : memref<16x1xf32>, vector<8x1xf32> return %1 : vector<8x1xf32> } ``` This is an edge case that could represent an out-of-bounds access, though that will depend on the actual value of `%j`. NOTE: This PR is limited to tests for `vector.transfer_read`. Depends on: llvm#94490, llvm#94604

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, the very first test for `vector.transfer_write` is complemented with all the possible combinations: * scalable (rather than fixed) unit trailing dim, * dynamic (rather than static) trailing dim in the source memref. To this end, the following tests: * `@leading_scalable_dimension_transfer_write` `@trailing_scalable_one_dim_transfer_write` are replaced with: * `@drop_two_inner_most_dim_scalable_inner_dim` and `@negative_scalable_unit_dim`, respectively. In addition: * "_for_transfer_write" is removed from function names (to reduce noise). This is a follow-up for: llvm#94490, llvm#94604 NOTE: This PR is limited to tests for `vector.transfer_write`.

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR: 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. This test was introduced to make sure that the `in_bounds` attribute is correctly preserved, but that's already verified by some earlier tests. The updated name highlights the differentiating factor of this test when compared to the other tests _currently_ present in the file, i.e. the presence of `memref.subview` in the input IR. 2. Renamed `@contiguous_inner_most_dim_out_of_bounds_2d` as `@negative_non_unit_inner_vec_dim`. While this test does contain an out-of-bounds access, the actual reason for the tested pattern to fail is the fact that the inner dim in the output vector is not "1". A complimentary test was added to verify that the pattern also fails when the source memref has non-unit trailing dim (`@negative_non_unit_inner_memref_dim`). 3. Renamed `@contiguous_inner_most_dim` as `@contiguous_inner_most_dim_non_zero_idxs` - this test verifies that the pattern works in the presence of non-zero idxs. 4. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. NOTE: This PR is limited to tests for `vector.transfer_read`. Follow-up for: llvm#94490

Restrict `DropInnerMostUnitDimsTransferRead` so that it fails when one of the indices to be dropped could be != 0, e.g. ``` func.func @negative_example(%A: memref<16x1xf32>, %i:index, %j:index) -> (vector<8x1xf32>) { %f0 = arith.constant 0.0 : f32 %1 = vector.transfer_read %A[%i, %j], %f0 : memref<16x1xf32>, vector<8x1xf32> return %1 : vector<8x1xf32> } ``` This is an edge case that could represent an out-of-bounds access, though that will depend on the actual value of `%j`. NOTE: This PR is limited to tests for `vector.transfer_read`. Depends on: llvm#94490, llvm#94604

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, the very first test for `vector.transfer_write` is complemented with all the possible combinations: * scalable (rather than fixed) unit trailing dim, * dynamic (rather than static) trailing dim in the source memref. To this end, the following tests: * `@leading_scalable_dimension_transfer_write` `@trailing_scalable_one_dim_transfer_write` are replaced with: * `@drop_two_inner_most_dim_scalable_inner_dim` and `@negative_scalable_unit_dim`, respectively. In addition: * "_for_transfer_write" is removed from function names (to reduce noise). This is a follow-up for: llvm#94490, llvm#94604 NOTE: This PR is limited to tests for `vector.transfer_write`.

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir Changes in this PR: 1. Renamed `@contiguous_inner_most_dim_bounds` as `@contiguous_inner_most_dim_with_subview`. This test was introduced to make sure that the `in_bounds` attribute is correctly preserved, but that's already verified by some earlier tests. The updated name highlights the differentiating factor of this test when compared to the other tests _currently_ present in the file, i.e. the presence of `memref.subview` in the input IR. 2. Renamed `@contiguous_inner_most_dim_out_of_bounds_2d` as `@negative_non_unit_inner_vec_dim`. While this test does contain an out-of-bounds access, the actual reason for the tested pattern to fail is the fact that the inner dim in the output vector is not "1". A complimentary test was added to verify that the pattern also fails when the source memref has non-unit trailing dim (`@negative_non_unit_inner_memref_dim`). 3. Renamed `@contiguous_inner_most_dim` as `@contiguous_inner_most_dim_non_zero_idxs` - this test verifies that the pattern works in the presence of non-zero idxs. 4. Added more tests for scalable vectors - this should cover all cases for `vector.transfer_read`. NOTE: This PR is limited to tests for `vector.transfer_read`. Follow-up for: #94490

Restrict `DropInnerMostUnitDimsTransferRead` so that it fails when one of the indices to be dropped could be != 0, e.g. ``` func.func @negative_example(%A: memref<16x1xf32>, %i:index, %j:index) -> (vector<8x1xf32>) { %f0 = arith.constant 0.0 : f32 %1 = vector.transfer_read %A[%i, %j], %f0 : memref<16x1xf32>, vector<8x1xf32> return %1 : vector<8x1xf32> } ``` This is an edge case that could represent an out-of-bounds access, though that will depend on the actual value of `%j`. NOTE: This PR is limited to tests for `vector.transfer_read`. Depends on: llvm#94490, llvm#94604

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, the very first test for `vector.transfer_write` is complemented with all the possible combinations: * scalable (rather than fixed) unit trailing dim, * dynamic (rather than static) trailing dim in the source memref. To this end, the following tests: * `@leading_scalable_dimension_transfer_write` `@trailing_scalable_one_dim_transfer_write` are replaced with: * `@drop_two_inner_most_dim_scalable_inner_dim` and `@negative_scalable_unit_dim`, respectively. In addition: * "_for_transfer_write" is removed from function names (to reduce noise). This is a follow-up for: llvm#94490, llvm#94604 NOTE: This PR is limited to tests for `vector.transfer_write`.

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, the very first test for `vector.transfer_write` is complemented with all the possible combinations: * scalable (rather than fixed) unit trailing dim, * dynamic (rather than static) trailing dim in the source memref. To this end, the following tests: * `@leading_scalable_dimension_transfer_write` `@trailing_scalable_one_dim_transfer_write` are replaced with: * `@drop_two_inner_most_dim_scalable_inner_dim` and `@negative_scalable_unit_dim`, respectively. In addition: * "_for_transfer_write" is removed from function names (to reduce noise). In addition, to maintain consistency between the tests for `xfer_read` and `xfer_write`, 2 negative tests for `xfer_read` are also renamed. This is to follow the suggestion made during the review of this PR. Extra comments in "VectorTransforms.cpp" are added to better document the limitations related to scalable vectors and which tests added here excercise. This is a follow-up for: #94490 and #94604 NOTE: This PR is limited to tests for `vector.transfer_write`.

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, `@outer_dyn_drop_inner_most_dim` is replaced with: * `@contiguous_inner_most_dynamic_outer` I am also adding a similar test for scalable vectors. In addition, * `@drop_two_inner_most_dim` and `@drop_two_inner_most_dim_scalable_inner_dim`, are renamed as `@contiguous_inner_most_scalable_inner_dim` to match their counterpart for xfer_read. NOTE: This PR is limited to tests for `vector.transfer_write` This is a follow-up for: llvm#94490, llvm#94604, llvm#94906

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, I am simply adding more tests for `vector.transfer_write` so that for every test for `xfer_read`, there's a corresponding test for `xfer_write`. This is a follow-up for: llvm#94490, llvm#94604, llvm#94906, llvm#96214

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, `@outer_dyn_drop_inner_most_dim` is replaced with: * `@contiguous_inner_most_dynamic_outer` I am also adding a similar test for scalable vectors. In addition, * `@drop_two_inner_most_dim` and `@drop_two_inner_most_dim_scalable_inner_dim`, are renamed as `@contiguous_inner_most` and `@contiguous_inner_most_scalable_inner_dim`, respectively, to match their counterpart for `xfer_read`. NOTE: This PR is limited to tests for `vector.transfer_write` This is a follow-up for: #94490, #94604, #94906

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, the very first test for `vector.transfer_write` is complemented with all the possible combinations: * scalable (rather than fixed) unit trailing dim, * dynamic (rather than static) trailing dim in the source memref. To this end, the following tests: * `@leading_scalable_dimension_transfer_write` `@trailing_scalable_one_dim_transfer_write` are replaced with: * `@drop_two_inner_most_dim_scalable_inner_dim` and `@negative_scalable_unit_dim`, respectively. In addition: * "_for_transfer_write" is removed from function names (to reduce noise). In addition, to maintain consistency between the tests for `xfer_read` and `xfer_write`, 2 negative tests for `xfer_read` are also renamed. This is to follow the suggestion made during the review of this PR. Extra comments in "VectorTransforms.cpp" are added to better document the limitations related to scalable vectors and which tests added here excercise. This is a follow-up for: llvm#94490 and llvm#94604 NOTE: This PR is limited to tests for `vector.transfer_write`.

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, `@outer_dyn_drop_inner_most_dim` is replaced with: * `@contiguous_inner_most_dynamic_outer` I am also adding a similar test for scalable vectors. In addition, * `@drop_two_inner_most_dim` and `@drop_two_inner_most_dim_scalable_inner_dim`, are renamed as `@contiguous_inner_most` and `@contiguous_inner_most_scalable_inner_dim`, respectively, to match their counterpart for `xfer_read`. NOTE: This PR is limited to tests for `vector.transfer_write` This is a follow-up for: llvm#94490, llvm#94604, llvm#94906

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, I am simply adding more tests for `vector.transfer_write` so that for every test for `xfer_read`, there's a corresponding test for `xfer_write`. This is a follow-up for: llvm#94490, llvm#94604, llvm#94906, llvm#96214

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, I am simply adding more tests for `vector.transfer_write` so that for every test for `xfer_read`, there's a corresponding test for `xfer_write`. This is a follow-up for: #94490, #94604, #94906, #96214

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, I am making the following changes: * All input memrefs for `xfer_read` are are renamed as `%src`. * All input memrefs for `xfer_write` are are renamed as `%dest`. * All variables representing pad values for `xfer_read` are renamed as `%pad`. * All vector variables (for `xfer_read` and `xfer_write`) are renamed as `%v`. * Add `@contiguous_inner_most_non_zero_idx_in_bounds_scalable` for `xfer_read` (similar test already exists for `xfer_write`) * All indiex variables are renamed as `%i` (1st index) and `%ii` (2nd index). The above were marked as TODOs in the test file - these are not resolved. In addition (to avoid sending another PR): * `@drop_inner_most_dim` is deleted - it duplicates `@contiguous_inner_most` for xfer_write * For consistency with other negative tests, renamed `@non_unit_strides` as `@negative_non_unit_strides` and added a similar test for `xfer_read` This is a follow-up for: llvm#94490, llvm#94604, llvm#94906, llvm#96214, llvm#96227

The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, I am making the following changes: * All input memrefs for `xfer_read` are are renamed as `%src`. * All input memrefs for `xfer_write` are are renamed as `%dest`. * All variables representing pad values for `xfer_read` are renamed as `%pad`. * All vector variables (for `xfer_read` and `xfer_write`) are renamed as `%v`. * Add `@contiguous_inner_most_non_zero_idx_in_bounds_scalable` for `xfer_read` (similar test already exists for `xfer_write`) * All index variables are renamed as `%i` (1st index) and `%ii` (2nd index). The above were marked as TODOs in the test file - these are not resolved. In addition (to avoid sending another PR): * `@drop_inner_most_dim` is deleted - it duplicates `@contiguous_inner_most` for xfer_write * For consistency with other negative tests, renamed `@non_unit_strides` as `@negative_non_unit_strides` and added a similar test for `xfer_read` * `@non_unit_strides` is renamed as `@negative_non_unit_strides` and a similar test is added for `xfer_read`. This is a follow-up for: #94490, #94604, #94906, #96214, #96227

Summary: The main goal of this PR (and subsequent PRs), is to add more tests with scalable vectors to: * vector-transfer-collapse-inner-most-dims.mlir There's quite a few cases to consider, hence this is split into multiple PRs. In this PR, I am making the following changes: * All input memrefs for `xfer_read` are are renamed as `%src`. * All input memrefs for `xfer_write` are are renamed as `%dest`. * All variables representing pad values for `xfer_read` are renamed as `%pad`. * All vector variables (for `xfer_read` and `xfer_write`) are renamed as `%v`. * Add `@contiguous_inner_most_non_zero_idx_in_bounds_scalable` for `xfer_read` (similar test already exists for `xfer_write`) * All index variables are renamed as `%i` (1st index) and `%ii` (2nd index). The above were marked as TODOs in the test file - these are not resolved. In addition (to avoid sending another PR): * `@drop_inner_most_dim` is deleted - it duplicates `@contiguous_inner_most` for xfer_write * For consistency with other negative tests, renamed `@non_unit_strides` as `@negative_non_unit_strides` and added a similar test for `xfer_read` * `@non_unit_strides` is renamed as `@negative_non_unit_strides` and a similar test is added for `xfer_read`. This is a follow-up for: #94490, #94604, #94906, #96214, #96227 Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251592

llvmbot added mlir:vectorops mlir mlir:vector labels Jun 5, 2024

banach-space requested review from MacDue and hanhanW June 5, 2024 15:48

banach-space force-pushed the andrzej/update_collapse_inner_1 branch 2 times, most recently from 9fa450f to 839465c Compare June 6, 2024 10:51

banach-space mentioned this pull request Jun 6, 2024

[mlir][vector] Update tests for collapse 2/n (nfc) #94604

Merged

MacDue reviewed Jun 6, 2024

View reviewed changes

hanhanW mentioned this pull request Jun 6, 2024

[mlir][vector][NFC] Make function name more meaningful in lit tests. #94538

Merged

banach-space force-pushed the andrzej/update_collapse_inner_1 branch from e27a0f5 to 13c44b5 Compare June 7, 2024 10:27

banach-space added 2 commits June 9, 2024 14:16

fixup! [mlir][vector] Update tests for collapse 1/n (nfc)

e9731a0

Fix failing test

banach-space force-pushed the andrzej/update_collapse_inner_1 branch from 13c44b5 to e9731a0 Compare June 9, 2024 13:20

banach-space mentioned this pull request Jun 9, 2024

[mlir][vector] Restrict DropInnerMostUnitDimsTransferRead #94904

Merged

banach-space mentioned this pull request Jun 9, 2024

[mlir][vector] Update tests for collapse 3/n (nfc) #94906

Merged

banach-space merged commit d4c6478 into llvm:main Jun 11, 2024
7 checks passed

banach-space deleted the andrzej/update_collapse_inner_1 branch June 11, 2024 13:48

HerrCai0907 mentioned this pull request Jun 13, 2024

tidy #95384

Closed

banach-space mentioned this pull request Jun 14, 2024

Audit vector-transfer-collapse-inner-most-dims.mlir (scalable vectors) #95534

Closed

banach-space mentioned this pull request Jun 20, 2024

[mlir][vector] Update tests for collapse 4/n (nfc) #96214

Merged

banach-space mentioned this pull request Jun 20, 2024

[mlir][vector] Update tests for collapse 5/n (nfc) #96227

Merged

banach-space mentioned this pull request Jul 15, 2024

[mlir][vector] Update tests for collapse 6/n (nfc) #98902

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir][vector] Update tests for collapse 1/n (nfc) #94490

[mlir][vector] Update tests for collapse 1/n (nfc) #94490

Uh oh!

banach-space commented Jun 5, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jun 5, 2024 •

edited

Loading

Uh oh!

banach-space commented Jun 5, 2024

Uh oh!

MacDue commented Jun 6, 2024

Uh oh!

MacDue Jun 6, 2024

Uh oh!

banach-space Jun 6, 2024

Uh oh!

MacDue Jun 6, 2024

Uh oh!

banach-space Jun 7, 2024 •

edited

Loading

Uh oh!

banach-space commented Jun 6, 2024

Uh oh!

Uh oh!

Uh oh!

[mlir][vector] Update tests for collapse 1/n (nfc) #94490

[mlir][vector] Update tests for collapse 1/n (nfc) #94490

Uh oh!

Conversation

banach-space commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

banach-space commented Jun 5, 2024

Uh oh!

MacDue commented Jun 6, 2024

Uh oh!

MacDue Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

banach-space Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

MacDue Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

banach-space Jun 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

banach-space commented Jun 6, 2024

Uh oh!

Uh oh!

Uh oh!

banach-space commented Jun 5, 2024 •

edited

Loading

llvmbot commented Jun 5, 2024 •

edited

Loading

banach-space Jun 7, 2024 •

edited

Loading