[flang][OpenMP] Enhance parser support for atomic construct to OpenMP 5.0

[kiranktp]

Summary

This patch enhances parser support for atomic construct to OpenMP 5.0.

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

The patch includes code changes and testcase modifications.

[DavidTruby]

Looks good to me! As a nit, perhaps you could add some tests that shouldn't parse correctly as well?

[kiranktp]

In D82931#2132486, @DavidTruby wrote:

Looks good to me! As a nit, perhaps you could add some tests that shouldn't parse correctly as well?

Thanks for reviewing the code David!.

As a nit, perhaps you could add some tests that shouldn't parse correctly as well?

Any syntax error will lead to chain of errors. It will be tough to add a case for invalid syntax.
I will check this once.

[sscalpone]

Any syntax error will lead to chain of errors. It will be tough to add a case for invalid syntax.

Please consider adding error recovery to prevent the cascade of errors. See the docs and the parser for examples.

[kiranktp]

In D82931#2134924, @sscalpone wrote:

Any syntax error will lead to chain of errors. It will be tough to add a case for invalid syntax.

Please consider adding error recovery to prevent the cascade of errors. See the docs and the parser for examples.

Yes Steve. Checking this.

[kiranktp]

Used Common openmp Directives and added a negative test case.

[kiranchandramohan]

Looks OK to me.

Two questions inline. Please add a few more tests covering all new memory-orders.

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

3631

Is the source field used by the parser? I didn't see a sourced parser for OmpAtomicMemoryOrderClause.

flang/test/Semantics/omp-atomic.f90

14

Can you use the omp_lib module and add tests for hints with the enum/kind value? like HINT(OMP_LOCK_HINT_CONTENDED)

[kiranktp]

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

3631

I will remove the source field as it is not used.

flang/test/Semantics/omp-atomic.f90

14

Sure. i will use omp_lib module and replace constants with Synchronization Hints.

[kiranktp]

Incorporated review comments.

[kiranchandramohan]

LGTM.

flang/docs/OpenMP-4.5-grammar.txt

 2.13.5 taskgroup -> TASKGROUP
 
 2.13.5 end-taskgroup -> END TASKGROUP
 
 2.13.6 atomic -> ATOMIC [seq_cst[,]] atomic-clause [[,]seq_cst] |
                  ATOMIC [seq_cst]
        atomic-clause -> READ | WRITE | UPDATE | CAPTURE
 
+2.13.6 end-atomic -> END ATOMIC
+
 2.13.7 flush -> FLUSH [(variable-name-list)]
 
 2.13.8 ordered -> ORDERED ordered-construct-clause [[[,] ordered-construct-clause]...]
        ordered-construct-clause -> depend-clause
 
 2.13.8 end-ordered -> END ORDERED
 
 2.14.1 cancel -> CANCEL construct-type-clause [ [,] if-clause]

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

#include "llvm/Frontend/OpenACC/ACC.cpp.inc"
   NODE(parser, NullInit)
   NODE(parser, ObjectDecl)
   NODE(parser, OldParameterStmt)
   NODE(parser, OmpAlignedClause)
   NODE(parser, OmpAtomic)
   NODE(parser, OmpAtomicCapture)
   NODE(OmpAtomicCapture, Stmt1)
   NODE(OmpAtomicCapture, Stmt2)
+  NODE(parser, OmpAtomicMemoryOrderClause)
+  NODE(parser, OmpAtomicMemoryOrderClauseList)
+  NODE(parser, OmpAtomicMemoryOrderClausePostList)
   NODE(parser, OmpAtomicRead)
   NODE(parser, OmpAtomicUpdate)
   NODE(parser, OmpAtomicWrite)
   NODE(parser, OmpBeginBlockDirective)
   NODE(parser, OmpBeginLoopDirective)
   NODE(parser, OmpBeginSectionsDirective)
   NODE(parser, OmpBlockDirective)
   static std::string GetNodeName(const llvm::omp::Directive &x) {
     return llvm::Twine(
         "llvm::omp::Directive = ", llvm::omp::getOpenMPDirectiveName(x))
         .str();
   }
   NODE(parser, OmpCancelType)
   NODE_ENUM(OmpCancelType, Type)
   NODE(parser, OmpClause)
 #define GEN_FLANG_DUMP_PARSE_TREE_CLAUSES
 #include "llvm/Frontend/OpenMP/OMP.cpp.inc"
   NODE(parser, OmpClauseList)
   NODE(parser, OmpCriticalDirective)
-  NODE(OmpCriticalDirective, Hint)
   NODE(parser, OmpDeclareTargetSpecifier)
   NODE(parser, OmpDeclareTargetWithClause)
   NODE(parser, OmpDeclareTargetWithList)
   NODE(parser, OmpDefaultClause)
   NODE_ENUM(OmpDefaultClause, Type)
   NODE(parser, OmpDefaultmapClause)
   NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
   NODE_ENUM(OmpDefaultmapClause, VariableCategory)
   NODE(parser, OmpDependClause)
   NODE(OmpDependClause, InOut)
   NODE(OmpDependClause, Sink)
   NODE(OmpDependClause, Source)
   NODE(parser, OmpDependenceType)
   NODE_ENUM(OmpDependenceType, Type)
   NODE(parser, OmpDependSinkVec)
   NODE(parser, OmpDependSinkVecLength)
   NODE(parser, OmpDistScheduleClause)
   NODE(parser, OmpEndAtomic)
   NODE(parser, OmpEndBlockDirective)
   NODE(parser, OmpEndCriticalDirective)
   NODE(parser, OmpEndLoopDirective)
   NODE(parser, OmpEndSectionsDirective)
+  NODE(parser, OmpHintExpr)
   NODE(parser, OmpIfClause)
   NODE_ENUM(OmpIfClause, DirectiveNameModifier)
   NODE(parser, OmpLinearClause)
   NODE(OmpLinearClause, WithModifier)
   NODE(OmpLinearClause, WithoutModifier)
   NODE(parser, OmpLinearModifier)
   NODE_ENUM(OmpLinearModifier, Type)
   NODE(parser, OmpLoopDirective)
   NODE(parser, OmpMapClause)
   NODE(parser, OmpMapType)
   NODE(OmpMapType, Always)
   NODE_ENUM(OmpMapType, Type)
-  NODE(parser, OmpMemoryClause)
-  NODE_ENUM(OmpMemoryClause, MemoryOrder)
-  NODE(parser, OmpMemoryClauseList)
-  NODE(parser, OmpMemoryClausePostList)
+  NODE(parser, OmpMemoryOrderClause)
+  static std::string GetNodeName(const llvm::omp::Clause &x) {
+    return llvm::Twine(
+        "llvm::omp::Clause = ", llvm::omp::getOpenMPClauseName(x))
+        .str();
+  }
   NODE(parser, OmpNowait)
   NODE(parser, OmpObject)
   NODE(parser, OmpObjectList)
   NODE(parser, OmpProcBindClause)
   NODE_ENUM(OmpProcBindClause, Type)
   NODE(parser, OmpReductionClause)
   NODE(parser, OmpReductionCombiner)
   NODE(OmpReductionCombiner, FunctionCombiner)
#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, OmpFlushMemoryClause)
-  NODE_ENUM(OmpFlushMemoryClause, FlushMemoryOrder)
   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 OpenMPDeclarativeConstruct {
   UNION_CLASS_BOILERPLATE(OpenMPDeclarativeConstruct);
   CharBlock source;
   std::variant<OpenMPDeclareReductionConstruct, OpenMPDeclareSimdConstruct,
       OpenMPDeclareTargetConstruct, OpenMPThreadprivate>
       u;
 };
 
+// HINT(hint-expression)
+WRAPPER_CLASS(OmpHintExpr, ConstantExpr);
+
 // 2.13.2 CRITICAL [Name] <block> END CRITICAL [Name]
 struct OmpCriticalDirective {
   TUPLE_CLASS_BOILERPLATE(OmpCriticalDirective);
-  WRAPPER_CLASS(Hint, ConstantExpr);
   CharBlock source;
-  std::tuple<Verbatim, std::optional<Name>, std::optional<Hint>> t;
+  std::tuple<Verbatim, std::optional<Name>, std::optional<OmpHintExpr>> t;
 };
 struct OmpEndCriticalDirective {
   TUPLE_CLASS_BOILERPLATE(OmpEndCriticalDirective);
   CharBlock source;
   std::tuple<Verbatim, std::optional<Name>> t;
 };
 struct OpenMPCriticalConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
   std::tuple<OmpCriticalDirective, Block, OmpEndCriticalDirective> t;
 };
 
-// 2.13.6 atomic -> ATOMIC [seq_cst[,]] atomic-clause [[,]seq_cst] |
-//                  ATOMIC [seq_cst]
+// 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
 
 // END ATOMIC
 EMPTY_CLASS(OmpEndAtomic);
 
-// ATOMIC Memory related clause
-struct OmpMemoryClause {
-  ENUM_CLASS(MemoryOrder, SeqCst)
-  WRAPPER_CLASS_BOILERPLATE(OmpMemoryClause, MemoryOrder);
+// Memory order clause
+struct OmpMemoryOrderClause {
+  WRAPPER_CLASS_BOILERPLATE(OmpMemoryOrderClause, llvm::omp::Clause);
   CharBlock source;
 };
 
-WRAPPER_CLASS(OmpMemoryClauseList, std::list<OmpMemoryClause>);
-WRAPPER_CLASS(OmpMemoryClausePostList, std::list<OmpMemoryClause>);
+// ATOMIC Memory order clause or hint expression
+struct OmpAtomicMemoryOrderClause {
+  UNION_CLASS_BOILERPLATE(OmpAtomicMemoryOrderClause);
+  std::variant<OmpMemoryOrderClause, OmpHintExpr> u;
+};
+
+WRAPPER_CLASS(
+    OmpAtomicMemoryOrderClauseList, std::list<OmpAtomicMemoryOrderClause>);
+WRAPPER_CLASS(
+    OmpAtomicMemoryOrderClausePostList, std::list<OmpAtomicMemoryOrderClause>);
 
 // ATOMIC READ
 struct OmpAtomicRead {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicRead);
-  std::tuple<OmpMemoryClauseList, Verbatim, OmpMemoryClausePostList,
-      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
+  std::tuple<OmpAtomicMemoryOrderClauseList, Verbatim,
+      OmpAtomicMemoryOrderClausePostList, Statement<AssignmentStmt>,
+      std::optional<OmpEndAtomic>>
       t;
 };
 
 // ATOMIC WRITE
 struct OmpAtomicWrite {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicWrite);
-  std::tuple<OmpMemoryClauseList, Verbatim, OmpMemoryClausePostList,
-      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
+  std::tuple<OmpAtomicMemoryOrderClauseList, Verbatim,
+      OmpAtomicMemoryOrderClausePostList, Statement<AssignmentStmt>,
+      std::optional<OmpEndAtomic>>
       t;
 };
 
 // ATOMIC UPDATE
 struct OmpAtomicUpdate {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicUpdate);
-  std::tuple<OmpMemoryClauseList, Verbatim, OmpMemoryClausePostList,
-      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
+  std::tuple<OmpAtomicMemoryOrderClauseList, Verbatim,
+      OmpAtomicMemoryOrderClausePostList, Statement<AssignmentStmt>,
+      std::optional<OmpEndAtomic>>
       t;
 };
 
 // ATOMIC CAPTURE
 struct OmpAtomicCapture {
   TUPLE_CLASS_BOILERPLATE(OmpAtomicCapture);
   WRAPPER_CLASS(Stmt1, Statement<AssignmentStmt>);
   WRAPPER_CLASS(Stmt2, Statement<AssignmentStmt>);
-  std::tuple<OmpMemoryClauseList, Verbatim, OmpMemoryClausePostList, Stmt1,
-      Stmt2, OmpEndAtomic>
+  std::tuple<OmpAtomicMemoryOrderClauseList, Verbatim,
+      OmpAtomicMemoryOrderClausePostList, Stmt1, Stmt2, OmpEndAtomic>
       t;
 };
 
 // ATOMIC
 struct OmpAtomic {
   TUPLE_CLASS_BOILERPLATE(OmpAtomic);
-  std::tuple<Verbatim, OmpMemoryClauseList, Statement<AssignmentStmt>,
-      std::optional<OmpEndAtomic>>
+  std::tuple<Verbatim, OmpAtomicMemoryOrderClauseList,
+      Statement<AssignmentStmt>, std::optional<OmpEndAtomic>>
       t;
 };
 
 struct OpenMPAtomicConstruct {
   UNION_CLASS_BOILERPLATE(OpenMPAtomicConstruct);
   std::variant<OmpAtomicRead, OmpAtomicWrite, OmpAtomicCapture, OmpAtomicUpdate,
       OmpAtomic>
       u;
struct OpenMPCancelConstruct {
   std::tuple<Verbatim, OmpCancelType, std::optional<If>> t;
 };
 
 // 2.17.8 Flush Construct [OpenMP 5.0]
 // memory-order-clause -> acq_rel
 //                        release
 //                        acquire
 struct OmpFlushMemoryClause {
-  ENUM_CLASS(FlushMemoryOrder, AcqRel, Release, Acquire)
-  WRAPPER_CLASS_BOILERPLATE(OmpFlushMemoryClause, FlushMemoryOrder);
+  WRAPPER_CLASS_BOILERPLATE(OmpFlushMemoryClause, llvm::omp::Clause);
   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<OmpFlushMemoryClause>,

flang/lib/Parser/openmp-parsers.cpp

TYPE_PARSER(sourced(construct<OpenMPCancelConstruct>(verbatim("CANCEL"_tok),
     Parser<OmpCancelType>{}, maybe("IF" >> parenthesized(scalarLogicalExpr)))))
 
 // 2.17.8 Flush construct [OpenMP 5.0]
 //        flush -> FLUSH [memory-order-clause] [(variable-name-list)]
 //        memory-order-clause -> acq_rel
 //                               release
 //                               acquire
 TYPE_PARSER(sourced(construct<OmpFlushMemoryClause>(
-    "ACQ_REL" >> pure(OmpFlushMemoryClause::FlushMemoryOrder::AcqRel) ||
-    "RELEASE" >> pure(OmpFlushMemoryClause::FlushMemoryOrder::Release) ||
-    "ACQUIRE" >> pure(OmpFlushMemoryClause::FlushMemoryOrder::Acquire))))
+    "ACQ_REL" >> pure(llvm::omp::Clause::OMPC_acq_rel) ||
+    "RELEASE" >> pure(llvm::omp::Clause::OMPC_release) ||
+    "ACQUIRE" >> pure(llvm::omp::Clause::OMPC_acquire))))
 
 TYPE_PARSER(sourced(construct<OpenMPFlushConstruct>(verbatim("FLUSH"_tok),
     maybe(Parser<OmpFlushMemoryClause>{}),
     maybe(parenthesized(Parser<OmpObjectList>{})))))
 
 // Simple Standalone Directives
 TYPE_PARSER(sourced(construct<OmpSimpleStandaloneDirective>(first(
     "BARRIER" >> pure(llvm::omp::Directive::OMPD_barrier),
TYPE_PARSER(sourced(construct<OpenMPDeclareTargetConstruct>(
     verbatim("DECLARE TARGET"_tok), Parser<OmpDeclareTargetSpecifier>{})))
 
 TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
     construct<OmpReductionCombiner>(
         construct<OmpReductionCombiner::FunctionCombiner>(
             construct<Call>(Parser<ProcedureDesignator>{},
                 parenthesized(optionalList(actualArgSpec))))))
 
-// 2.13.6 ATOMIC [seq_cst[,]] atomic-clause [[,]seq_cst] | ATOMIC [seq_cst]
-//        atomic-clause -> READ | WRITE | UPDATE | CAPTURE
+// Hint Expression => HINT(hint-expression)
+TYPE_PARSER("HINT" >> construct<OmpHintExpr>(parenthesized(constantExpr)))
+
+// 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
 
 // OMP END ATOMIC
 TYPE_PARSER(construct<OmpEndAtomic>(startOmpLine >> "END ATOMIC"_tok))
 
-// ATOMIC Memory related clause
-TYPE_PARSER(sourced(construct<OmpMemoryClause>(
-    "SEQ_CST" >> pure(OmpMemoryClause::MemoryOrder::SeqCst))))
-
-// ATOMIC Memory Clause List
-TYPE_PARSER(construct<OmpMemoryClauseList>(
-    many(maybe(","_tok) >> Parser<OmpMemoryClause>{})))
+// Memory order clause
+TYPE_PARSER(sourced(construct<OmpMemoryOrderClause>(
+    "SEQ_CST" >> pure(llvm::omp::Clause::OMPC_seq_cst) ||
+    "ACQ_REL" >> pure(llvm::omp::Clause::OMPC_acq_rel) ||
+    "RELEASE" >> pure(llvm::omp::Clause::OMPC_release) ||
+    "ACQUIRE" >> pure(llvm::omp::Clause::OMPC_acquire) ||
+    "RELAXED" >> pure(llvm::omp::Clause::OMPC_relaxed))))
+
+// ATOMIC Memory order clause or Hint expression
+TYPE_PARSER(
+    construct<OmpAtomicMemoryOrderClause>(Parser<OmpMemoryOrderClause>{}) ||
+    construct<OmpAtomicMemoryOrderClause>(Parser<OmpHintExpr>{}))
+
+// ATOMIC Memory order Clause List
+TYPE_PARSER(construct<OmpAtomicMemoryOrderClauseList>(
+    many(maybe(","_tok) >> Parser<OmpAtomicMemoryOrderClause>{})))
 
-TYPE_PARSER(construct<OmpMemoryClausePostList>(
-    many(maybe(","_tok) >> Parser<OmpMemoryClause>{})))
+TYPE_PARSER(construct<OmpAtomicMemoryOrderClausePostList>(
+    many(maybe(","_tok) >> Parser<OmpAtomicMemoryOrderClause>{})))
 
-// OMP [SEQ_CST] ATOMIC READ [SEQ_CST]
+// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] READ [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicRead>(Parser<OmpMemoryClauseList>{} / maybe(","_tok),
-        verbatim("READ"_tok), Parser<OmpMemoryClausePostList>{} / endOmpLine,
+    construct<OmpAtomicRead>(
+        Parser<OmpAtomicMemoryOrderClauseList>{} / maybe(","_tok),
+        verbatim("READ"_tok),
+        Parser<OmpAtomicMemoryOrderClausePostList>{} / endOmpLine,
         statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
-// OMP ATOMIC [SEQ_CST] CAPTURE [SEQ_CST]
-TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicCapture>(Parser<OmpMemoryClauseList>{} / maybe(","_tok),
-        verbatim("CAPTURE"_tok), Parser<OmpMemoryClausePostList>{} / endOmpLine,
+// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] CAPTURE [MEMORY-ORDER-CLAUSE-LIST]
+TYPE_PARSER(
+    "ATOMIC" >> construct<OmpAtomicCapture>(
+                    Parser<OmpAtomicMemoryOrderClauseList>{} / maybe(","_tok),
+                    verbatim("CAPTURE"_tok),
+                    Parser<OmpAtomicMemoryOrderClausePostList>{} / endOmpLine,
                     statement(assignmentStmt), statement(assignmentStmt),
                     Parser<OmpEndAtomic>{} / endOmpLine))
 
-// OMP ATOMIC [SEQ_CST] UPDATE [SEQ_CST]
+// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] UPDATE [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicUpdate>(Parser<OmpMemoryClauseList>{} / maybe(","_tok),
-        verbatim("UPDATE"_tok), Parser<OmpMemoryClausePostList>{} / endOmpLine,
+    construct<OmpAtomicUpdate>(
+        Parser<OmpAtomicMemoryOrderClauseList>{} / maybe(","_tok),
+        verbatim("UPDATE"_tok),
+        Parser<OmpAtomicMemoryOrderClausePostList>{} / endOmpLine,
         statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
-// OMP ATOMIC [SEQ_CST]
+// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER(construct<OmpAtomic>(verbatim("ATOMIC"_tok),
-    Parser<OmpMemoryClauseList>{} / endOmpLine, statement(assignmentStmt),
-    maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
+    Parser<OmpAtomicMemoryOrderClauseList>{} / endOmpLine,
+    statement(assignmentStmt), maybe(Parser<OmpEndAtomic>{} / endOmpLine)))
 
-// ATOMIC [SEQ_CST] WRITE [SEQ_CST]
+// OMP ATOMIC [MEMORY-ORDER-CLAUSE-LIST] WRITE [MEMORY-ORDER-CLAUSE-LIST]
 TYPE_PARSER("ATOMIC" >>
-    construct<OmpAtomicWrite>(Parser<OmpMemoryClauseList>{} / maybe(","_tok),
-        verbatim("WRITE"_tok), Parser<OmpMemoryClausePostList>{} / endOmpLine,
+    construct<OmpAtomicWrite>(
+        Parser<OmpAtomicMemoryOrderClauseList>{} / maybe(","_tok),
+        verbatim("WRITE"_tok),
+        Parser<OmpAtomicMemoryOrderClausePostList>{} / 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>{}))
 
 // 2.13.2 OMP CRITICAL
 TYPE_PARSER(startOmpLine >>
     sourced(construct<OmpEndCriticalDirective>(
         verbatim("END CRITICAL"_tok), maybe(parenthesized(name)))) /
         endOmpLine)
 TYPE_PARSER(sourced(construct<OmpCriticalDirective>(verbatim("CRITICAL"_tok),
-                maybe(parenthesized(name)),
-                maybe("HINT" >> construct<OmpCriticalDirective::Hint>(
-                                    parenthesized(constantExpr))))) /
+                maybe(parenthesized(name)), maybe(Parser<OmpHintExpr>{}))) /
     endOmpLine)
 
 TYPE_PARSER(construct<OpenMPCriticalConstruct>(
     Parser<OmpCriticalDirective>{}, block, Parser<OmpEndCriticalDirective>{}))
 
 // 2.8.2 Declare Simd construct
 TYPE_PARSER(
     sourced(construct<OpenMPDeclareSimdConstruct>(verbatim("DECLARE SIMD"_tok),

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 OmpMemoryClause &x) {
+  void Unparse(const OmpHintExpr &x) { Word("HINT("), Walk(x.v), Put(')'); }
+  void Unparse(const OmpMemoryOrderClause &x) {
     switch (x.v) {
-    case OmpMemoryClause::MemoryOrder::SeqCst:
+    case llvm::omp::Clause::OMPC_seq_cst:
       Word("SEQ_CST");
       break;
+    case llvm::omp::Clause::OMPC_acq_rel:
+      Word("ACQ_REL");
+      break;
+    case llvm::omp::Clause::OMPC_release:
+      Word("RELEASE");
+      break;
+    case llvm::omp::Clause::OMPC_acquire:
+      Word("ACQUIRE");
+      break;
+    case llvm::omp::Clause::OMPC_relaxed:
+      Word("RELAXED");
+      break;
+    default:
+      break;
     }
   }
-  void Unparse(const OmpMemoryClauseList &x) { Walk(" ", x.v, " "); }
-  void Unparse(const OmpMemoryClausePostList &x) { Walk(" ", x.v, " "); }
+  void Unparse(const OmpAtomicMemoryOrderClauseList &x) { Walk(" ", x.v, " "); }
+  void Unparse(const OmpAtomicMemoryOrderClausePostList &x) {
+    Walk(" ", x.v, " ");
+  }
   void Unparse(const OmpAtomic &x) {
     BeginOpenMP();
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpMemoryClauseList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClauseList>(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) {
     BeginOpenMP();
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpMemoryClauseList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClauseList>(x.t));
     Word(" CAPTURE");
-    Walk(std::get<OmpMemoryClausePostList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClausePostList>(x.t));
     Put("\n");
     EndOpenMP();
     Walk(std::get<OmpAtomicCapture::Stmt1>(x.t));
     Put("\n");
     Walk(std::get<OmpAtomicCapture::Stmt2>(x.t));
     BeginOpenMP();
     Word("!$OMP END ATOMIC\n");
     EndOpenMP();
   }
   void Unparse(const OmpAtomicRead &x) {
     BeginOpenMP();
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpMemoryClauseList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClauseList>(x.t));
     Word(" READ");
-    Walk(std::get<OmpMemoryClausePostList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClausePostList>(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 OmpAtomicUpdate &x) {
     BeginOpenMP();
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpMemoryClauseList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClauseList>(x.t));
     Word(" UPDATE");
-    Walk(std::get<OmpMemoryClausePostList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClausePostList>(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 OmpAtomicWrite &x) {
     BeginOpenMP();
     Word("!$OMP ATOMIC");
-    Walk(std::get<OmpMemoryClauseList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClauseList>(x.t));
     Word(" WRITE");
-    Walk(std::get<OmpMemoryClausePostList>(x.t));
+    Walk(std::get<OmpAtomicMemoryOrderClausePostList>(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 OmpCriticalDirective &x) {
     BeginOpenMP();
     Word("!$OMP CRITICAL");
     Walk(" (", std::get<std::optional<Name>>(x.t), ")");
-    Walk(" HINT(", std::get<std::optional<OmpCriticalDirective::Hint>>(x.t),
-        ")");
+    Walk(std::get<std::optional<OmpHintExpr>>(x.t));
     Put("\n");
     EndOpenMP();
   }
   void Unparse(const OmpEndCriticalDirective &x) {
     BeginOpenMP();
     Word("!$OMP END CRITICAL");
     Walk(" (", std::get<std::optional<Name>>(x.t), ")");
     Put("\n");
void Unparse(const OpenMPCancelConstruct &x) {
     Word("!$OMP CANCEL ");
     Walk(std::get<OmpCancelType>(x.t));
     Walk(std::get<std::optional<OpenMPCancelConstruct::If>>(x.t));
     Put("\n");
     EndOpenMP();
   }
   void Unparse(const OmpFlushMemoryClause &x) {
     switch (x.v) {
-    case OmpFlushMemoryClause::FlushMemoryOrder::AcqRel:
+    case llvm::omp::Clause::OMPC_acq_rel:
       Word("ACQ_REL ");
       break;
-    case OmpFlushMemoryClause::FlushMemoryOrder::Release:
+    case llvm::omp::Clause::OMPC_release:
       Word("RELEASE ");
       break;
-    case OmpFlushMemoryClause::FlushMemoryOrder::Acquire:
+    case llvm::omp::Clause::OMPC_acquire:
       Word("ACQUIRE ");
       break;
+    default:
+      break;
     }
   }
   void Unparse(const OpenMPFlushConstruct &x) {
     BeginOpenMP();
     Word("!$OMP FLUSH ");
     Walk(std::get<std::optional<OmpFlushMemoryClause>>(x.t));
     Walk(" (", std::get<std::optional<OmpObjectList>>(x.t), ")");
     Put("\n");

flang/test/Semantics/omp-atomic.f90

 ! RUN: %S/test_errors.sh %s %t %f18 -fopenmp
-
+use omp_lib
 ! Check OpenMP 2.13.6 atomic Construct
 
   a = 1.0
   !$omp parallel num_threads(4)
   !$omp atomic seq_cst, read
   b = a
 
   !$omp atomic seq_cst write
   a = b
   !$omp end atomic
 
+  !$omp atomic read acquire hint(OMP_LOCK_HINT_CONTENDED)
+  a = b
+
+  !$omp atomic release hint(OMP_LOCK_HINT_UNCONTENDED) write
+  a = b
+
   !$omp atomic capture seq_cst
   b = a
   a = a + 1
   !$omp end atomic
 
+  !$omp atomic hint(1) acq_rel capture
+  b = a
+  a = a + 1
+  !$omp end atomic
+
+  !ERROR: expected end of line
+  !ERROR: expected end of line
+  !$omp atomic read write
+  a = a + 1
+
   !$omp atomic
   a = a + 1
+
+  !$omp atomic relaxed
+  a = a + 1
+
   !$omp end parallel
 end