[Flang][openmp][openacc] Extend CheckNoBranching to handle branching provided by LabelEnforce.

[sameeranjoshi]

Summary

CheckNoBranching is currently handling only illegal branching out for constructs
with Parser::Name in them.
Extend the same for handling illegal branching out caused by Parser::Label based statements.
This patch could possibly solve one of the issues(typically branching out) mentioned in D92735.

[sameeranjoshi]

Addresses one of the comments from
https://reviews.llvm.org/D88655#2356685

[clementval]

flang/lib/Semantics/check-directive-structure.h

45

Braces should be removed for simple if

[kiranchandramohan]

LGTM.

[sameeranjoshi]

flang/lib/Semantics/check-directive-structure.h

45

Was it a comment on fir-dev?
I think flang[1] allows and fir-dev follows llvm coding standard[2].

[1] https://github.com/llvm/llvm-project/blob/master/flang/docs/C%2B%2Bstyle.md#layout
[2] https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

[klausler]

flang/lib/Semantics/check-directive-structure.h

45

Please conform to the style used in the Fortran front-end; we always use braces for safety and future maintainability.

[clementval]

flang/lib/Semantics/check-directive-structure.h

45

My bad :(

[awarzynski]

Hi @sameeranjoshi , thank you for working on this!

Sadly with this patch one of our upstream builders has started failing: http://lab.llvm.org:8011/#/builders/33/builds/1866. More specifically,Semantics/omp-parallell01.f90 is failing.

All other builders are fine, which makes this weird. The only unique thing about the failing builder (flang-aarch64-ubuntu-clang) is that it uses clang-10. Could you take a look?

Edit: @kiranchandramohan just pointed out to me offline that this is probably fixed here: https://reviews.llvm.org/D94618.

[sameeranjoshi]

In D93447#2498085, @awarzynski wrote:

Hi @sameeranjoshi , thank you for working on this!

Sadly with this patch one of our upstream builders has started failing: http://lab.llvm.org:8011/#/builders/33/builds/1866. More specifically,Semantics/omp-parallell01.f90 is failing.

All other builders are fine, which makes this weird. The only unique thing about the failing builder (flang-aarch64-ubuntu-clang) is that it uses clang-10. Could you take a look?

Edit: @kiranchandramohan just pointed out to me offline that this is probably fixed here: https://reviews.llvm.org/D94618.

Thanks for reporting and reducing it to the exact problem.

flang/lib/Semantics/check-directive-structure.h

 // directive language.
 
 #ifndef FORTRAN_SEMANTICS_CHECK_DIRECTIVE_STRUCTURE_H_
 #define FORTRAN_SEMANTICS_CHECK_DIRECTIVE_STRUCTURE_H_
 
 #include "flang/Common/enum-set.h"
 #include "flang/Semantics/semantics.h"
 #include "flang/Semantics/tools.h"
-
 #include <unordered_map>
 
 namespace Fortran::semantics {
 
 template <typename C, std::size_t ClauseEnumSize> struct DirectiveClauses {
   const common::EnumSet<C, ClauseEnumSize> allowed;
   const common::EnumSet<C, ClauseEnumSize> allowedOnce;
   const common::EnumSet<C, ClauseEnumSize> allowedExclusive;
NoBranchingEnforce(SemanticsContext &context,
       : context_{context}, sourcePosition_{sourcePosition},
         upperCaseDirName_{std::move(upperCaseDirName)}, 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;
+    if (statement.label.has_value()) {
+      labels_.insert(*statement.label);
+    }
     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"); }
 
+  std::set<parser::Label> labels() { return labels_; }
+
 private:
   parser::MessageFormattedText GetEnclosingMsg() const {
     return {"Enclosing %s construct"_en_US, upperCaseDirName_};
   }
 
   void EmitBranchOutError(const char *stmt) const {
     context_
         .Say(currentStatementSourcePosition_,
void CheckConstructNameBranching(
     }
   }
 
   SemanticsContext &context_;
   parser::CharBlock currentStatementSourcePosition_;
   parser::CharBlock sourcePosition_;
   std::string upperCaseDirName_;
   D currentDirective_;
+  std::set<parser::Label> labels_;
 };
 
 // Generic structure checker for directives/clauses language such as OpenMP
 // and OpenACC.
 // typename D is the directive enumeration.
 // tyepname C is the clause enumeration.
 // typename PC is the parser class defined in parse-tree.h for the clauses.
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
protected:
 
   template <typename B> void CheckMatching(const B &beginDir, const B &endDir) {
     const auto &begin{beginDir.v};
     const auto &end{endDir.v};
     if (begin != end) {
       SayNotMatching(beginDir.source, endDir.source);
     }
   }
+  // Check illegal branching out of `Parser::Block` for `Parser::Name` based
+  // nodes (examples `Parser::ExitStmt`) along with `Parser::Label`
+  // based nodes (example `Parser::GotoStmt`).
   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);
 
   void CheckRequireAtLeastOneOf();
 

 
 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);
+
+  LabelEnforce directiveLabelEnforce{context_, noBranchingEnforce.labels(),
+      directiveSource,
+      parser::ToUpperCaseLetters(getDirectiveName(directive).str()).c_str()};
+  parser::Walk(block, directiveLabelEnforce);
 }
 
 // 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>
 void DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::CheckOnlyAllowedAfter(
     C clause, common::EnumSet<C, ClauseEnumSize> set) {
   bool enforceCheck = false;

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

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);
-    break;
-  default:
-    break;
-  }
+  CheckNoBranching(block, beginDir.v, beginDir.source);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
   const auto &beginSectionsDir{

flang/test/Semantics/omp-parallell01.f90

 ! RUN: %S/test_errors.sh %s %t %f18 -fopenmp
-! XFAIL: *
 
 ! OpenMP Version 4.5
 ! 2.5 parallel construct.
 ! A program that branches into or out of a parallel region
 ! is non-conforming.
 
 program omp_parallel
   integer i, j, k
 
   !$omp parallel
   do i = 1, 10
     do j = 1, 10
       print *, "Hello"
-      !ERROR: invalid branch to/from OpenMP structured block
+      !ERROR: Control flow escapes from PARALLEL
       goto 10
     end do
   end do
   !$omp end parallel
 
   10 stop
 
 end program omp_parallel