[Mlir-commits] [mlir] [mlir][Vector] Fix vector.insert folder for scalar to 0-d inserts (PR #113828)

[Andrzej Warzyński]

================
@@ -2745,6 +2745,18 @@ func.func @vector_insert_const_regression(%arg0: i8) -> vector<4xi8> {
 
 // -----
 
+// CHECK-LABEL: func @insert_into_0d_regression(
+//  CHECK-SAME:     %[[v:.*]]: vector<f32>)
+//       CHECK:   %[[extract:.*]] = vector.insert %{{.*}}, %[[v]] [] : f32 into vector<f32>
+//       CHECK:   return %[[extract]]
+func.func @insert_into_0d_regression(%v: vector<f32>) -> vector<f32> {
+  %cst = arith.constant 0.000000e+00 : f32
+  %0 = vector.insert %cst, %v [] : f32 into vector<f32>
+  return %0 : vector<f32>
+}
----------------
banach-space wrote:

> The reason for the name was, ExtractOp::fold had the same bug before and extract_from_0d_regression is the test name for that :)

Consistency is key, I agree. Let's rename `@extract_from_0d_regression` as well so that tests for `vector.insert` and `vector.extract` are symmetrical (at least for the logic that's nearly identical).

> There are no tests for vector::InsertOp::Fold. 

This explains why we keep discovering these bugs - if we don't test the folder, we can't really claim it "works" ™️ . Let's fix that! 

Could you please add tests for `vector::InsertOp::Fold`? Both positive (missing) and negative (already included in this PR). Some Ops follow this naming convention that makes sense to me:
* `@<op_name>_fold{_<suffix>}` - positive tests,
* `@<op_name>_no_fold{_<suffix>}` - negative tests.

Sadly, tests for `vector.extract` are more like `@fold_extract_shapecast` 😢 Pick one style and I'll try to unify things when adding tests for scalable vectors.

You can add the new tests near where `ExtractOp::fold` is tested (e.g. after `@dont_fold_0d_extract_shapecast`). I can't think of a better place just now.

Thanks for improving this!

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