[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

4

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

4

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?

515

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.

519

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.

515

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

515

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

515

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.

515

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 ↗

(On Diff #308012)

Is the comment at the right place?

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

538

Where do you check that it is only for update?

[sameeranjoshi]

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

3634 ↗

(On Diff #308012)

No, thx.

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

538

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

538

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

538

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

538

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/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 &);
+  // Atomic-clause
+  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 &);

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/Common/Fortran.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/tools.h"
 
 namespace Fortran::semantics {
 
 bool OmpStructureChecker::HasInvalidWorksharingNesting(
     const parser::CharBlock &source, const OmpDirectiveSet &set) {
   // set contains all the invalid closely nested directives
PushContextAndClauseSets(
       dir.source, llvm::omp::Directive::OMPD_cancellation_point);
 }
 
 void OmpStructureChecker::Leave(
     const parser::OpenMPCancellationPointConstruct &) {
   dirContext_.pop_back();
 }
 
+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();
+}
 void OmpStructureChecker::Enter(const parser::OmpEndBlockDirective &x) {
   const auto &dir{std::get<parser::OmpBlockDirective>(x.t)};
   ResetPartialContext(dir.source);
   switch (dir.v) {
   // 2.7.3 end-single-clause -> copyprivate-clause |
   //                            nowait-clause
   case llvm::omp::Directive::OMPD_single: {
     SetContextDirectiveEnum(llvm::omp::Directive::OMPD_end_single);
void OmpStructureChecker::Enter(const parser::OmpClause::Uniform &) {
   CheckAllowed(llvm::omp::Clause::OMPC_uniform);
 }
 void OmpStructureChecker::Enter(const parser::OmpClause::UseDevicePtr &) {
   CheckAllowed(llvm::omp::Clause::OMPC_use_device_ptr);
 }
 void OmpStructureChecker::Enter(const parser::OmpClause::IsDevicePtr &) {
   CheckAllowed(llvm::omp::Clause::OMPC_is_device_ptr);
 }
+// Memory-order-clause
+void OmpStructureChecker::Enter(const parser::OmpClause::SeqCst &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_seq_cst);
+}
+void OmpStructureChecker::Enter(const parser::OmpClause::AcqRel &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_acq_rel);
+}
+void OmpStructureChecker::Enter(const parser::OmpClause::Release &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_release);
+}
+void OmpStructureChecker::Enter(const parser::OmpClause::Acquire &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_acquire);
+}
+void OmpStructureChecker::Enter(const parser::OmpClause::Relaxed &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_relaxed);
+}
+
+// Atomic-clause
+void OmpStructureChecker::Enter(const parser::OmpAtomicRead &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_read);
+}
+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::Enter(const parser::OmpAtomicWrite &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_write);
+}
+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::Enter(const parser::OmpAtomicUpdate &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_update);
+}
+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::Enter(const parser::OmpAtomicCapture &) {
+  CheckAllowed(llvm::omp::Clause::OMPC_capture);
+}
+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);
+  }
+}
 
 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)}) {
     if (const auto v{GetIntValue(*expr)}) {
       if (*v <= 0) {
         context_.Say(GetContext().clauseSource,
             "The ALIGNMENT parameter of the ALIGNED clause must be "
             "a constant positive integer expression"_err_en_US);
       }
     }
   }
   // 2.8.1 TODO: list-item attribute check
 }
 void OmpStructureChecker::Enter(const parser::OmpAllocateClause &) {
   CheckAllowed(llvm::omp::Clause::OMPC_allocate);
 }
 void OmpStructureChecker::Enter(const parser::OmpDefaultClause &) {
   CheckAllowed(llvm::omp::Clause::OMPC_default);
 }
 void OmpStructureChecker::Enter(const parser::OmpDefaultmapClause &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_defaultmap);
   using VariableCategory = parser::OmpDefaultmapClause::VariableCategory;
   if (!std::get<std::optional<VariableCategory>>(x.t)) {
     context_.Say(GetContext().clauseSource,
         "The argument TOFROM:SCALAR must be specified on the DEFAULTMAP "

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.
+
+  !TODO: At most one hint clause may appear on the construct.
+use omp_lib
+  implicit none
+  integer :: i, j = 10, k=-100, a
+
+! 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
+! TODO: Hint is currently not found in `actualClauses`.
+! At most one hint clause may appear on the construct.
+  !$omp atomic hint(10) hint(10) read
+    i = j
+  !$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>,
-    VersionedClause<OMPC_SeqCst>,
+  ];
+  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>
   ];
 }
 def OMP_Target : Directive<"target"> {