[PATCH] D89583: [Flang][OpenMP] Semantic checks for Atomic construct.

[sameeran joshi via Phabricator via llvm-commits]

sameeranjoshi marked 5 inline comments as done.
sameeranjoshi added a comment.

Thank you for reviewing.



================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:487
+void OmpStructureChecker::Leave(const parser::OmpAtomic &) {
+  if (const auto *clause{FindClause(llvm::omp::Clause::OMPC_acquire)}) {
+    context_.Say(clause->source,
----------------
clementval wrote:
> sameeranjoshi wrote:
> > clementval wrote:
> > > Where do you check that it is only for update? 
> > I check separately for both
> > `If atomic-clause is update or not present then memory-order-clause must not be acq_rel or acquire.`
> > break into below
> > restriction:
> > ```If atomic-clause is update then memory-order-clause must not be acq_rel or acquire.```
> > in
> > `void OmpStructureChecker::Leave(const parser::OmpAtomicUpdate &)`
> > &
> > ```If atomic-clause is not present then memory-order-clause must not be acq_rel or acquire.```
> > in this `Leave`.
> Ok I see. The `OmpAtomic` is the atomic construct without clause? If so, maybe adding small comment here to precise this would be nice. 
> Ok I see. The `OmpAtomic` is the atomic construct without clause? If so, maybe adding small comment here to precise this would be nice. 

To be specific it's not a general clause but called as 'atomic-clause' in standard.
Which has values read/write/update/capture.

To answer the question - yes it doesn't have 'atomic-clause'.
```
 !$omp atomic [clause[[,] clause] ... ] <no atomic clause here>
      update-statement
 [!$omp end atomic]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89583