[MLIR][OpenMP] Removed the ambiguity in flush op assembly syntax

[kiranktp]

Summary

Bugzilla Ticket No: Bug 46884 [https://bugs.llvm.org/show_bug.cgi?id=46884]

Flush op assembly syntax was ambiguous:

Consider the below test case:

flush operation is not having any arguments.
But the next statement token i.e "%2" is read as the argument for flush operation and then translator issues an error.

---

$ cat -n flush.mlir

1  llvm.func @_QQmain(%arg0: !llvm.i32) {
2    %0 = llvm.mlir.constant(1 : i64) : !llvm.i64
3    %1 = llvm.alloca %0 x !llvm.i32 {in_type = i32, name = "a"} : (!llvm.i64) -> !llvm.ptr<i32>
4    omp.flush
5    %2 = llvm.load %1 : !llvm.ptr<i32>
6    llvm.return
7  }

$ mlir-translate -mlir-to-llvmir flush.mlir
flush.mlir:5:6: error: expected ':'

%2 = llvm.load %1 : !llvm.ptr<i32>
   ^

---

Solution:

Introduced begin ( ( ) and end token ( ) ) to determince the begin and end of variadic arguments.

The patch includes code changes and testcase modifications.

[mehdi_amini]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

127

Is this intended to put the attr-dict inside the parenthesis? Usually they are mostly trailing after the operands.

Also you could keep the parentheses optional as well by adding them inside the optional group, what do you think of:

( `(` $varList^ `:` type($varList) `)` )? attr-dict

[clementval]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

127

+1 for the optional parentheses. attr-dict position looks weird ... is it intended?

[kiranktp]

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

127

Thanks for reviewing Mehdi!, Valentine!

Also you could keep the parentheses optional as well
+1 for the optional parentheses.

Thanks for the suggestion. I will make these parenthesis optional too.

attr-dict position looks weird ... is it intended?

In all the examples that i have seen in documentation, attr-dict was in the beginning. So I didn't try to change the position as it was not causing any problem.
I will move it to the end.

[kiranktp]

Incorporated review comments.

[clementval]

LGTM