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

[Igor Wodiany]

================
@@ -115,7 +115,7 @@ def SPIRV_ImageDrefGatherOp : SPIRV_Op<"ImageDrefGather",
 
   let assemblyFormat = [{
     $sampled_image `,` $coordinate `,` $dref custom<ImageOperands>($image_operands) ( `(` $operand_arguments^ `)` )? attr-dict 
-    `:` type($sampled_image) `,` type($coordinate) `,` type($dref) ( `(` type($operand_arguments)^ `)` )?
+    `:` type($sampled_image) `,` type($coordinate) `,` type($dref) ` ` ( `(` type($operand_arguments)^ `)` )?
----------------
IgWod-IMG wrote:

I'm not convinced about the comma after the bracket. If we look at the example with comma:

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

This can imply that operands are attached to `%2`. Unless we put a comma before the bracket as well:

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

But personally, I prefer no commas before or after operands list. But I have no strong feelings about it, so I'll let you decide.

> I'd prefer to update the existing op syntax first and then land the other PR that adds new ops with the same syntax

Sounds good, it's my preferred solution as well.


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