[flang-commits] [flang] 61f11f8 - [Flang][OpenMP][OpenACC] Fix exit out of a region in OpenMP parallel construct.

Sameeran joshi via flang-commits flang-commits at lists.llvm.org
Thu Oct 29 22:33:51 PDT 2020


Author: sameeran joshi
Date: 2020-10-30T11:03:30+05:30
New Revision: 61f11f807cfc359b38b9e22d005456f924464c30

URL: https://github.com/llvm/llvm-project/commit/61f11f807cfc359b38b9e22d005456f924464c30
DIFF: https://github.com/llvm/llvm-project/commit/61f11f807cfc359b38b9e22d005456f924464c30.diff

LOG: [Flang][OpenMP][OpenACC] Fix exit out of a region in OpenMP parallel construct.

>From below mentioned standard references
OpenACC 3.0 Standards document
840 • A program may not branch into or out of an OpenACC parallel construct

OpenMP 5.0 Standards document
A program that branches into or out of a parallel region is non-conforming.

This patch
Resolves the issue of exit out of a parallel region, other branching out issues like goto statements are not handled with this patch.
Moves code from D87906 to be reused by other OpenMP/OpenACC to check-directive-structure.h.
Adds support in OpenMP parallel construct and a test case to verify.

Reviewed By: clementval

Differential Revision: https://reviews.llvm.org/D88655

Added: 
    

Modified: 
    flang/lib/Semantics/check-acc-structure.cpp
    flang/lib/Semantics/check-acc-structure.h
    flang/lib/Semantics/check-directive-structure.h
    flang/lib/Semantics/check-omp-structure.cpp
    flang/test/Semantics/omp-clause-validity01.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 4559050e6935..8c05b585dda5 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -44,83 +44,6 @@ static constexpr inline AccClauseSet routineOnlyAllowedAfterDeviceTypeClauses{
     llvm::acc::Clause::ACCC_bind, llvm::acc::Clause::ACCC_gang,
     llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_worker};
 
-class NoBranchingEnforce {
-public:
-  NoBranchingEnforce(SemanticsContext &context,
-      parser::CharBlock sourcePosition, llvm::acc::Directive directive)
-      : context_{context}, sourcePosition_{sourcePosition}, currentDirective_{
-                                                                directive} {}
-  template <typename T> bool Pre(const T &) { return true; }
-  template <typename T> void Post(const T &) {}
-
-  template <typename T> bool Pre(const parser::Statement<T> &statement) {
-    currentStatementSourcePosition_ = statement.source;
-    return true;
-  }
-
-  void Post(const parser::ReturnStmt &) { EmitBranchOutError("RETURN"); }
-  void Post(const parser::ExitStmt &exitStmt) {
-    if (const auto &exitName{exitStmt.v}) {
-      CheckConstructNameBranching("EXIT", exitName.value());
-    }
-  }
-  void Post(const parser::StopStmt &) { EmitBranchOutError("STOP"); }
-
-private:
-  parser::MessageFormattedText GetEnclosingMsg() const {
-    return {"Enclosing %s construct"_en_US,
-        parser::ToUpperCaseLetters(
-            llvm::acc::getOpenACCDirectiveName(currentDirective_).str())};
-  }
-
-  void EmitBranchOutError(const char *stmt) const {
-    context_
-        .Say(currentStatementSourcePosition_,
-            "%s statement is not allowed in a %s construct"_err_en_US, stmt,
-            parser::ToUpperCaseLetters(
-                llvm::acc::getOpenACCDirectiveName(currentDirective_).str()))
-        .Attach(sourcePosition_, GetEnclosingMsg());
-  }
-
-  void EmitBranchOutErrorWithName(
-      const char *stmt, const parser::Name &toName) const {
-    const std::string branchingToName{toName.ToString()};
-    const auto upperCaseConstructName{parser::ToUpperCaseLetters(
-        llvm::acc::getOpenACCDirectiveName(currentDirective_).str())};
-    context_
-        .Say(currentStatementSourcePosition_,
-            "%s to construct '%s' outside of %s construct is not allowed"_err_en_US,
-            stmt, branchingToName, upperCaseConstructName)
-        .Attach(sourcePosition_, GetEnclosingMsg());
-  }
-
-  // Current semantic checker is not following OpenACC constructs as they are
-  // not Fortran constructs. Hence the ConstructStack doesn't capture OpenACC
-  // constructs. Apply an inverse way to figure out if a construct-name is
-  // branching out of an OpenACC construct. The control flow goes out of an
-  // OpenACC construct, if a construct-name from statement is found in
-  // ConstructStack.
-  void CheckConstructNameBranching(
-      const char *stmt, const parser::Name &stmtName) {
-    const ConstructStack &stack{context_.constructStack()};
-    for (auto iter{stack.cend()}; iter-- != stack.cbegin();) {
-      const ConstructNode &construct{*iter};
-      const auto &constructName{MaybeGetNodeName(construct)};
-      if (constructName) {
-        if (stmtName.source == constructName->source) {
-          EmitBranchOutErrorWithName(stmt, stmtName);
-          return;
-        }
-      }
-    }
-  }
-
-  SemanticsContext &context_;
-  parser::CharBlock currentStatementSourcePosition_;
-  parser::CharBlock sourcePosition_;
-  llvm::acc::Directive currentDirective_;
-};
-
 bool AccStructureChecker::CheckAllowedModifier(llvm::acc::Clause clause) {
   if (GetContext().directive == llvm::acc::ACCD_enter_data ||
       GetContext().directive == llvm::acc::ACCD_exit_data) {
@@ -186,13 +109,6 @@ void AccStructureChecker::Leave(const parser::OpenACCBlockConstruct &x) {
   dirContext_.pop_back();
 }
 
-void AccStructureChecker::CheckNoBranching(const parser::Block &block,
-    const llvm::acc::Directive directive,
-    const parser::CharBlock &directiveSource) const {
-  NoBranchingEnforce noBranchingEnforce{context_, directiveSource, directive};
-  parser::Walk(block, noBranchingEnforce);
-}
-
 void AccStructureChecker::Enter(
     const parser::OpenACCStandaloneDeclarativeConstruct &x) {
   const auto &declarativeDir{std::get<parser::AccDeclarativeDirective>(x.t)};

diff  --git a/flang/lib/Semantics/check-acc-structure.h b/flang/lib/Semantics/check-acc-structure.h
index 4373ffc60a91..04c08b14598c 100644
--- a/flang/lib/Semantics/check-acc-structure.h
+++ b/flang/lib/Semantics/check-acc-structure.h
@@ -111,12 +111,7 @@ class AccStructureChecker
 
 private:
 
-  void CheckNoBranching(const parser::Block &block,
-      const llvm::acc::Directive directive,
-      const parser::CharBlock &directiveSource) const;
-
   bool CheckAllowedModifier(llvm::acc::Clause clause);
-
   llvm::StringRef getClauseName(llvm::acc::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::acc::Directive directive) override;
 };

diff  --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
index 6755e475832a..1de0b3b9d26b 100644
--- a/flang/lib/Semantics/check-directive-structure.h
+++ b/flang/lib/Semantics/check-directive-structure.h
@@ -27,6 +27,84 @@ template <typename C, std::size_t ClauseEnumSize> struct DirectiveClauses {
   const common::EnumSet<C, ClauseEnumSize> requiredOneOf;
 };
 
+// Generic branching checker for invalid branching out of OpenMP/OpenACC
+// directive.
+// typename D is the directive enumeration.
+template <typename D> class NoBranchingEnforce {
+public:
+  NoBranchingEnforce(SemanticsContext &context,
+      parser::CharBlock sourcePosition, D directive,
+      std::string &&upperCaseDirName)
+      : context_{context}, sourcePosition_{sourcePosition},
+        currentDirective_{directive}, upperCaseDirName_{
+                                          std::move(upperCaseDirName)} {}
+  template <typename T> bool Pre(const T &) { return true; }
+  template <typename T> void Post(const T &) {}
+
+  template <typename T> bool Pre(const parser::Statement<T> &statement) {
+    currentStatementSourcePosition_ = statement.source;
+    return true;
+  }
+
+  void Post(const parser::ReturnStmt &) { EmitBranchOutError("RETURN"); }
+  void Post(const parser::ExitStmt &exitStmt) {
+    if (const auto &exitName{exitStmt.v}) {
+      CheckConstructNameBranching("EXIT", exitName.value());
+    }
+  }
+  void Post(const parser::StopStmt &) { EmitBranchOutError("STOP"); }
+
+private:
+  parser::MessageFormattedText GetEnclosingMsg() const {
+    return {"Enclosing %s construct"_en_US, upperCaseDirName_};
+  }
+
+  void EmitBranchOutError(const char *stmt) const {
+    context_
+        .Say(currentStatementSourcePosition_,
+            "%s statement is not allowed in a %s construct"_err_en_US, stmt,
+            upperCaseDirName_)
+        .Attach(sourcePosition_, GetEnclosingMsg());
+  }
+
+  void EmitBranchOutErrorWithName(
+      const char *stmt, const parser::Name &toName) const {
+    const std::string branchingToName{toName.ToString()};
+    context_
+        .Say(currentStatementSourcePosition_,
+            "%s to construct '%s' outside of %s construct is not allowed"_err_en_US,
+            stmt, branchingToName, upperCaseDirName_)
+        .Attach(sourcePosition_, GetEnclosingMsg());
+  }
+
+  // Current semantic checker is not following OpenACC/OpenMP constructs as they
+  // are not Fortran constructs. Hence the ConstructStack doesn't capture
+  // OpenACC/OpenMP constructs. Apply an inverse way to figure out if a
+  // construct-name is branching out of an OpenACC/OpenMP construct. The control
+  // flow goes out of an OpenACC/OpenMP construct, if a construct-name from
+  // statement is found in ConstructStack.
+  void CheckConstructNameBranching(
+      const char *stmt, const parser::Name &stmtName) {
+    const ConstructStack &stack{context_.constructStack()};
+    for (auto iter{stack.cend()}; iter-- != stack.cbegin();) {
+      const ConstructNode &construct{*iter};
+      const auto &constructName{MaybeGetNodeName(construct)};
+      if (constructName) {
+        if (stmtName.source == constructName->source) {
+          EmitBranchOutErrorWithName(stmt, stmtName);
+          return;
+        }
+      }
+    }
+  }
+
+  SemanticsContext &context_;
+  parser::CharBlock currentStatementSourcePosition_;
+  parser::CharBlock sourcePosition_;
+  std::string upperCaseDirName_;
+  D currentDirective_;
+};
+
 // Generic structure checker for directives/clauses language such as OpenMP
 // and OpenACC.
 // typename D is the directive enumeration.
@@ -148,6 +226,8 @@ class DirectiveStructureChecker : public virtual BaseChecker {
       SayNotMatching(beginDir.source, endDir.source);
     }
   }
+  void CheckNoBranching(const parser::Block &block, D directive,
+      const parser::CharBlock &directiveSource);
 
   // Check that only clauses in set are after the specific clauses.
   void CheckOnlyAllowedAfter(C clause, common::EnumSet<C, ClauseEnumSize> set);
@@ -186,6 +266,15 @@ class DirectiveStructureChecker : public virtual BaseChecker {
   std::string ClauseSetToString(const common::EnumSet<C, ClauseEnumSize> set);
 };
 
+template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
+void DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::CheckNoBranching(
+    const parser::Block &block, D directive,
+    const parser::CharBlock &directiveSource) {
+  NoBranchingEnforce<D> noBranchingEnforce{
+      context_, directiveSource, directive, ContextDirectiveAsFortran()};
+  parser::Walk(block, noBranchingEnforce);
+}
+
 // Check that only clauses included in the given set are present after the given
 // clause.
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>

diff  --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9140cfe74e3b..583021b5c701 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -95,9 +95,18 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   const auto &endBlockDir{std::get<parser::OmpEndBlockDirective>(x.t)};
   const auto &beginDir{std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
   const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir.t)};
+  const parser::Block &block{std::get<parser::Block>(x.t)};
+
   CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
 
   PushContextAndClauseSets(beginDir.source, beginDir.v);
+
+  switch (beginDir.v) {
+  case llvm::omp::OMPD_parallel:
+    CheckNoBranching(block, llvm::omp::OMPD_parallel, beginDir.source);
+  default:
+    break;
+  }
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {

diff  --git a/flang/test/Semantics/omp-clause-validity01.f90 b/flang/test/Semantics/omp-clause-validity01.f90
index 358c9fc56549..a4e668d29f4f 100644
--- a/flang/test/Semantics/omp-clause-validity01.f90
+++ b/flang/test/Semantics/omp-clause-validity01.f90
@@ -163,6 +163,22 @@
   !ERROR: Unmatched END TARGET directive
   !$omp end target
 
+  ! OMP 5.0 - 2.6 Restriction point 1
+  outofparallel: do k =1, 10
+  !$omp parallel
+  !$omp do
+  outer: do i=0, 10
+    inner: do j=1, 10
+      exit
+      exit outer
+      !ERROR: EXIT to construct 'outofparallel' outside of PARALLEL construct is not allowed
+      exit outofparallel
+    end do inner
+  end do outer
+  !$end omp do
+  !$omp end parallel
+  end do outofparallel
+
 ! 2.7.1  do-clause -> private-clause |
 !                     firstprivate-clause |
 !                     lastprivate-clause |


        


More information about the flang-commits mailing list