[flang][OpenMP] Add semantic check for teams nesting

[peixin]

Summary

This patch implements the following check for TEAMS construct:

OpenMP Version 5.0 Teams construct restriction: A teams region can
only be strictly nested within the implicit parallel region or a target
region. If a teams construct is nested within a target construct, that
target construct must contain no statements, declarations or directives
outside of the teams construct.

Also add one test case for the check.

[peixin]

Fix errors info in related test cases.

[clementval]

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

212

This is only for OpenMP so it should be moved to check-omp-structure.h. Same for the two functions below. The check-directive-structure.h is meant to be shared bewteen OpenMP and OpenACC.

flang/test/Semantics/omp-firstprivate01.f90

15 ↗

(On Diff #360006)

Which statement is outside?

flang/test/Semantics/omp-nested-master.f90

90 ↗

(On Diff #360006)

parallel region instead of parallelregion

106 ↗

(On Diff #360006)

Missing a space here as well.

122 ↗

(On Diff #360006)

Missing space

139 ↗

(On Diff #360006)

Missing space

flang/test/Semantics/omp-nested-simd.f90

45 ↗

(On Diff #360006)

Missing space

[peixin]

@clementval Thanks for the review.

Moved two sets of functions and counters to check-omp-structure.h and merged them. Also fix the errors info in the test cases.

[peixin]

flang/test/Semantics/omp-firstprivate01.f90

15 ↗

(On Diff #360006)

This target directive has two teams construct nested inside and both of teams construct contain statements. Change the error info by replacing statements by statements or directives, which should be more reasonable.

[kiranchandramohan]

Looks OK. Have two questions.

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

632

Is the parent teams check required?

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

255

Nit: Can we add a LastType at the end of the enum and use it as the size of the directiveNest_ array?

[peixin]

@kiranchandramohan Thanks for the review. Fixed the patch according to your comments.

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

632

The restriction for two strictly nested teams has already been checked with the error info of distribute and parallel region only allowed strictly nested inside teams region. Rethink about it and reporting one more error here is more reasonable. Deleted the check of parent teams and added the error info in test case omp-nested-teams.f90.

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

255

Good idea. Added it.

[peixin]

Change directiveNest_[2] to directiveNest_[3] since one more element LastType was added.

[kiranchandramohan]

LGTM.

@clementval do you have further comments?

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

254

Nit: Sorry, I meant can you move this below the enum and use LastType (with SIMDNest =1 ) or LastType +1 as the size of the directiveNest_array?

enum directiveNestType { SIMDNest, TargetBlockOnlyTeams, LastType };
int directiveNest_[LastType+1] = {0};

[peixin]

@kiranchandramohan Thanks for the suggestions, which prevents from using magic number. Replaced directiveNest_[3] with directiveNest_[LastType+1].

[clementval]

LGTM as well. Sorry for the delay in reviewing the change to the patch. You can go ahead and land it.

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

protected:
   }
 
   void EnterSIMDNest() { simdNest_++; }
 
   void ExitSIMDNest() { simdNest_--; }
 
   int GetSIMDNest() { return simdNest_; }
 
+  void EnterTargetBlockOnlyTeams() { targetBlockOnlyTeams_ = true; }
+
+  void ExitTargetBlockOnlyTeams() { targetBlockOnlyTeams_ = false; }
+
+  bool GetTargetBlockOnlyTeams() { return targetBlockOnlyTeams_; }
+
   // Check if the given clause is present in the current context
   const PC *FindClause(C type) {
     auto it{GetContext().clauseInfo.find(type)};
     if (it != GetContext().clauseInfo.end()) {
       return it->second;
     }
     return nullptr;
   }
protected:
 
   SemanticsContext &context_;
   std::vector<DirectiveContext> dirContext_; // used as a stack
   std::unordered_map<D, DirectiveClauses<C, ClauseEnumSize>>
       directiveClausesMap_;
 
   std::string ClauseSetToString(const common::EnumSet<C, ClauseEnumSize> set);
   int simdNest_{0};
+  bool targetBlockOnlyTeams_{false};
 };
 
 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()};

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

private:
   void GetSymbolsInObjectList(const parser::OmpObjectList &, SymbolSourceMap &);
   void CheckDefinableObjects(SymbolSourceMap &, const llvm::omp::Clause);
   void CheckPrivateSymbolsInOuterCxt(
       SymbolSourceMap &, DirectivesClauseTriple &, const llvm::omp::Clause);
   const parser::Name GetLoopIndex(const parser::DoConstruct *x);
   void SetLoopInfo(const parser::OpenMPLoopConstruct &x);
   void CheckIsLoopIvPartOfClause(
       llvmOmpClause clause, const parser::OmpObjectList &ompObjectList);
+  bool CheckTargetBlockOnlyTeams(const parser::Block &);
   void CheckWorkshareBlockStmts(const parser::Block &, parser::CharBlock);
 
   void CheckLoopItrVariableIsInt(const parser::OpenMPLoopConstruct &x);
   void CheckDoWhile(const parser::OpenMPLoopConstruct &x);
   void CheckCycleConstraints(const parser::OpenMPLoopConstruct &x);
   void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
   void CheckSIMDNest(const parser::OpenMPConstruct &x);
   std::int64_t GetOrdCollapseLevel(const parser::OpenMPLoopConstruct &x);
private:
   const parser::OmpObjectList *GetOmpObjectList(const parser::OmpClause &);
   void CheckPredefinedAllocatorRestriction(const parser::CharBlock &source,
       const parser::OmpObjectList &ompObjectList);
   void CheckPredefinedAllocatorRestriction(
       const parser::CharBlock &source, const parser::Name &name);
   bool isPredefinedAllocator{false};
 };
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_

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

void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   if (CurrentDirectiveIsNested()) {
     CheckIfDoOrderedClause(beginDir);
     if (llvm::omp::teamSet.test(GetContextParent().directive)) {
       HasInvalidTeamsNesting(beginDir.v, beginDir.source);
     }
     if (GetContext().directive == llvm::omp::Directive::OMPD_master) {
       CheckMasterNesting(x);
     }
+    // A teams region can only be strictly nested within the implicit parallel
+    // region or a target region.
+    if (GetContext().directive == llvm::omp::Directive::OMPD_teams &&
+        GetContextParent().directive != llvm::omp::Directive::OMPD_target &&
+        GetContextParent().directive != llvm::omp::Directive::OMPD_teams) {
+      context_.Say(parser::FindSourceLocation(x),
+          "%s region can only be strictly nested within the implicit parallel"
+          "region or TARGET region"_err_en_US,
+          ContextDirectiveAsFortran());
+    }
+    // If a teams construct is nested within a target construct, that target
+    // construct must contain no statements, declarations or directives outside
+    // of the teams construct.
+    if (GetContext().directive == llvm::omp::Directive::OMPD_teams &&
+        GetContextParent().directive == llvm::omp::Directive::OMPD_target &&
+        !GetTargetBlockOnlyTeams()) {
+      context_.Say(GetContextParent().directiveSource,
+          "TARGET construct with nested TEAMS region contains statements "
+          "outside of the TEAMS construct"_err_en_US);
+    }
   }
 
   CheckNoBranching(block, beginDir.v, beginDir.source);
 
   switch (beginDir.v) {
+  case llvm::omp::Directive::OMPD_target:
+    if (CheckTargetBlockOnlyTeams(block)) {
+      EnterTargetBlockOnlyTeams();
+    }
+    break;
   case llvm::omp::OMPD_workshare:
   case llvm::omp::OMPD_parallel_workshare:
     CheckWorkshareBlockStmts(block, beginDir.source);
     HasInvalidWorksharingNesting(
         beginDir.source, llvm::omp::nestedWorkshareErrSet);
     break;
   case llvm::omp::Directive::OMPD_single:
     // TODO: This check needs to be extended while implementing nesting of
else if (IsCloselyNestedRegion(llvm::omp::nestedOrderedErrSet)) {
       context_.Say(blkDirective.source,
           "`ORDERED` region may not be closely nested inside of "
           "`CRITICAL`, `ORDERED`, explicit `TASK` or `TASKLOOP` region."_err_en_US);
     }
   }
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {
+  if (GetContext().directive == llvm::omp::Directive::OMPD_target) {
+    ExitTargetBlockOnlyTeams();
+  }
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
   const auto &beginSectionsDir{
       std::get<parser::OmpBeginSectionsDirective>(x.t)};
   const auto &endSectionsDir{std::get<parser::OmpEndSectionsDirective>(x.t)};
   const auto &beginDir{
if (auto *enclosingContext{GetEnclosingContextWithDir(enclosingDir)}) {
               parser::ToUpperCaseLetters(getClauseName(currClause).str()),
               symbol->name());
         }
       }
     }
   }
 }
 
+bool OmpStructureChecker::CheckTargetBlockOnlyTeams(
+    const parser::Block &block) {
+  bool nestedTeams{false};
+  auto it{block.begin()};
+
+  if (const auto *ompConstruct{parser::Unwrap<parser::OpenMPConstruct>(*it)}) {
+    if (const auto *ompBlockConstruct{
+            std::get_if<parser::OpenMPBlockConstruct>(&ompConstruct->u)}) {
+      const auto &beginBlockDir{
+          std::get<parser::OmpBeginBlockDirective>(ompBlockConstruct->t)};
+      const auto &beginDir{
+          std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
+      if (beginDir.v == llvm::omp::Directive::OMPD_teams) {
+        nestedTeams = true;
+      }
+    }
+  }
+
+  if (nestedTeams && ++it == block.end()) {
+    return true;
+  } else {
+    return false;
+  }
+}
+
 void OmpStructureChecker::CheckWorkshareBlockStmts(
     const parser::Block &block, parser::CharBlock source) {
   OmpWorkshareBlockChecker ompWorkshareBlockChecker{context_, source};
 
   for (auto it{block.begin()}; it != block.end(); ++it) {
     if (parser::Unwrap<parser::AssignmentStmt>(*it) ||
         parser::Unwrap<parser::ForallStmt>(*it) ||
         parser::Unwrap<parser::ForallConstruct>(*it) ||

flang/test/Semantics/omp-teams01.f90

+! RUN: %S/test_errors.sh %s %t %flang -fopenmp
+! REQUIRES: shell
+
+! OpenMP Version 5.0
+! Check OpenMP construct validity for the following directives:
+! 2.7 Teams Construct
+
+program main
+  integer :: i, j, N = 10
+  real :: a, b, c
+
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+
+  !$omp target
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end target
+
+  !$omp target
+  !$omp parallel
+  !ERROR: TEAMS region can only be strictly nested within the implicit parallelregion or TARGET region
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end parallel
+  !$omp end target
+
+  !$omp parallel
+  !ERROR: TEAMS region can only be strictly nested within the implicit parallelregion or TARGET region
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end parallel
+
+  !$omp do
+  do i = 1, N
+  !ERROR: TEAMS region can only be strictly nested within the implicit parallelregion or TARGET region
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  end do
+
+  !$omp master
+  !ERROR: TEAMS region can only be strictly nested within the implicit parallelregion or TARGET region
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end master
+
+  !$omp target parallel
+  !ERROR: TEAMS region can only be strictly nested within the implicit parallelregion or TARGET region
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end target parallel
+
+  !$omp target
+  !$omp teams
+  !ERROR: Only `DISTRIBUTE` or `PARALLEL` regions are allowed to be strictly nested inside `TEAMS` region.
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end teams
+  !$omp end target
+
+  !$omp target teams
+  !ERROR: TEAMS region can only be strictly nested within the implicit parallelregion or TARGET region
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end target teams
+
+  !ERROR: TARGET construct with nested TEAMS region contains statements outside of the TEAMS construct
+  !$omp target
+  do i = 1, N
+    !$omp teams
+    a = 3.14
+    !$omp end teams
+  enddo
+  !$omp end target
+
+  !ERROR: TARGET construct with nested TEAMS region contains statements outside of the TEAMS construct
+  !$omp target
+  if (i .GT. 1) then
+    if (j .GT. 1) then
+      !$omp teams
+      a = 3.14
+      !$omp end teams
+    end if
+  end if
+  !$omp end target
+
+  !ERROR: TARGET construct with nested TEAMS region contains statements outside of the TEAMS construct
+  !$omp target
+  b = 3.14
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  !$omp end target
+
+  !ERROR: TARGET construct with nested TEAMS region contains statements outside of the TEAMS construct
+  !$omp target
+  !$omp teams
+  a = 3.14
+  !$omp end teams
+  c = 3.14
+  !$omp end target
+
+end program main