[Flang][OpenMP] Semantic checks for Atomic construct.

[sameeranjoshi]

Summary

Patch implements restrictions from 2.17.7 of OpenMP 5.0 standard for atomic Construct. Tests for the same are added.

One of the restriction
OpenMP constructs may not be encountered during execution of an atomic region.
Is mentioned in 5.0 standard to be a semantic restriction, but given the stricter nature of parser in F18 it's caught at parsing itself.

This patch is a next patch in series from D88965.

[clementval]

flang/test/Semantics/omp-atomic01.f90

5

Regarding your general comment. hint does not have a parser::Hint since it is in the OmpClause variant. So you should be able to resolve this todo in this patch as well.

[sameeranjoshi]

Resolves the TODO, by fixing a small bug for Hint.
Adds more test cases for At most one hint clause may appear on the construct.

[sameeranjoshi]

flang/test/Semantics/omp-atomic01.f90

5

Thank you @clementval.
I have resolved the TODO.
OMPC_Hint inside OMP.td was supposed to use flangClassValue instead of flangClass.

[clementval]

flang/lib/Semantics/check-omp-structure.cpp

10

Is this necessary?

486

Why is there a specific check for this? If it is not allowed it should not be in OMP.td and then catch be CheckAllowed in enter acquire.

490

Same here. This should be catch be the generic code and the TableGen map.

flang/lib/Semantics/check-omp-structure.h

162

These are not really clause but atomic constructs.

[sameeranjoshi]

flang/lib/Semantics/check-omp-structure.cpp

10

For common::visitors I will see of they can be removed.

486

Standard says:
If atomic-clause is update or not present then memory-order-clause must not be acq_rel or acquire.

I couldn't use CheckNotAllowedIfClause here reason being there is no OMPC_atomic.
I see OMPC_atomic_default_mem_order but I doubt it's the same what I was trying to find.

also aquire acq_rel are not allowed only here they might be allowed somewhere else, hence changing them in OMP.td would affect other atomic directives as well.

[clementval]

flang/lib/Semantics/check-omp-structure.cpp

486

Then the check for update is missing? and it will fail for atomic read even it should be allowed?

[kiranchandramohan]

flang/lib/Semantics/check-omp-structure.h

155

Thinking about this again. It will be good if we can retain memory-order-clause in the parse tree. Since the standard specifically talks about memory-order-clauses. If we write a frontend tool to list all memory order clauses then if there is a representation for memory-order-clause in the parse tree it will be easy to process all memory-order-clauses.

[sameeranjoshi]

flang/lib/Semantics/check-omp-structure.h

155

The whole discussion to change parser started when failing to implement semantic checks(given that we had code) without changing parser.

Reason being that memory-order-clauses were not part of OmpClause inside openmp-parsers.cpp.

I am happy to change it If that's possible.
Also if @clementval shares some opinions on this issue, he might have a better overview.

[clementval]

flang/lib/Semantics/check-omp-structure.h

155

Well, I don't have a strong opinion on that but if you want to use the directive-clauses map generated by TableGen you need to have clauses in OmpClause.

I think there could be a way to keep the clauses in OmpClause and have a separate parser that handle the memory-order-clauses and that could be applied to Atomic for example.

[clementval]

I guess you need to rebase this now that the memory-order-clause is in.

[sameeranjoshi]

Changes in atomic to use OmpAtomicClauseList after D91839 ( memory-order-clause related).

[sameeranjoshi]

flang/lib/Semantics/check-omp-structure.cpp

486

Why is there a specific check for this?

Due to missing clause for ? inside below function which isn't in OMP.td.

CheckNotAllowedIfClause( ? ,
      {llvm::omp::Clause::OMPC_acquire, llvm::omp::Clause::OMPC_acq_rel})

Reason being that there is nothing mentioned inside TableGen.

If it is not allowed it should not be in OMP.td and then catch be CheckAllowed in enter acquire.

If we remove it from OMP.td that would be not found for other atomic clauses where it was expected to work.

486

Then the check for update is missing?

umm... update check was added and I see that we have
test with comment ! 2.17.7.6 which works well due to CheckNotAllowedIfClause which fits for llvm::omp::Clause::OMPC_update

I might not be still clear on how to leverage TableGen for this particular test case.
I am happy to use TableGen wherever possible.
I even tried to add OMPC_Atomic but that's not helping.

flang/lib/Semantics/check-omp-structure.h

155

Handled by D91839.

[sameeranjoshi]

Ping.

[clementval]

flang/include/flang/Parser/parse-tree.h

3634

Is the comment at the right place?

flang/lib/Semantics/check-omp-structure.cpp

487

Where do you check that it is only for update?

[sameeranjoshi]

flang/include/flang/Parser/parse-tree.h

3634

No, thx.

flang/lib/Semantics/check-omp-structure.cpp

487

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.

[clementval]

Looks ok to me. Just a small comment on the OmpAtomic leave

flang/lib/Semantics/check-omp-structure.cpp

487

Ok I see. The OmpAtomic is the atomic construct without clause? If so, maybe adding small comment here to precise this would be nice.

[sameeranjoshi]

Thank you for reviewing.

flang/lib/Semantics/check-omp-structure.cpp

487

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]

[clementval]

flang/lib/Semantics/check-omp-structure.cpp

487

Yeah I get it. Since other "with" clause are represented by a node as well like OmpAtomicUpdate I still think it would make sense to have a small comment here to mentioned that the OmpAtomic node represent and atomic directive without atomic-clause

[sameeranjoshi]

Add a clarification comment.

[clementval]

In D89583#2446451, @sameeranjoshi wrote:

Add a clarification comment.

Thanks for adding this. Looks good to land from my side

flang/include/flang/Parser/dump-parse-tree.h

#include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   NODE(parser, OpenMPCancellationPointConstruct)
   NODE(parser, OpenMPConstruct)
   NODE(parser, OpenMPCriticalConstruct)
   NODE(parser, OpenMPDeclarativeConstruct)
   NODE(parser, OpenMPDeclareReductionConstruct)
   NODE(parser, OpenMPDeclareSimdConstruct)
   NODE(parser, OpenMPDeclareTargetConstruct)
   NODE(parser, OmpMemoryOrderClause)
+  NODE(parser, OmpAtomicClause)
+  NODE(parser, OmpAtomicClauseList)
   NODE(parser, OpenMPFlushConstruct)
   NODE(parser, OpenMPLoopConstruct)
   NODE(parser, OpenMPSimpleStandaloneConstruct)
   NODE(parser, OpenMPStandaloneConstruct)
   NODE(parser, OpenMPSectionsConstruct)
   NODE(parser, OpenMPThreadprivate)
   NODE(parser, OpenStmt)
   NODE(parser, Optional)

flang/include/flang/Parser/parse-tree.h

struct OmpEndCriticalDirective {
   CharBlock source;
   std::tuple<Verbatim, std::optional<Name>> t;
 };
 struct OpenMPCriticalConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
   std::tuple<OmpCriticalDirective, Block, OmpEndCriticalDirective> t;
 };
 
-// 2.17.7 atomic -> ATOMIC [clause[,]] atomic-clause [[,]clause] |
-//                  ATOMIC [clause]
-//        clause -> memory-order-clause | HINT(hint-expression)
-//        memory-order-clause -> SEQ_CST | ACQ_REL | RELEASE | ACQUIRE | RELAXED
-//        atomic-clause -> READ | WRITE | UPDATE | CAPTURE
+// 2.17.7 Atomic construct/2.17.8 Flush construct [OpenMP 5.0]
+//        memory-order-clause -> acq_rel
+//                               release
+//                               acquire
+//                               seq_cst
+//                               relaxed
+struct OmpMemoryOrderClause {
+  WRAPPER_CLASS_BOILERPLATE(OmpMemoryOrderClause, OmpClause);
+  CharBlock source;
+};
+
+// 2.17.7 Atomic construct
+//        atomic-clause -> memory-order-clause | HINT(hint-expression)
+struct OmpAtomicClause {
+  UNION_CLASS_BOILERPLATE(OmpAtomicClause);
+  CharBlock source;
+  std::variant<OmpMemoryOrderClause, OmpClause> u;
+};
+
+// atomic-clause-list -> [atomic-clause, [atomic-clause], ...]
+struct OmpAtomicClauseList {
+  WRAPPER_CLASS_BOILERPLATE(OmpAtomicClauseList, std::list<OmpAtomicClause>);
+  CharBlock source;
+};
+// 2.17.7 atomic ->
+//        ATOMIC [atomic-clause-list] atomic-construct [atomic-clause-list] |
+//        ATOMIC [atomic-clause-list]
+//        atomic-construct -> READ | WRITE | UPDATE | CAPTURE
 
 // END ATOMIC
 EMPTY_CLASS(OmpEndAtomic);
 
 // ATOMIC READ
 struct OmpAtomicRead {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicRead);
   CharBlock source;
-  std::tuple<OmpClauseList, Verbatim, OmpClauseList, Statement<AssignmentStmt>,
-      std::optional<OmpEndAtomic>>
+  std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
+      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
       t;
 };
 
 // ATOMIC WRITE
 struct OmpAtomicWrite {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicWrite);
   CharBlock source;
-  std::tuple<OmpClauseList, Verbatim, OmpClauseList, Statement<AssignmentStmt>,
-      std::optional<OmpEndAtomic>>
+  std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
+      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
       t;
 };
 
 // ATOMIC UPDATE
 struct OmpAtomicUpdate {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicUpdate);
   CharBlock source;
-  std::tuple<OmpClauseList, Verbatim, OmpClauseList, Statement<AssignmentStmt>,
-      std::optional<OmpEndAtomic>>
+  std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList,
+      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
       t;
 };
 
 // ATOMIC CAPTURE
 struct OmpAtomicCapture {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicCapture);
   CharBlock source;
   WRAPPER_CLASS(Stmt1, Statement<AssignmentStmt>);
   WRAPPER_CLASS(Stmt2, Statement<AssignmentStmt>);
-  std::tuple<OmpClauseList, Verbatim, OmpClauseList, Stmt1, Stmt2, OmpEndAtomic>
+  std::tuple<OmpAtomicClauseList, Verbatim, OmpAtomicClauseList, Stmt1, Stmt2,
+      OmpEndAtomic>
       t;
 };
 
 // ATOMIC
 struct OmpAtomic {
   TUPLE_CLASS_BOILERPLATE(OmpAtomic);
   CharBlock source;
-  std::tuple<Verbatim, OmpClauseList, Statement<AssignmentStmt>,
+  std::tuple<Verbatim, OmpAtomicClauseList, Statement<AssignmentStmt>,
       std::optional<OmpEndAtomic>>
       t;
 };
 
 struct OpenMPAtomicConstruct {
   UNION_CLASS_BOILERPLATE(OpenMPAtomicConstruct);
   std::variant<OmpAtomicRead, OmpAtomicWrite, OmpAtomicCapture, OmpAtomicUpdate,
       OmpAtomic>

 // 2.14.1 cancel -> CANCEL construct-type-clause [ [,] if-clause]
 struct OpenMPCancelConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPCancelConstruct);
   WRAPPER_CLASS(If, ScalarLogicalExpr);
   CharBlock source;
   std::tuple<Verbatim, OmpCancelType, std::optional<If>> t;
 };
 
-// 2.17.8 Flush Construct [OpenMP 5.0]
-// memory-order-clause -> acq_rel
-//                        release
-//                        acquire
-struct OmpMemoryOrderClause {
-  WRAPPER_CLASS_BOILERPLATE(OmpMemoryOrderClause, OmpClause);
-  CharBlock source;
-};
 
 // 2.17.8 flush -> FLUSH [memory-order-clause] [(variable-name-list)]
 struct OpenMPFlushConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPFlushConstruct);
   CharBlock source;
   std::tuple<Verbatim, std::optional<OmpMemoryOrderClause>,
       std::optional<OmpObjectList>>
       t;

flang/lib/Parser/openmp-parsers.cpp

 //                               relaxed
 TYPE_PARSER(sourced(construct<OmpMemoryOrderClause>(
     sourced("SEQ_CST" >> construct<OmpClause>(construct<OmpClause::SeqCst>()) ||
         "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
         "RELEASE" >> construct<OmpClause>(construct<OmpClause::Release>()) ||
         "ACQUIRE" >> construct<OmpClause>(construct<OmpClause::Acquire>()) ||
         "RELAXED" >> construct<OmpClause>(construct<OmpClause::Relaxed>())))))
 
+// 2.17.7 Atomic construct
+//        atomic-clause -> memory-order-clause | HINT(hint-expression)
+TYPE_PARSER(sourced(construct<OmpAtomicClause>(
+    construct<OmpAtomicClause>(Parser<OmpMemoryOrderClause>{}) ||
+    construct<OmpAtomicClause>("HINT" >>
+        sourced(construct<OmpClause>(
+            construct<OmpClause::Hint>(parenthesized(constantExpr))))))))
+
+// atomic-clause-list -> [atomic-clause, [atomic-clause], ...]
+TYPE_PARSER(sourced(construct<OmpAtomicClauseList>(
+    many(maybe(","_tok) >> sourced(Parser<OmpAtomicClause>{})))))
+
 TYPE_PARSER(sourced(construct<OpenMPFlushConstruct>(verbatim("FLUSH"_tok),
     maybe(Parser<OmpMemoryOrderClause>{}),
     maybe(parenthesized(Parser<OmpObjectList>{})))))
 
 // Simple Standalone Directives
 TYPE_PARSER(sourced(construct<OmpSimpleStandaloneDirective>(first(
     "BARRIER" >> pure(llvm::omp::Directive::OMPD_barrier),
     "ORDERED" >> pure(llvm::omp::Directive::OMPD_ordered),

 //       memory-order-clause -> SEQ_CST | ACQ_REL | RELEASE | ACQUIRE | RELAXED
 //       atomic-clause -> READ | WRITE | UPDATE | CAPTURE
 
 // OMP END ATOMIC
 TYPE_PARSER(construct<OmpEndAtomic>(startOmpLine >> "END ATOMIC"_tok))
 
 // OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] READ [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicRead>(Parser<OmpClauseList>{} / maybe(","_tok),
-        verbatim("READ"_tok), Parser<OmpClauseList>{} / endOmpLine,
+    construct<OmpAtomicRead>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
+        verbatim("READ"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
         statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
 // OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] CAPTURE [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicCapture>(Parser<OmpClauseList>{} / maybe(","_tok),
-        verbatim("CAPTURE"_tok), Parser<OmpClauseList>{} / endOmpLine,
+    construct<OmpAtomicCapture>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
+        verbatim("CAPTURE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
         statement(assignmentStmt), statement(assignmentStmt),
         Parser<OmpEndAtomic>{} / endOmpLine))
 
 // OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] UPDATE [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicUpdate>(Parser<OmpClauseList>{} / maybe(","_tok),
-        verbatim("UPDATE"_tok), Parser<OmpClauseList>{} / endOmpLine,
+    construct<OmpAtomicUpdate>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
+        verbatim("UPDATE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
         statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
-// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST]
+// OMP ATOMIC [atomic-clause-list]
 TYPE_PARSER(construct<OmpAtomic>(verbatim("ATOMIC"_tok),
-    Parser<OmpClauseList>{} / endOmpLine, statement(assignmentStmt),
+    Parser<OmpAtomicClauseList>{} / endOmpLine, statement(assignmentStmt),
     maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
 // OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] WRITE [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicWrite>(Parser<OmpClauseList>{} / maybe(","_tok),
-        verbatim("WRITE"_tok), Parser<OmpClauseList>{} / endOmpLine,
+    construct<OmpAtomicWrite>(Parser<OmpAtomicClauseList>{} / maybe(","_tok),
+        verbatim("WRITE"_tok), Parser<OmpAtomicClauseList>{} / endOmpLine,
         statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
 // Atomic Construct
 TYPE_PARSER(construct<OpenMPAtomicConstruct>(Parser<OmpAtomicRead>{}) ||
     construct<OpenMPAtomicConstruct>(Parser<OmpAtomicCapture>{}) ||
     construct<OpenMPAtomicConstruct>(Parser<OmpAtomicWrite>{}) ||
     construct<OpenMPAtomicConstruct>(Parser<OmpAtomicUpdate>{}) ||
     construct<OpenMPAtomicConstruct>(Parser<OmpAtomic>{}))

flang/lib/Parser/unparse.cpp

void Unparse(const OmpBlockDirective &x) {
     case llvm::omp::Directive::OMPD_workshare:
       Word("WORKSHARE ");
       break;
     default:
       // Nothing to be done
       break;
     }
   }
+  void Unparse(const OmpAtomicClauseList &x) { Walk(" ", x.v, " "); }
+
   void Unparse(const OmpAtomic &x) {
     BeginOpenMP();
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpClauseList>(x.t));
+    Walk(std::get<OmpAtomicClauseList>(x.t));
     Put("\n");
     EndOpenMP();
     Walk(std::get<Statement<AssignmentStmt>>(x.t));
     BeginOpenMP();
     Walk(std::get<std::optional<OmpEndAtomic>>(x.t), "!$OMP END ATOMIC\n");
     EndOpenMP();
   }
   void Unparse(const OmpAtomicCapture &x) {
void Unparse(const OpenMPCancelConstruct &x) {
     BeginOpenMP();
     Word("!$OMP CANCEL ");
     Walk(std::get<OmpCancelType>(x.t));
     Walk(std::get<std::optional<OpenMPCancelConstruct::If>>(x.t));
     Put("\n");
     EndOpenMP();
   }
   void Unparse(const OmpMemoryOrderClause &x) { Walk(x.v); }
+  void Unparse(const OmpAtomicClause &x) {
+    std::visit(common::visitors{
+                   [&](const OmpMemoryOrderClause &y) { Walk(y); },
+                   [&](const OmpClause &z) { Walk(z); },
+               },
+        x.u);
+  }
   void Unparse(const OpenMPFlushConstruct &x) {
     BeginOpenMP();
     Word("!$OMP FLUSH ");
     Walk(std::get<std::optional<OmpMemoryOrderClause>>(x.t));
     Walk(" (", std::get<std::optional<OmpObjectList>>(x.t), ")");
     Put("\n");
     EndOpenMP();
   }

flang/lib/Semantics/check-omp-structure.h

#include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   void Enter(const parser::OpenMPFlushConstruct &);
   void Leave(const parser::OpenMPFlushConstruct &);
   void Enter(const parser::OpenMPCancelConstruct &);
   void Leave(const parser::OpenMPCancelConstruct &);
   void Enter(const parser::OpenMPCancellationPointConstruct &);
   void Leave(const parser::OpenMPCancellationPointConstruct &);
   void Enter(const parser::OpenMPCriticalConstruct &);
   void Leave(const parser::OpenMPCriticalConstruct &);
+  void Enter(const parser::OpenMPAtomicConstruct &);
+  void Leave(const parser::OpenMPAtomicConstruct &);
 
   void Leave(const parser::OmpClauseList &);
   void Enter(const parser::OmpClause &);
   void Enter(const parser::OmpNowait &);
   void Enter(const parser::OmpClause::Inbranch &);
   void Enter(const parser::OmpClause::Mergeable &);
   void Enter(const parser::OmpClause::Nogroup &);
   void Enter(const parser::OmpClause::Notinbranch &);
#include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   void Enter(const parser::OmpClause::Shared &);
   void Enter(const parser::OmpClause::Simdlen &);
   void Enter(const parser::OmpClause::ThreadLimit &);
   void Enter(const parser::OmpClause::To &);
   void Enter(const parser::OmpClause::Link &);
   void Enter(const parser::OmpClause::Uniform &);
   void Enter(const parser::OmpClause::UseDevicePtr &);
   void Enter(const parser::OmpClause::IsDevicePtr &);
   // Memory-order-clause
   void Enter(const parser::OmpClause::SeqCst &);
   void Enter(const parser::OmpClause::AcqRel &);
   void Enter(const parser::OmpClause::Release &);
   void Enter(const parser::OmpClause::Acquire &);
   void Enter(const parser::OmpClause::Relaxed &);
+  void Enter(const parser::OmpClause::Hint &);
 
+  void Enter(const parser::OmpAtomicRead &);
+  void Leave(const parser::OmpAtomicRead &);
+  void Enter(const parser::OmpAtomicWrite &);
+  void Leave(const parser::OmpAtomicWrite &);
+  void Enter(const parser::OmpAtomicUpdate &);
+  void Leave(const parser::OmpAtomicUpdate &);
+  void Enter(const parser::OmpAtomicCapture &);
+  void Leave(const parser::OmpAtomic &);
   void Enter(const parser::OmpAlignedClause &);
   void Enter(const parser::OmpAllocateClause &);
   void Enter(const parser::OmpDefaultClause &);
   void Enter(const parser::OmpDefaultmapClause &);
   void Enter(const parser::OmpDependClause &);
   void Enter(const parser::OmpDistScheduleClause &);
   void Enter(const parser::OmpIfClause &);
   void Enter(const parser::OmpLinearClause &);

flang/lib/Semantics/check-omp-structure.cpp

 //===-- lib/Semantics/check-omp-structure.cpp -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "check-omp-structure.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/tools.h"
 #include <algorithm>
 
 namespace Fortran::semantics {
 
 // Use when clause falls under 'struct OmpClause' in 'parse-tree.h'.
 #define CHECK_SIMPLE_CLAUSE(X, Y) \
   void OmpStructureChecker::Enter(const parser::OmpClause::X &) { \
PushContextAndClauseSets(
         dir.source, llvm::omp::Directive::OMPD_end_workshare);
     break;
   default:
     // no clauses are allowed
     break;
   }
 }
 
+void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
+  std::visit(
+      common::visitors{
+          [&](const auto &someAtomicConstruct) {
+            const auto &dir{std::get<parser::Verbatim>(someAtomicConstruct.t)};
+            PushContextAndClauseSets(
+                dir.source, llvm::omp::Directive::OMPD_atomic);
+          },
+      },
+      x.u);
+}
+
+void OmpStructureChecker::Leave(const parser::OpenMPAtomicConstruct &) {
+  dirContext_.pop_back();
+}
+
 // Clauses
 // Mainly categorized as
 // 1. Checks on 'OmpClauseList' from 'parse-tree.h'.
 // 2. Checks on clauses which fall under 'struct OmpClause' from parse-tree.h.
 // 3. Checks on clauses which are not in 'struct OmpClause' from parse-tree.h.
 
 void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
   // 2.7 Loop Construct Restriction

 CHECK_SIMPLE_CLAUSE(Uniform, OMPC_uniform)
 CHECK_SIMPLE_CLAUSE(Untied, OMPC_untied)
 CHECK_SIMPLE_CLAUSE(UseDevicePtr, OMPC_use_device_ptr)
 CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
 CHECK_SIMPLE_CLAUSE(Acquire, OMPC_acquire)
 CHECK_SIMPLE_CLAUSE(SeqCst, OMPC_seq_cst)
 CHECK_SIMPLE_CLAUSE(Release, OMPC_release)
 CHECK_SIMPLE_CLAUSE(Relaxed, OMPC_relaxed)
+CHECK_SIMPLE_CLAUSE(Hint, OMPC_hint)
 
 CHECK_REQ_SCALAR_INT_CLAUSE(Grainsize, OMPC_grainsize)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTasks, OMPC_num_tasks)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumTeams, OMPC_num_teams)
 CHECK_REQ_SCALAR_INT_CLAUSE(NumThreads, OMPC_num_threads)
 CHECK_REQ_SCALAR_INT_CLAUSE(Priority, OMPC_priority)
 CHECK_REQ_SCALAR_INT_CLAUSE(ThreadLimit, OMPC_thread_limit)
 

 }
 // Following clauses have a seperate node in parse-tree.h.
 CHECK_SIMPLE_PARSER_CLAUSE(OmpAllocateClause, OMPC_allocate)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpDefaultClause, OMPC_default)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpDistScheduleClause, OMPC_dist_schedule)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpNowait, OMPC_nowait)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpProcBindClause, OMPC_proc_bind)
 CHECK_SIMPLE_PARSER_CLAUSE(OmpReductionClause, OMPC_reduction)
-
+// Atomic-clause
+CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicRead, OMPC_read)
+CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicWrite, OMPC_write)
+CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicUpdate, OMPC_update)
+CHECK_SIMPLE_PARSER_CLAUSE(OmpAtomicCapture, OMPC_capture)
+
+void OmpStructureChecker::Leave(const parser::OmpAtomicRead &) {
+  CheckNotAllowedIfClause(llvm::omp::Clause::OMPC_read,
+      {llvm::omp::Clause::OMPC_release, llvm::omp::Clause::OMPC_acq_rel});
+}
+void OmpStructureChecker::Leave(const parser::OmpAtomicWrite &) {
+  CheckNotAllowedIfClause(llvm::omp::Clause::OMPC_write,
+      {llvm::omp::Clause::OMPC_acquire, llvm::omp::Clause::OMPC_acq_rel});
+}
+void OmpStructureChecker::Leave(const parser::OmpAtomicUpdate &) {
+  CheckNotAllowedIfClause(llvm::omp::Clause::OMPC_update,
+      {llvm::omp::Clause::OMPC_acquire, llvm::omp::Clause::OMPC_acq_rel});
+}
+void OmpStructureChecker::Leave(const parser::OmpAtomic &) {
+  if (const auto *clause{FindClause(llvm::omp::Clause::OMPC_acquire)}) {
+    context_.Say(clause->source,
+        "Clause ACQUIRE is not allowed on the ATOMIC directive"_err_en_US);
+  }
+  if (const auto *clause{FindClause(llvm::omp::Clause::OMPC_acq_rel)}) {
+    context_.Say(clause->source,
+        "Clause ACQ_REL is not allowed on the ATOMIC directive"_err_en_US);
+  }
+}
 // Restrictions specific to each clause are implemented apart from the
 // generalized restrictions.
 void OmpStructureChecker::Enter(const parser::OmpAlignedClause &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_aligned);
 
   if (const auto &expr{
           std::get<std::optional<parser::ScalarIntConstantExpr>>(x.t)}) {
     RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_aligned, *expr);

flang/test/Semantics/omp-atomic.f90

! Check OpenMP 2.13.6 atomic Construct
 
   !ERROR: expected end of line
   !ERROR: expected end of line
   !$omp atomic read write
   a = a + 1
 
   !$omp atomic
   a = a + 1
+  !ERROR: expected 'UPDATE'
+  !ERROR: expected 'WRITE'
+  !ERROR: expected 'CAPTURE'
+  !ERROR: expected 'READ'
+  !$omp atomic num_threads(4)
+  a = a + 1
+
+  !ERROR: expected end of line
+  !ERROR: expected end of line
+  !$omp atomic capture num_threads(4)
+  a = a + 1
 
   !$omp atomic relaxed
   a = a + 1
 
   !ERROR: expected 'UPDATE'
   !ERROR: expected 'WRITE'
   !ERROR: expected 'CAPTURE'
   !ERROR: expected 'READ'
   !$omp atomic num_threads write
   a = a + 1
 
   !$omp end parallel
 end

flang/test/Semantics/omp-atomic01.f90

+! RUN: %S/test_errors.sh %s %t %f18 -fopenmp
+! Semantic checks for OpenMP 5.0 standard 2.17.7 atomic Construct.
+
+use omp_lib
+  implicit none
+  integer :: i, j = 10, k=-100, a
+! 2.17.7.1
+! Handled inside parser.
+! OpenMP constructs may not be encountered during execution of an atomic region
+
+! 2.17.7.2
+! At most one memory-order-clause may appear on the construct.
+
+!READ
+  !ERROR: At most one SEQ_CST clause can appear on the READ directive
+  !$omp atomic seq_cst seq_cst read
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the READ directive
+  !$omp atomic read seq_cst seq_cst
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the READ directive
+  !$omp atomic seq_cst read seq_cst
+    i = j
+
+  !ERROR: At most one ACQUIRE clause can appear on the READ directive
+  !$omp atomic acquire acquire read
+    i = j
+  !ERROR: At most one ACQUIRE clause can appear on the READ directive
+  !$omp atomic read acquire acquire
+    i = j
+  !ERROR: At most one ACQUIRE clause can appear on the READ directive
+  !$omp atomic acquire read acquire
+    i = j
+
+  !ERROR: At most one RELAXED clause can appear on the READ directive
+  !$omp atomic relaxed relaxed read
+    i = j
+  !ERROR: At most one RELAXED clause can appear on the READ directive
+  !$omp atomic read relaxed relaxed
+    i = j
+  !ERROR: At most one RELAXED clause can appear on the READ directive
+  !$omp atomic relaxed read relaxed
+    i = j
+
+!UPDATE
+  !ERROR: At most one SEQ_CST clause can appear on the UPDATE directive
+  !$omp atomic seq_cst seq_cst update
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the UPDATE directive
+  !$omp atomic update seq_cst seq_cst
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the UPDATE directive
+  !$omp atomic seq_cst update seq_cst
+    i = j
+
+  !ERROR: At most one RELEASE clause can appear on the UPDATE directive
+  !$omp atomic release release update
+    i = j
+  !ERROR: At most one RELEASE clause can appear on the UPDATE directive
+  !$omp atomic update release release
+    i = j
+  !ERROR: At most one RELEASE clause can appear on the UPDATE directive
+  !$omp atomic release update release
+    i = j
+
+  !ERROR: At most one RELAXED clause can appear on the UPDATE directive
+  !$omp atomic relaxed relaxed update
+    i = j
+  !ERROR: At most one RELAXED clause can appear on the UPDATE directive
+  !$omp atomic update relaxed relaxed
+    i = j
+  !ERROR: At most one RELAXED clause can appear on the UPDATE directive
+  !$omp atomic relaxed update relaxed
+    i = j
+
+!CAPTURE
+  !ERROR: At most one SEQ_CST clause can appear on the CAPTURE directive
+  !$omp atomic seq_cst seq_cst capture
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one SEQ_CST clause can appear on the CAPTURE directive
+  !$omp atomic capture seq_cst seq_cst
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one SEQ_CST clause can appear on the CAPTURE directive
+  !$omp atomic seq_cst capture seq_cst
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one RELEASE clause can appear on the CAPTURE directive
+  !$omp atomic release release capture
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one RELEASE clause can appear on the CAPTURE directive
+  !$omp atomic capture release release
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one RELEASE clause can appear on the CAPTURE directive
+  !$omp atomic release capture release
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one RELAXED clause can appear on the CAPTURE directive
+  !$omp atomic relaxed relaxed capture
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one RELAXED clause can appear on the CAPTURE directive
+  !$omp atomic capture relaxed relaxed
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one RELAXED clause can appear on the CAPTURE directive
+  !$omp atomic relaxed capture relaxed
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one ACQ_REL clause can appear on the CAPTURE directive
+  !$omp atomic acq_rel acq_rel capture
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one ACQ_REL clause can appear on the CAPTURE directive
+  !$omp atomic capture acq_rel acq_rel
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one ACQ_REL clause can appear on the CAPTURE directive
+  !$omp atomic acq_rel capture acq_rel
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one ACQUIRE clause can appear on the CAPTURE directive
+  !$omp atomic acquire acquire capture
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one ACQUIRE clause can appear on the CAPTURE directive
+  !$omp atomic capture acquire acquire
+    i = j
+    j = k
+  !$omp end atomic
+
+  !ERROR: At most one ACQUIRE clause can appear on the CAPTURE directive
+  !$omp atomic acquire capture acquire
+    i = j
+    j = k
+  !$omp end atomic
+
+!WRITE
+  !ERROR: At most one SEQ_CST clause can appear on the WRITE directive
+  !$omp atomic seq_cst seq_cst write
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the WRITE directive
+  !$omp atomic write seq_cst seq_cst
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the WRITE directive
+  !$omp atomic seq_cst write seq_cst
+    i = j
+
+  !ERROR: At most one RELEASE clause can appear on the WRITE directive
+  !$omp atomic release release write
+    i = j
+  !ERROR: At most one RELEASE clause can appear on the WRITE directive
+  !$omp atomic write release release
+    i = j
+  !ERROR: At most one RELEASE clause can appear on the WRITE directive
+  !$omp atomic release write release
+    i = j
+
+  !ERROR: At most one RELAXED clause can appear on the WRITE directive
+  !$omp atomic relaxed relaxed write
+    i = j
+  !ERROR: At most one RELAXED clause can appear on the WRITE directive
+  !$omp atomic write relaxed relaxed
+    i = j
+  !ERROR: At most one RELAXED clause can appear on the WRITE directive
+  !$omp atomic relaxed write relaxed
+    i = j
+
+!No atomic-clause
+  !ERROR: At most one RELAXED clause can appear on the ATOMIC directive
+  !$omp atomic relaxed relaxed
+    i = j
+  !ERROR: At most one SEQ_CST clause can appear on the ATOMIC directive
+  !$omp atomic seq_cst seq_cst
+    i = j
+  !ERROR: At most one RELEASE clause can appear on the ATOMIC directive
+  !$omp atomic release release
+    i = j
+
+! 2.17.7.3
+! At most one hint clause may appear on the construct.
+
+  !ERROR: At most one HINT clause can appear on the READ directive
+  !$omp atomic hint(omp_sync_hint_speculative) hint(omp_sync_hint_speculative) read
+    i = j
+  !ERROR: At most one HINT clause can appear on the READ directive
+  !$omp atomic hint(omp_sync_hint_nonspeculative) read hint(omp_sync_hint_nonspeculative)
+    i = j
+  !ERROR: At most one HINT clause can appear on the READ directive
+  !$omp atomic read hint(omp_sync_hint_uncontended) hint (omp_sync_hint_uncontended)
+    i = j
+  !ERROR: At most one HINT clause can appear on the WRITE directive
+  !$omp atomic hint(omp_sync_hint_contended) hint(omp_sync_hint_speculative) write
+    i = j
+  !ERROR: At most one HINT clause can appear on the WRITE directive
+  !$omp atomic hint(omp_sync_hint_nonspeculative) write hint(omp_sync_hint_nonspeculative)
+    i = j
+  !ERROR: At most one HINT clause can appear on the WRITE directive
+  !$omp atomic write hint(omp_sync_hint_none) hint (omp_sync_hint_uncontended)
+    i = j
+  !ERROR: At most one HINT clause can appear on the WRITE directive
+  !$omp atomic hint(omp_sync_hint_contended) hint(omp_sync_hint_speculative) write
+    i = j
+  !ERROR: At most one HINT clause can appear on the WRITE directive
+  !$omp atomic hint(omp_sync_hint_nonspeculative) write hint(omp_sync_hint_nonspeculative)
+    i = j
+  !ERROR: At most one HINT clause can appear on the WRITE directive
+  !$omp atomic write hint(omp_sync_hint_none) hint (omp_sync_hint_uncontended)
+    i = j
+  !ERROR: At most one HINT clause can appear on the UPDATE directive
+  !$omp atomic hint(omp_sync_hint_contended) hint(omp_sync_hint_speculative) update
+    i = j
+  !ERROR: At most one HINT clause can appear on the UPDATE directive
+  !$omp atomic hint(omp_sync_hint_nonspeculative) update hint(omp_sync_hint_nonspeculative)
+    i = j
+  !ERROR: At most one HINT clause can appear on the UPDATE directive
+  !$omp atomic update hint(omp_sync_hint_none) hint (omp_sync_hint_uncontended)
+    i = j
+  !ERROR: At most one HINT clause can appear on the ATOMIC directive
+  !$omp atomic hint(omp_sync_hint_contended) hint(omp_sync_hint_speculative)
+    i = j
+  !ERROR: At most one HINT clause can appear on the ATOMIC directive
+  !$omp atomic hint(omp_sync_hint_none) hint(omp_sync_hint_nonspeculative)
+    i = j
+  !ERROR: At most one HINT clause can appear on the ATOMIC directive
+  !$omp atomic hint(omp_sync_hint_none) hint (omp_sync_hint_uncontended)
+    i = j
+
+  !ERROR: At most one HINT clause can appear on the CAPTURE directive
+  !$omp atomic hint(omp_sync_hint_contended) hint(omp_sync_hint_speculative) capture
+    i = j
+    j = k
+  !$omp end atomic
+  !ERROR: At most one HINT clause can appear on the CAPTURE directive
+  !$omp atomic hint(omp_sync_hint_nonspeculative) capture hint(omp_sync_hint_nonspeculative)
+    i = j
+    j = k
+  !$omp end atomic
+  !ERROR: At most one HINT clause can appear on the CAPTURE directive
+  !$omp atomic capture hint(omp_sync_hint_none) hint (omp_sync_hint_uncontended)
+    i = j
+    j = k
+  !$omp end atomic
+! 2.17.7.4
+! If atomic-clause is read then memory-order-clause must not be acq_rel or release.
+
+  !ERROR: Clause ACQ_REL is not allowed if clause READ appears on the ATOMIC directive
+  !$omp atomic acq_rel read
+    i = j
+  !ERROR: Clause ACQ_REL is not allowed if clause READ appears on the ATOMIC directive
+  !$omp atomic read acq_rel
+    i = j
+
+  !ERROR: Clause RELEASE is not allowed if clause READ appears on the ATOMIC directive
+  !$omp atomic release read
+    i = j
+  !ERROR: Clause RELEASE is not allowed if clause READ appears on the ATOMIC directive
+  !$omp atomic read release
+    i = j
+
+! 2.17.7.5
+! If atomic-clause is write then memory-order-clause must not be acq_rel or acquire.
+
+  !ERROR: Clause ACQ_REL is not allowed if clause WRITE appears on the ATOMIC directive
+  !$omp atomic acq_rel write
+    i = j
+  !ERROR: Clause ACQ_REL is not allowed if clause WRITE appears on the ATOMIC directive
+  !$omp atomic write acq_rel
+    i = j
+
+  !ERROR: Clause ACQUIRE is not allowed if clause WRITE appears on the ATOMIC directive
+  !$omp atomic acquire write
+    i = j
+  !ERROR: Clause ACQUIRE is not allowed if clause WRITE appears on the ATOMIC directive
+  !$omp atomic write acquire
+    i = j
+
+
+! 2.17.7.6
+! If atomic-clause is update or not present then memory-order-clause must not be acq_rel or acquire.
+
+  !ERROR: Clause ACQ_REL is not allowed if clause UPDATE appears on the ATOMIC directive
+  !$omp atomic acq_rel update
+    i = j
+  !ERROR: Clause ACQ_REL is not allowed if clause UPDATE appears on the ATOMIC directive
+  !$omp atomic update acq_rel
+    i = j
+
+  !ERROR: Clause ACQUIRE is not allowed if clause UPDATE appears on the ATOMIC directive
+  !$omp atomic acquire update
+    i = j
+
+  !ERROR: Clause ACQUIRE is not allowed if clause UPDATE appears on the ATOMIC directive
+  !$omp atomic update acquire
+    i = j
+
+  !ERROR: Clause ACQ_REL is not allowed on the ATOMIC directive
+  !$omp atomic acq_rel
+    i = j
+
+  !ERROR: Clause ACQUIRE is not allowed on the ATOMIC directive
+  !$omp atomic acquire
+    i = j
+end program
+

llvm/include/llvm/Frontend/OpenMP/OMP.td

def OMP_Ordered : Directive<"ordered"> {
   ];
 }
 def OMP_Atomic : Directive<"atomic"> {
   let allowedClauses = [
     VersionedClause<OMPC_Read>,
     VersionedClause<OMPC_Write>,
     VersionedClause<OMPC_Update>,
     VersionedClause<OMPC_Capture>,
+  ];
+  let allowedOnceClauses = [
     VersionedClause<OMPC_SeqCst>,
     VersionedClause<OMPC_AcqRel, 50>,
     VersionedClause<OMPC_Acquire, 50>,
     VersionedClause<OMPC_Release, 50>,
     VersionedClause<OMPC_Relaxed, 50>,
     VersionedClause<OMPC_Hint, 50>
   ];
 }