[flang] [OpenMP] Add Sections and Single Construct check (flang-compiler/f18#585)

* [OpenMP] Add Sections and Single Construct check

Parse tree for OmpEndSingle needs to be modified to save the provenance
of END SINGLE directive and check its own clauses

* Update on reviews

1. PushContext is created to push new context with source provenance

2. Tweak the logic for SECTION nesting, treak Orphaned or wrong nesting
   as the same error type

3. Make sure the check for NOWAIT clause only applies to the ones that
   are not handled by parser.
   Note that the case for DO or DO_SIMD will take effect after the
   loop association work (parse tree change) is done. But I still list
   them there for completeness.

4. Happen to find that NOWAIT is not accepted by PARALLEL SECTIONS,
   fixed it in the parser.

Original-commit: flang-compiler/f18@236cf1efea77b8af5c533aa55029ce49a2c2e72c
Reviewed-on: https://github.com/flang-compiler/f18/pull/585

diff --git a/flang/lib/parser/openmp-grammar.h b/flang/lib/parser/openmp-grammar.h
index f34d106..ef9a948 100644 (file)
--- a/flang/lib/parser/openmp-grammar.h
+++ b/flang/lib/parser/openmp-grammar.h
@@ -463,8 +463,9 @@ TYPE_PARSER(construct<OpenMPStandaloneConstruct>(
     Parser<OmpStandaloneDirective>{}, Parser<OmpClauseList>{} / endOmpLine))
 
 // OMP SINGLE
-TYPE_PARSER(startOmpLine >> "END"_tok >>
-    construct<OmpEndSingle>("SINGLE" >> Parser<OmpClauseList>{}) / endOmpLine)
+TYPE_PARSER(startOmpLine >> construct<OmpEndSingle>(verbatim("END SINGLE"_tok),
+                                Parser<OmpClauseList>{}) /
+        endOmpLine)
 
 TYPE_PARSER(construct<OpenMPSingleConstruct>(verbatim("SINGLE"_tok),
     Parser<OmpClauseList>{} / endOmpLine, block, Parser<OmpEndSingle>{}))
@@ -490,10 +491,9 @@ TYPE_PARSER(startOmpLine >> "END SECTIONS"_tok >>
 TYPE_PARSER(construct<OpenMPSectionsConstruct>(verbatim("SECTIONS"_tok),
     Parser<OmpClauseList>{} / endOmpLine, block, Parser<OmpEndSections>{}))
 
-// OMP END PARALLEL SECTIONS [NOWAIT]
-TYPE_PARSER(startOmpLine >> "END PARALLEL SECTIONS"_tok >>
-    construct<OmpEndParallelSections>(
-        maybe("NOWAIT" >> construct<OmpNowait>()) / endOmpLine))
+// OMP END PARALLEL SECTIONS
+TYPE_PARSER(construct<OmpEndParallelSections>(
+    startOmpLine >> "END PARALLEL SECTIONS"_tok / endOmpLine))
 
 // OMP PARALLEL SECTIONS
 TYPE_PARSER(construct<OpenMPParallelSectionsConstruct>(

diff --git a/flang/lib/parser/parse-tree.h b/flang/lib/parser/parse-tree.h
index 5a2ece9..a5daf7a 100644 (file)
--- a/flang/lib/parser/parse-tree.h
+++ b/flang/lib/parser/parse-tree.h
@@ -3470,14 +3470,13 @@ struct OmpClauseList {
 
 // SECTIONS, PARALLEL SECTIONS
 WRAPPER_CLASS(OmpEndSections, std::optional<OmpNowait>);
-WRAPPER_CLASS(OmpEndParallelSections, std::optional<OmpNowait>);
 WRAPPER_CLASS(OmpSection, Verbatim);
-
 struct OpenMPSectionsConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPSectionsConstruct);
   std::tuple<Verbatim, OmpClauseList, Block, OmpEndSections> t;
 };
 
+EMPTY_CLASS(OmpEndParallelSections);
 struct OpenMPParallelSectionsConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPParallelSectionsConstruct);
   std::tuple<Verbatim, OmpClauseList, Block, OmpEndParallelSections> t;
@@ -3490,7 +3489,10 @@ struct OpenMPWorkshareConstruct {
 };
 
 // SINGLE
-WRAPPER_CLASS(OmpEndSingle, OmpClauseList);
+struct OmpEndSingle {
+  TUPLE_CLASS_BOILERPLATE(OmpEndSingle);
+  std::tuple<Verbatim, OmpClauseList> t;
+};
 struct OpenMPSingleConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPSingleConstruct);
   std::tuple<Verbatim, OmpClauseList, Block, OmpEndSingle> t;

diff --git a/flang/lib/semantics/check-omp-structure.cc b/flang/lib/semantics/check-omp-structure.cc
index fe058b1..8724183 100644 (file)
--- a/flang/lib/semantics/check-omp-structure.cc
+++ b/flang/lib/semantics/check-omp-structure.cc
@@ -80,8 +80,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
   }
 
   // push a context even in the error case
-  OmpContext ct{dir.source};
-  ompContext_.push_back(ct);
+  PushContext(dir.source);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
@@ -90,14 +89,70 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
 
 void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   const auto &dir{std::get<parser::OmpBlockDirective>(x.t)};
-  OmpContext ct{dir.source};
-  ompContext_.push_back(ct);
+  PushContext(dir.source);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {
   ompContext_.pop_back();
 }
 
+void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
+  const auto &dir{std::get<parser::Verbatim>(x.t)};
+  PushContext(dir.source, OmpDirective::SECTIONS);
+  OmpClauseSet allowed{OmpClause::PRIVATE, OmpClause::FIRSTPRIVATE,
+      OmpClause::LASTPRIVATE, OmpClause::REDUCTION};
+  SetContextAllowed(allowed);
+}
+
+void OmpStructureChecker::Leave(const parser::OpenMPSectionsConstruct &x) {
+  ompContext_.pop_back();
+}
+
+void OmpStructureChecker::Enter(const parser::OmpSection &x) {
+  const auto &dir{x.v};  // Verbatim
+  if (!CurrentDirectiveIsNested() ||
+      GetContext().directive != OmpDirective::SECTIONS) {
+    // if not currently nested, SECTION is orphaned
+    context_.Say(dir.source,
+        "SECTION directive must appear within "
+        "the SECTIONS construct"_err_en_US);
+  }
+}
+
+void OmpStructureChecker::Enter(const parser::OpenMPSingleConstruct &x) {
+  const auto &dir{std::get<parser::Verbatim>(x.t)};
+  PushContext(dir.source, OmpDirective::SINGLE);
+  OmpClauseSet allowed{OmpClause::PRIVATE, OmpClause::FIRSTPRIVATE};
+  SetContextAllowed(allowed);
+}
+
+void OmpStructureChecker::Leave(const parser::OpenMPSingleConstruct &x) {
+  ompContext_.pop_back();
+}
+
+void OmpStructureChecker::Enter(const parser::OmpEndSingle &x) {
+  // EndSingle is in SingleConstruct context
+  const auto &dir{std::get<parser::Verbatim>(x.t)};
+  PushContext(dir.source, OmpDirective::END_SINGLE);
+  OmpClauseSet allowed{OmpClause::COPYPRIVATE};
+  SetContextAllowed(allowed);
+  OmpClauseSet allowedOnce{OmpClause::NOWAIT};
+  SetContextAllowedOnce(allowedOnce);
+}
+
+void OmpStructureChecker::Leave(const parser::OmpEndSingle &x) {
+  ompContext_.pop_back();
+}
+
+void OmpStructureChecker::Enter(const parser::OpenMPWorkshareConstruct &x) {
+  const auto &dir{std::get<parser::Verbatim>(x.t)};
+  PushContext(dir.source, OmpDirective::WORKSHARE);
+}
+
+void OmpStructureChecker::Leave(const parser::OpenMPWorkshareConstruct &x) {
+  ompContext_.pop_back();
+}
+
 void OmpStructureChecker::Enter(const parser::OmpBlockDirective &x) {
   switch (x.v) {
   // 2.5 parallel-clause -> if-clause |
@@ -291,12 +346,32 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
 
     // TODO: A list-item cannot appear in more than one aligned clause
   }  // SIMD
+
+  // 2.7.3 Single Construct Restriction
+  if (GetContext().directive == OmpDirective::END_SINGLE) {
+    if (auto *clause{FindClause(OmpClause::COPYPRIVATE)}) {
+      if (FindClause(OmpClause::NOWAIT)) {
+        context_.Say(clause->source,
+            "The COPYPRIVATE clause must not be used with "
+            "the NOWAIT clause"_err_en_US);
+      }
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause &x) {
   SetContextClause(x);
 }
 
+void OmpStructureChecker::Enter(const parser::OmpNowait &) {
+  switch (GetContext().directive) {
+  case OmpDirective::SECTIONS:
+  case OmpDirective::WORKSHARE:
+  case OmpDirective::DO_SIMD:
+  case OmpDirective::DO: break;  // NOWAIT is handled by parser
+  default: CheckAllowed(OmpClause::NOWAIT); break;
+  }
+}
 void OmpStructureChecker::Enter(const parser::OmpClause::Defaultmap &) {
   CheckAllowed(OmpClause::DEFAULTMAP);
 }

diff --git a/flang/lib/semantics/check-omp-structure.h b/flang/lib/semantics/check-omp-structure.h
index 2370d86..e5d5a53 100644 (file)
--- a/flang/lib/semantics/check-omp-structure.h
+++ b/flang/lib/semantics/check-omp-structure.h
@@ -26,14 +26,14 @@
 
 namespace Fortran::semantics {
 
-ENUM_CLASS(OmpDirective, PARALLEL, DO, SECTIONS, SECTION, SINGLE, WORKSHARE,
-    SIMD, DECLARE_SIMD, DO_SIMD, TASK, TASKLOOP, TASKLOOP_SIMD, TASKYIELD,
-    TARGET_DATA, TARGET_ENTER_DATA, TARGET_EXIT_DATA, TARGET, TARGET_UPDATE,
-    DECLARE_TARGET, TEAMS, DISTRIBUTE, DISTRIBUTE_SIMD, DISTRIBUTE_PARALLEL_DO,
-    DISTRIBUTE_PARALLEL_DO_SIMD, PARALLEL_DO, PARALLEL_SECTIONS,
-    PARALLEL_WORKSHARE, PARALLEL_DO_SIMD, TARGET_PARALLEL, TARGET_PARALLEL_DO,
-    TARGET_PARALLEL_DO_SIMD, TARGET_SIMD, TARGET_TEAMS, TEAMS_DISTRIBUTE,
-    TEAMS_DISTRIBUTE_SIMD, TARGET_TEAMS_DISTRIBUTE,
+ENUM_CLASS(OmpDirective, PARALLEL, DO, SECTIONS, SECTION, SINGLE, END_SINGLE,
+    WORKSHARE, SIMD, DECLARE_SIMD, DO_SIMD, TASK, TASKLOOP, TASKLOOP_SIMD,
+    TASKYIELD, TARGET_DATA, TARGET_ENTER_DATA, TARGET_EXIT_DATA, TARGET,
+    TARGET_UPDATE, DECLARE_TARGET, TEAMS, DISTRIBUTE, DISTRIBUTE_SIMD,
+    DISTRIBUTE_PARALLEL_DO, DISTRIBUTE_PARALLEL_DO_SIMD, PARALLEL_DO,
+    PARALLEL_SECTIONS, PARALLEL_WORKSHARE, PARALLEL_DO_SIMD, TARGET_PARALLEL,
+    TARGET_PARALLEL_DO, TARGET_PARALLEL_DO_SIMD, TARGET_SIMD, TARGET_TEAMS,
+    TEAMS_DISTRIBUTE, TEAMS_DISTRIBUTE_SIMD, TARGET_TEAMS_DISTRIBUTE,
     TARGET_TEAMS_DISTRIBUTE_SIMD, TEAMS_DISTRIBUTE_PARALLEL_DO,
     TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO, TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD,
     TARGET_TEAMS_DISTRIBUTE_PARALLEL_DO_SIMD, MASTER, CRITICAL, BARRIER,
@@ -64,8 +64,21 @@ public:
   void Leave(const parser::OpenMPBlockConstruct &);
   void Enter(const parser::OmpBlockDirective &);
 
+  void Enter(const parser::OpenMPSectionsConstruct &);
+  void Leave(const parser::OpenMPSectionsConstruct &);
+  void Enter(const parser::OmpSection &);
+
+  void Enter(const parser::OpenMPSingleConstruct &x);
+  void Leave(const parser::OpenMPSingleConstruct &x);
+  void Enter(const parser::OmpEndSingle &x);
+  void Leave(const parser::OmpEndSingle &x);
+
+  void Enter(const parser::OpenMPWorkshareConstruct &x);
+  void Leave(const parser::OpenMPWorkshareConstruct &x);
+
   void Leave(const parser::OmpClauseList &);
   void Enter(const parser::OmpClause &);
+  void Enter(const parser::OmpNowait &);
   void Enter(const parser::OmpClause::Defaultmap &);
   void Enter(const parser::OmpClause::Inbranch &);
   void Enter(const parser::OmpClause::Mergeable &);
@@ -120,25 +133,28 @@ private:
     std::multimap<OmpClause, const parser::OmpClause *> clauseInfo;
   };
   // back() is the top of the stack
-  const OmpContext &GetContext() const { return ompContext_.back(); }
+  OmpContext &GetContext() {
+    CHECK(!ompContext_.empty());
+    return ompContext_.back();
+  }
   void SetContextDirectiveSource(const parser::CharBlock &directive) {
-    ompContext_.back().directiveSource = directive;
+    GetContext().directiveSource = directive;
   }
   void SetContextClause(const parser::OmpClause &clause) {
-    ompContext_.back().clauseSource = clause.source;
-    ompContext_.back().clause = &clause;
+    GetContext().clauseSource = clause.source;
+    GetContext().clause = &clause;
   }
   void SetContextDirectiveEnum(const OmpDirective &dir) {
-    ompContext_.back().directive = dir;
+    GetContext().directive = dir;
   }
   void SetContextAllowed(const OmpClauseSet &allowed) {
-    ompContext_.back().allowedClauses = allowed;
+    GetContext().allowedClauses = allowed;
   }
   void SetContextAllowedOnce(const OmpClauseSet &allowedOnce) {
-    ompContext_.back().allowedOnceClauses = allowedOnce;
+    GetContext().allowedOnceClauses = allowedOnce;
   }
   void SetContextClauseInfo(const OmpClause &type) {
-    ompContext_.back().clauseInfo.emplace(type, ompContext_.back().clause);
+    GetContext().clauseInfo.emplace(type, GetContext().clause);
   }
   const parser::OmpClause *FindClause(const OmpClause &type) {
     auto it{GetContext().clauseInfo.find(type)};
@@ -147,6 +163,13 @@ private:
     }
     return nullptr;
   }
+  void PushContext(const parser::CharBlock &source) {
+    ompContext_.push_back(OmpContext{source});
+  }
+  void PushContext(const parser::CharBlock &source, const OmpDirective &dir) {
+    PushContext(source);
+    SetContextDirectiveEnum(dir);
+  }
 
   bool CurrentDirectiveIsNested() { return ompContext_.size() > 0; };
   bool HasInvalidWorksharingNesting(

diff --git a/flang/test/semantics/omp-clause-validity01.f90 b/flang/test/semantics/omp-clause-validity01.f90
index 28eab36..e6fae7e 100644 (file)
--- a/flang/test/semantics/omp-clause-validity01.f90
+++ b/flang/test/semantics/omp-clause-validity01.f90
@@ -154,6 +154,53 @@
      enddo
   enddo
 
+! 2.7.2 sections-clause -> private-clause |
+!                         firstprivate-clause |
+!                         lastprivate-clause |
+!                         reduction-clause
+
+  !$omp parallel
+  !$omp sections
+  !$omp section
+  a = 0.0
+  !$omp section
+  b = 1
+  !$omp end sections nowait
+  !$omp end parallel
+
+  !ERROR: SECTION directive must appear within the SECTIONS construct
+  !$omp section
+  a = 0.0
+  !$omp parallel
+  !ERROR: SECTION directive must appear within the SECTIONS construct
+  !$omp section
+  a = 3.14
+  !$omp end parallel
+
+! 2.7.3 single-clause -> private-clause |
+!                        firstprivate-clause
+!   end-single-clause -> copyprivate-clause |
+!                        nowait-clause
+
+  !$omp parallel
+  b = 1
+  !ERROR: LASTPRIVATE clause is not allowed on the SINGLE directive
+  !$omp single private(a) lastprivate(a)
+  a = 3.14
+  !ERROR: The COPYPRIVATE clause must not be used with the NOWAIT clause
+  !ERROR: At most one NOWAIT clause can appear on the END SINGLE directive
+  !$omp end single copyprivate(a) nowait nowait
+  c = 2
+  !$omp end parallel
+
+! 2.7.4 workshare
+
+  !$omp parallel
+  !$omp workshare
+  a = 1.0
+  !$omp end workshare nowait
+  !$omp end parallel
+
 ! 2.8.1 simd-clause -> safelen-clause |
 !                      simdlen-clause |
 !                      linear-clause |