[flang-commits] [PATCH] D89879: [Flang][OpenMP-5.0] Semantic checks for flush construct.

[Valentin Clement via Phabricator via flang-commits]

clementval requested changes to this revision.
clementval added inline comments.
This revision now requires changes to proceed.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:601
+  case llvm::omp::Clause::OMPC_acquire:
+    CheckAllowed(llvm::omp::Clause::OMPC_acquire);
+    break;
----------------
It makes little sense to use the `OmpClause` infrastructure to check something enforced by the parser.


================
Comment at: flang/test/Semantics/omp-flush.f90:36
+    !ERROR: If memory-order-clause is RELEASE, ACQUIRE, or ACQ_REL, list items must not be specified on the FLUSH directive
+    !$omp flush ACQ_REL (array)
+    !ERROR: If memory-order-clause is RELEASE, ACQUIRE, or ACQ_REL, list items must not be specified on the FLUSH directive
----------------
Any reason to have upper and lower cases? 

Same questions for the others below


================
Comment at: flang/test/Semantics/omp-flush01.f90:16
+    ! Allowed clauses.
+    !$omp flush ACQ_REL
+    array = (/1, 2, 3, 4, 5, 6, 7, 8, 9, 10/)
----------------
Any reason to have upper and lower cases? 

Same questions for the others below


================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMP.td:423
 def OMP_Flush : Directive<"flush"> {
-  let allowedClauses = [
+  let allowedOnceClauses = [
     VersionedClause<OMPC_AcqRel, 50>,
----------------
Since you are not using the `OmpClause` this change is not needed here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89879