[PATCH] D82177: [flang][OpenMP] Enhance parser support for flush construct to OpenMP 5.0

[Kiran Kumar T P via Phabricator via llvm-commits]

kiranktp added a comment.

In D82177#2105223 <https://reviews.llvm.org/D82177#2105223>, @kiranchandramohan wrote:

> Two comments.
>
>   1. memory-order-clause is defined for both atomic and flush instructions. We already have MemoryOrder defined in the parse-tree for atomic in 4.5. In 4.5 memory-order can only be seq_cst. In 5.0 memory-order for atomic can be seq_cst, acq_rel, release, acquire, relaxed. For flush it is only a subset and the allowed values are only acq_rel, release, acquire.
>
>     Would it make sense to have a common definition for memory-order-clause and handle the fact that it can only be a subset in semantics?
> 2. Flush is 2.17.8 (not 2.18.8) as part of the OpenMP 5.0 specification here. Is there a newer document? https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf


Thanks for the review Kiran.

> 1. memory-order-clause is defined for both atomic and flush instructions. We already have MemoryOrder defined in the parse-tree for atomic in 4.5. In 4.5 memory-order can only be seq_cst. In 5.0 memory-order for atomic can be seq_cst, acq_rel, release, acquire, relaxed. For flush it is only a subset and the allowed values are only acq_rel, release, acquire.
> 
>   Would it make sense to have a common definition for memory-order-clause and handle the fact that it can only be a subset in semantics?

I did thought of this approach, but keeping the definitions separate will be neat and will be easier to extend each construct for further changes in future.
I did see the same approach for couple of constructs handling.

> 2. Flush is 2.17.8 (not 2.18.8) as part of the OpenMP 5.0 specification here. Is there a newer document? https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf

Thanks for pointing it out. I was referring to OpenMP 5.1 draft version as well. I have used the same section numbers.
I will change it back to OpenMP 5.0 section number.
Probably i will have to change it for other construct as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82177