[Mlir-commits] [mlir] [mlir][spirv] Update assembly format for Image operand types (PR #130758)

[Igor Wodiany]

================
@@ -22,7 +22,7 @@ func.func @image_dref_gather_with_single_imageoperands(%arg0 : !spirv.sampled_im
 
 func.func @image_dref_gather_with_mismatch_imageoperands(%arg0 : !spirv.sampled_image<!spirv.image<i32, Dim2D, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>, %arg1 : vector<4xf32>, %arg2 : f32) -> () {
   // expected-error @+1 {{the Image Operands should encode what operands follow, as per Image Operands}}
-  %0 = spirv.ImageDrefGather %arg0, %arg1, %arg2 (%arg2, %arg2) : !spirv.sampled_image<!spirv.image<i32, Dim2D, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>, vector<4xf32>, f32 (f32, f32) -> vector<4xi32>
+  %0 = spirv.ImageDrefGather %arg0, %arg1, %arg2 %arg2, %arg2 : !spirv.sampled_image<!spirv.image<i32, Dim2D, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>, vector<4xf32>, f32, f32, f32 -> vector<4xi32>
----------------
IgWod-IMG wrote:

So, this will not parse with the current assembly format. But also, it's a degenerate case that mises Image Operands before optional arguments, hence the assembly looks weird in the test. So, we either keep it as it is now, or we can re-evaluate the format. Adding comma like you suggested before would make this a valid assembly that parses. So maybe we should go for this format in the end:

```mlir
%0 = spirv.Op %1, %2 ["IOp1 | IOp2"], %3, %4, %5 : !spirv.sampled_image<...>, vector<2xf32>, f32, f32, f32 -> vector<4xf32>
```

With the current example, I can see it's useful to have this comma. If you're happy to go with the comma as you initially suggested, I'll update the patch.

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