[flang-commits] [PATCH] D88965: [Flang][OpenMP] Rework parser changes for OpenMP atomic construct.

[sameeran joshi via Phabricator via flang-commits]

sameeranjoshi added a comment.

What I understand from your query is, you are basically trying to ask if we are using `OmpClauseList` in parser-tree.h so should all the clauses be allowed on atomic construct, which is not expected and you are trying to find where are the restrictions on the various clauses from `OmpClauseList` on Atomic construct.

In D88965#2326874 <https://reviews.llvm.org/D88965#2326874>, @kiranchandramohan wrote:

> @sameeranjoshi Is it incorrect to assume that the allowed clauses will work because it is handled by tablegen?

IIUC, it's partially correct.
The `allowedClauses` which are currently present in `OMP.td` will work in catching the test cases which don't obey what `OMP.td` has defined.
e.g for test cases with `num_threads` clause, which is not found in `allowedClauses` of `def OMP_Atomic` is caught in parsing stage.

  !!$omp atomic num_threads read
      i = j

Above test is a syntax error.

Now coming to what standard 5.0 says for one of the constraints:

  At most one memory-order-clause may appear on the construct. 

So a test like

  !$omp atomic SEQ_CST SEQ_CST read
       i  = j

Is currently not flagged as semantic error.
The possible reason which Valentine explained me was that "there should be at least a Enter function for each clauses with a CheckAllowed inside `check-omp-structure.cpp`".
And those functions lack in `check-omp-structure.cpp` which was my next patch.

> Like for e.g. will num_threads be accepted as a clause with atomic?

no.

> https://github.com/llvm/llvm-project/blob/1687a8d83b702332c4aae4f2a95d27c16688418d/llvm/include/llvm/Frontend/OpenMP/OMP.td#L444

please correct me If I am still missing something.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88965/new/

https://reviews.llvm.org/D88965