[Mlir-commits] [mlir] [mlir][Vector] Fix n-D vector.extract/insert lowering to LLVM (PR #87591)

[Diego Caballero]

================
@@ -738,6 +738,18 @@ func.func @extract_element_with_value_1d(%arg0: vector<16xf32>, %arg1: index) ->
 
 // -----
 
+func.func @extract_element_with_value_2d(%arg0: vector<1x16xf32>, %arg1: index) -> f32 {
+  %0 = vector.extract %arg0[0, %arg1]: f32 from vector<1x16xf32>
+  return %0 : f32
+}
+
+// Multi-dim vectors are not supported but this test shouldn't crash.
----------------
dcaballe wrote:

I agree with the regression point. The comment is helpful for the future reader to answer the question of "what is the goal of this test given that the vector.extract is not lowered to LLVM?". I haven't seen this kind of tests added redundantly as they are added as a result of a crash (so if someone hits a crash it's probably not covered by the existing tests).

We also have plenty of these tests all over the place so I wouldn't like to be a one-off just for this specific case, mostly because the absence of guidelines for regression tests would lead to people missing the "-unsupported" file and adding regression tests to this file instead. Perhaps we should define some guidelines in that regard...

In any case, let me know if you feel super strong about this and I'll create the new file. I'm just trying to be consistent with what we have today.

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