[flang][OpenMP] Add semantic checks for strict nesting inside `teams` construct.

[arnamoy10]

Summary

This patch implements the following two checks:

1. Only ‘distribute’ or ‘parallel’ regions are allowed to be strictly nested inside ‘teams’ region
2. DISTRIBUTE region has to be strictly nested inside TEAMS region.

Modifies the existing test cases with the new error messages and adds a new test.

[kiranchandramohan]

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

187

Nit: use braced initialization.

238

This way of iterating at the top-level blocks will miss cases where there is a construct (for e.g do loop) which has further blocks inside. See a test below,

!$omp teams
do i = 1, N
!$omp task
  do k = 1, N
   a = 3.14
enddo
!$omp end task
enddo
!$omp end teams

[arnamoy10]

As per reviewers suggestion, following a bottom-up approach (instead of the previously implemented top down approach) to find the violations.

Modified the test case to add the suggested test case.

[kiranchandramohan]

Apologies for the delay. I have a question and a suggestion.

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

238

This visit will cause an additional traversal of the subtree rooted at the teams node. Visits to these nodes will be happening anyway during the normal traversal with various Enter and Leave functions.

Would it be possible to do these checks in the following functions? You can add if they don't exist.

void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &c)
void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &c)

Let me know if there is an issue with this approach.

[arnamoy10]

Addressing reviewers' comments, avoiding additional traversals of the parse tree nodes.

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

238

Makes sense. Thanks for the suggestion.

[kiranchandramohan]

LGTM.

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

105

Nit: empty line.

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

static OmpDirectiveSet simdSet{Directive::OMPD_distribute_parallel_do_simd,
     Directive::OMPD_distribute_simd, Directive::OMPD_parallel_do_simd,
     Directive::OMPD_do_simd, Directive::OMPD_simd,
     Directive::OMPD_target_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_simd, Directive::OMPD_target_simd,
     Directive::OMPD_taskloop_simd,
     Directive::OMPD_teams_distribute_parallel_do_simd,
     Directive::OMPD_teams_distribute_simd};
+static OmpDirectiveSet teamSet{Directive::OMPD_teams,
+    Directive::OMPD_teams_distribute,
+    Directive::OMPD_teams_distribute_parallel_do,
+    Directive::OMPD_teams_distribute_parallel_do_simd,
+    Directive::OMPD_teams_distribute_parallel_for,
+    Directive::OMPD_teams_distribute_parallel_for_simd,
+    Directive::OMPD_teams_distribute_simd};
 static OmpDirectiveSet taskGeneratingSet{
     OmpDirectiveSet{Directive::OMPD_task} | taskloopSet};
 static OmpDirectiveSet nestedOrderedErrSet{Directive::OMPD_critical,
     Directive::OMPD_ordered, Directive::OMPD_atomic, Directive::OMPD_task,
     Directive::OMPD_taskloop};
 static OmpClauseSet privateSet{
     Clause::OMPC_private, Clause::OMPC_firstprivate, Clause::OMPC_lastprivate};
 static OmpClauseSet privateReductionSet{

 #define GEN_FLANG_CLAUSE_PARSER_KIND_MAP
 #include "llvm/Frontend/OpenMP/OMP.inc"
   }
 
 private:
   bool HasInvalidWorksharingNesting(
       const parser::CharBlock &, const OmpDirectiveSet &);
   bool IsCloselyNestedRegion(const OmpDirectiveSet &set);
+  void HasInvalidTeamsNesting(
+      const llvm::omp::Directive &dir, const parser::CharBlock &source);
+  void HasInvalidDistributeNesting(const parser::OpenMPLoopConstruct &x);
   // specific clause related
   bool ScheduleModifierHasType(const parser::OmpScheduleClause &,
       const parser::OmpScheduleModifierType::ModType &);
   void CheckAllowedMapTypes(const parser::OmpMapType::Type &,
       const std::list<parser::OmpMapType::Type> &);
   llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
 

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

context_.Say(source,
         "A worksharing region may not be closely nested inside a "
         "worksharing, explicit task, taskloop, critical, ordered, atomic, or "
         "master region"_err_en_US);
     return true;
   }
   return false;
 }
 
-void OmpStructureChecker::Enter(const parser::OpenMPConstruct &) {
+void OmpStructureChecker::HasInvalidDistributeNesting(
+    const parser::OpenMPLoopConstruct &x) {
+  bool violation{false};
+
+  OmpDirectiveSet distributeSet{llvm::omp::Directive::OMPD_distribute,
+      llvm::omp::Directive::OMPD_distribute_parallel_do,
+      llvm::omp::Directive::OMPD_distribute_parallel_do_simd,
+      llvm::omp::Directive::OMPD_distribute_parallel_for,
+      llvm::omp::Directive::OMPD_distribute_parallel_for_simd,
+      llvm::omp::Directive::OMPD_distribute_simd};
+
+  const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
+  const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
+  if (distributeSet.test(beginDir.v)) {
+    // `distribute` region has to be nested
+    if (!CurrentDirectiveIsNested()) {
+      violation = true;
+    } else {
+      // `distribute` region has to be strictly nested inside `teams`
+      if (!llvm::omp::teamSet.test(GetContextParent().directive)) {
+        violation = true;
+      }
+    }
+  }
+  if (violation) {
+    context_.Say(beginDir.source,
+        "`DISTRIBUTE` region has to be strictly nested inside `TEAMS` region."_err_en_US);
+  }
+}
+
+void OmpStructureChecker::HasInvalidTeamsNesting(
+    const llvm::omp::Directive &dir, const parser::CharBlock &source) {
+  OmpDirectiveSet allowedSet{llvm::omp::Directive::OMPD_parallel,
+      llvm::omp::Directive::OMPD_parallel_do,
+      llvm::omp::Directive::OMPD_parallel_do_simd,
+      llvm::omp::Directive::OMPD_parallel_for,
+      llvm::omp::Directive::OMPD_parallel_for_simd,
+      llvm::omp::Directive::OMPD_parallel_master,
+      llvm::omp::Directive::OMPD_parallel_master_taskloop,
+      llvm::omp::Directive::OMPD_parallel_master_taskloop_simd,
+      llvm::omp::Directive::OMPD_parallel_sections,
+      llvm::omp::Directive::OMPD_parallel_workshare,
+      llvm::omp::Directive::OMPD_distribute,
+      llvm::omp::Directive::OMPD_distribute_parallel_do,
+      llvm::omp::Directive::OMPD_distribute_parallel_do_simd,
+      llvm::omp::Directive::OMPD_distribute_parallel_for,
+      llvm::omp::Directive::OMPD_distribute_parallel_for_simd,
+      llvm::omp::Directive::OMPD_distribute_simd};
+
+  if (!allowedSet.test(dir)) {
+    context_.Say(source,
+        "Only `DISTRIBUTE` or `PARALLEL` regions are allowed to be strictly nested inside `TEAMS` region."_err_en_US);
+  }
+}
+
+void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
   // 2.8.1 TODO: Simd Construct with Ordered Construct Nesting check
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
   const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
   const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
 
   // check matching, End directive is optional
void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
   if (const auto &doConstruct{
           std::get<std::optional<parser::DoConstruct>>(x.t)}) {
     const auto &doBlock{std::get<parser::Block>(doConstruct->t)};
     CheckNoBranching(doBlock, beginDir.v, beginDir.source);
   }
   CheckDoWhile(x);
   CheckLoopItrVariableIsInt(x);
   CheckCycleConstraints(x);
+  HasInvalidDistributeNesting(x);
+  if (CurrentDirectiveIsNested() &&
+      llvm::omp::teamSet.test(GetContextParent().directive)) {
+    HasInvalidTeamsNesting(beginDir.v, beginDir.source);
+  }
 }
 const parser::Name OmpStructureChecker::GetLoopIndex(
     const parser::DoConstruct *x) {
   using Bounds = parser::LoopControl::Bounds;
   return std::get<Bounds>(x->GetLoopControl()->u).name.thing;
 }
 void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) {
   if (const auto &loopConstruct{
void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   PushContextAndClauseSets(beginDir.source, beginDir.v);
 
   // TODO: This check needs to be extended while implementing nesting of regions
   // checks.
   if (beginDir.v == llvm::omp::Directive::OMPD_single) {
     HasInvalidWorksharingNesting(
         beginDir.source, {llvm::omp::Directive::OMPD_do});
   }
-  if (CurrentDirectiveIsNested())
+  if (CurrentDirectiveIsNested()) {
     CheckIfDoOrderedClause(beginDir);
+    if (llvm::omp::teamSet.test(GetContextParent().directive)) {
+      HasInvalidTeamsNesting(beginDir.v, beginDir.source);
+    }
+  }
 
   CheckNoBranching(block, beginDir.v, beginDir.source);
 
   switch (beginDir.v) {
   case llvm::omp::OMPD_workshare:
   case llvm::omp::OMPD_parallel_workshare:
     CheckWorkshareBlockStmts(block, beginDir.source);
     break;

flang/test/Semantics/omp-combined-constructs.f90

 ! RUN: %S/test_errors.sh %s %t %flang -fopenmp
 
 program main
   implicit none
   integer :: N
   integer :: i
   real(8) :: a(256), b(256)
   N = 256
 
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
   !$omp distribute simd
   do i = 1, N
      a(i) = 3.14
   enddo
   !$omp end distribute simd
 
   !$omp target parallel device(0)
   do i = 1, N

flang/test/Semantics/omp-device-constructs.f90

program main
 
   !ERROR: At most one DEVICE clause can appear on the TARGET EXIT DATA directive
   !$omp target exit data map(from:a) device(0) device(1)
 
   !ERROR: Only the FROM, RELEASE, DELETE map types are permitted for MAP clauses on the TARGET EXIT DATA directive
   !$omp target exit data map(to:a)
 
   !$omp target
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
   !$omp distribute
   do i = 1, N
      a = 3.14
   enddo
   !$omp end distribute
   !$omp end target
 
   !$omp target
+  !$omp teams
+  !$omp distribute
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute
+  !$omp end teams
+  !$omp end target
+
+  !$omp target
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+  !ERROR: At most one COLLAPSE clause can appear on the DISTRIBUTE directive
+  !$omp distribute collapse(2) collapse(3)
+  do i = 1, N
+     do j = 1, N
+        do k = 1, N
+           a = 3.14
+        enddo
+     enddo
+  enddo
+  !$omp end distribute
+  !$omp end target
+
+  !$omp target
+  !$omp teams
   !ERROR: At most one COLLAPSE clause can appear on the DISTRIBUTE directive
   !$omp distribute collapse(2) collapse(3)
   do i = 1, N
      do j = 1, N
         do k = 1, N
            a = 3.14
         enddo
      enddo
   enddo
   !$omp end distribute
+  !$omp end teams
   !$omp end target
 
   !$omp target
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
   !$omp distribute dist_schedule(static, 2)
   do i = 1, N
      a = 3.14
   enddo
   !$omp end distribute
   !$omp end target
 
   !$omp target
+  !$omp teams
+  !$omp distribute dist_schedule(static, 2)
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute
+  !$omp end teams
+  !$omp end target
+
+  !$omp target
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
   !ERROR: At most one DIST_SCHEDULE clause can appear on the DISTRIBUTE directive
   !$omp distribute dist_schedule(static, 2) dist_schedule(static, 3)
   do i = 1, N
      a = 3.14
   enddo
   !$omp end distribute
   !$omp end target
 
+  !$omp target
+  !$omp teams
+  !ERROR: At most one DIST_SCHEDULE clause can appear on the DISTRIBUTE directive
+  !$omp distribute dist_schedule(static, 2) dist_schedule(static, 3)
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute
+  !$omp end teams
+  !$omp end target
+
 end program main

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

+! RUN: %S/test_errors.sh %s %t %flang -fopenmp
+! Check OpenMP clause validity for the following directives:
+!     2.10 Device constructs
+program main
+
+  real(8) :: arrayA(256), arrayB(256)
+  integer :: N
+
+  arrayA = 1.414
+  arrayB = 3.14
+  N = 256
+
+  !$omp task
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+  !$omp distribute 
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute
+  !$omp end task
+
+  !$omp teams
+   do i = 1, N
+      !ERROR: Only `DISTRIBUTE` or `PARALLEL` regions are allowed to be strictly nested inside `TEAMS` region.
+      !$omp task
+      do k = 1, N
+         a = 3.14
+      enddo
+      !$omp end task
+   enddo
+   !$omp end teams
+
+   !$omp teams
+   do i = 1, N
+      !$omp parallel
+      do k = 1, N
+         a = 3.14
+      enddo
+      !$omp end parallel
+   enddo
+   !$omp end teams
+
+  !$omp parallel
+  !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+  !$omp distribute 
+  do i = 1, N
+     a = 3.14
+  enddo
+  !$omp end distribute
+  !$omp end parallel
+
+  !$omp teams
+   !ERROR: Only `DISTRIBUTE` or `PARALLEL` regions are allowed to be strictly nested inside `TEAMS` region.
+   !$omp target
+      !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+      !$omp distribute 
+      do i = 1, 10
+         j = j + 1
+      end do
+      !$omp end distribute
+   !$omp end target
+  !$omp end teams
+
+  !$omp teams 
+   !$omp parallel
+   do k = 1,10
+      print *, "hello"
+   end do
+   !$omp end parallel
+   !$omp distribute firstprivate(a)
+   do i = 1, 10
+      j = j + 1
+   end do
+   !$omp end distribute
+  !$omp end teams
+
+  !$omp teams 
+      !ERROR: Only `DISTRIBUTE` or `PARALLEL` regions are allowed to be strictly nested inside `TEAMS` region.
+      !$omp task
+      do k = 1,10
+         print *, "hello"
+      end do
+      !$omp end task
+      !$omp distribute firstprivate(a)
+      do i = 1, 10
+         j = j + 1
+      end do
+      !$omp end distribute
+  !$omp end teams
+
+  !$omp task 
+      !$omp parallel
+      do k = 1,10
+         print *, "hello"
+      end do
+      !$omp end parallel
+      !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+      !$omp distribute firstprivate(a)
+      do i = 1, 10
+         j = j + 1
+      end do
+      !$omp end distribute
+  !$omp end task
+
+
+end program main