[flang][OpenMP][FIX] Fix the worksharing nesting check with inclusion of more constructs to cover entire workshare family.

[arnamoy10]

Summary

Existing code for checking the nesting of workshare regions was not covering all the possible constructs that can be workshare constructs. This patch creates a new group of constructs for workshare constructs.

It extends the check to check during sections constructs as well.

Also adds a few test cases to verify the check.

[kiranchandramohan]

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

44–45

Not sure why parallel is here. Remove if no test fails.

71–75

Why do a few of these have the llvm::omp prefix but others do not?

76

Nit: empty line.

[arnamoy10]

Addressing comments.

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

44–45

Done, thanks

71–75

typo, thanks for the catch

[kiranchandramohan]

LGTM. Have two Nit comments.

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

363–371

Nit: Can a call to HasInvalidWorksharingNesting be added here for the worksharing construct with name workshare? And add one test.

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

58

Nit: Can you construct this set using the doSet?

[arnamoy10]

Addressing comments.

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

363–371

Sure, done and added test case

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

58

Thanks, done!

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

static OmpDirectiveSet parallelSet{Directive::OMPD_distribute_parallel_do,
     Directive::OMPD_parallel_do, Directive::OMPD_parallel_do_simd,
     Directive::OMPD_parallel_sections, Directive::OMPD_parallel_workshare,
     Directive::OMPD_target_parallel, Directive::OMPD_target_parallel_do,
     Directive::OMPD_target_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_parallel_do,
     Directive::OMPD_target_teams_distribute_parallel_do_simd,
     Directive::OMPD_teams_distribute_parallel_do,
     Directive::OMPD_teams_distribute_parallel_do_simd};
 static OmpDirectiveSet doSet{Directive::OMPD_distribute_parallel_do,
-    Directive::OMPD_distribute_parallel_do_simd, Directive::OMPD_parallel,
-    Directive::OMPD_parallel_do, Directive::OMPD_parallel_do_simd,
-    Directive::OMPD_do, Directive::OMPD_do_simd,
-    Directive::OMPD_target_parallel_do, Directive::OMPD_target_parallel_do_simd,
+    Directive::OMPD_distribute_parallel_do_simd, Directive::OMPD_parallel_do,
+    Directive::OMPD_parallel_do_simd, Directive::OMPD_do,
+    Directive::OMPD_do_simd, Directive::OMPD_target_parallel_do,
+    Directive::OMPD_target_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_parallel_do,
     Directive::OMPD_target_teams_distribute_parallel_do_simd,
     Directive::OMPD_teams_distribute_parallel_do,
     Directive::OMPD_teams_distribute_parallel_do_simd};
 static OmpDirectiveSet doSimdSet{Directive::OMPD_distribute_parallel_do_simd,
     Directive::OMPD_parallel_do_simd, Directive::OMPD_do_simd,
     Directive::OMPD_target_parallel_do_simd,
     Directive::OMPD_target_teams_distribute_parallel_do_simd,
     Directive::OMPD_teams_distribute_parallel_do_simd};
+static OmpDirectiveSet workShareSet{
+    OmpDirectiveSet{Directive::OMPD_workshare,
+        Directive::OMPD_parallel_workshare, Directive::OMPD_parallel_sections,
+        Directive::OMPD_sections, Directive::OMPD_single} |
+    doSet};
 static OmpDirectiveSet taskloopSet{
     Directive::OMPD_taskloop, Directive::OMPD_taskloop_simd};
 static OmpDirectiveSet targetSet{Directive::OMPD_target,
     Directive::OMPD_target_parallel, Directive::OMPD_target_parallel_do,
     Directive::OMPD_target_parallel_do_simd, Directive::OMPD_target_simd,
     Directive::OMPD_target_teams, Directive::OMPD_target_teams_distribute,
     Directive::OMPD_target_teams_distribute_simd};
 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 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 OmpDirectiveSet nestedWorkshareErrSet{
+    OmpDirectiveSet{Directive::OMPD_task, Directive::OMPD_taskloop,
+        Directive::OMPD_critical, Directive::OMPD_ordered,
+        Directive::OMPD_atomic, Directive::OMPD_master} |
+    workShareSet};
 static OmpClauseSet privateSet{
     Clause::OMPC_private, Clause::OMPC_firstprivate, Clause::OMPC_lastprivate};
 static OmpClauseSet privateReductionSet{
     OmpClauseSet{Clause::OMPC_reduction} | privateSet};
 } // namespace omp
 } // namespace llvm
 
 namespace Fortran::semantics {

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

if (beginDir.v == llvm::omp::Directive::OMPD_do) {
     //                    lastprivate-clause |
     //                    linear-clause |
     //                    reduction-clause |
     //                    schedule-clause |
     //                    collapse-clause |
     //                    ordered-clause
 
     // nesting check
-    HasInvalidWorksharingNesting(beginDir.source,
-        {llvm::omp::Directive::OMPD_do, llvm::omp::Directive::OMPD_sections,
-            llvm::omp::Directive::OMPD_single,
-            llvm::omp::Directive::OMPD_workshare,
-            llvm::omp::Directive::OMPD_task,
-            llvm::omp::Directive::OMPD_taskloop,
-            llvm::omp::Directive::OMPD_critical,
-            llvm::omp::Directive::OMPD_ordered,
-            llvm::omp::Directive::OMPD_atomic,
-            llvm::omp::Directive::OMPD_master});
+    HasInvalidWorksharingNesting(
+        beginDir.source, llvm::omp::nestedWorkshareErrSet);
   }
   SetLoopInfo(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);
   }
void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   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);
 
-  // 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())
     CheckIfDoOrderedClause(beginDir);
 
   CheckNoBranching(block, beginDir.v, beginDir.source);
 
   switch (beginDir.v) {
   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
+    // regions checks.
+    HasInvalidWorksharingNesting(
+        beginDir.source, llvm::omp::nestedWorkshareErrSet);
     break;
   default:
     break;
   }
 }
 
 void OmpStructureChecker::CheckIfDoOrderedClause(
     const parser::OmpBlockDirective &blkDirective) {
void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
   const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir.t)};
   CheckMatching<parser::OmpSectionsDirective>(beginDir, endDir);
 
   PushContextAndClauseSets(beginDir.source, beginDir.v);
   const auto &sectionBlocks{std::get<parser::OmpSectionBlocks>(x.t)};
   for (const auto &block : sectionBlocks.v) {
     CheckNoBranching(block, beginDir.v, beginDir.source);
   }
+  HasInvalidWorksharingNesting(
+      beginDir.source, llvm::omp::nestedWorkshareErrSet);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPSectionsConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OmpEndSectionsDirective &x) {
   const auto &dir{std::get<parser::OmpSectionsDirective>(x.t)};

flang/test/Semantics/omp-do05.f90

do i=1,10
     !$omp single
     do j=1,10
       print *,"hello"
     end do
     !$omp end single
   end do
   !$omp end do
 
+  !$omp parallel do
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end parallel do
+
+  !$omp parallel do simd
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end parallel do simd
+
+  !$omp distribute parallel do
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end distribute parallel do
+
+  !$omp distribute parallel do simd
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end distribute parallel do simd
+
+  !$omp target parallel do 
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end target parallel do
+
+  !$omp target parallel do simd
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end target parallel do simd
+
+  !$omp target teams distribute parallel do
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end target teams distribute parallel do
+
+    !$omp target teams distribute parallel do simd
+  do i=1,10
+    !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
+    !$omp single
+    do j=1,10
+      print *,"hello"
+    end do
+    !$omp end single
+  end do
+  !$omp end target teams distribute parallel do simd
+
   !$omp do
   do i=1,10
     !$omp task
     do j=1,10
       !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
       !$omp single
       do k=1,10
         print *,"hello"

flang/test/Semantics/omp-workshare01.f90

subroutine workshare(aa, bb, cc, dd, ee, ff, n)
   !ERROR: The structured block in a WORKSHARE construct may consist of only SCALAR or ARRAY assignments, FORALL or WHERE statements, FORALL, WHERE, ATOMIC, CRITICAL or PARALLEL constructs
   !ERROR: OpenMP constructs enclosed in WORKSHARE construct may consist of ATOMIC, CRITICAL or PARALLEL constructs only
   !$omp workshare
   do i = 1, n
     print *, "omp workshare"
   end do
 
   !$omp critical
+  !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
   !$omp single
   aa = bb
   !$omp end single
   !$omp end critical
 
   !$omp parallel
   !$omp single
   cc = dd
   !$omp end single
   !$omp end parallel
 
   ee = ff
   !$omp end workshare
 
 end subroutine workshare

flang/test/Semantics/omp-workshare04.f90

subroutine omp_workshare(aa, bb, cc, dd, ee, ff, n)
   !$omp workshare
   p => t
 
   !$omp parallel
   cc = dd
   !$omp end parallel
 
   !ERROR: OpenMP constructs enclosed in WORKSHARE construct may consist of ATOMIC, CRITICAL or PARALLEL constructs only
+  !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
   !$omp parallel workshare
+  !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
   !$omp single
   ee = ff
   !$omp end single
   !$omp end parallel workshare
 
   where (aa .ne. 0) cc = bb / aa
 
   where (b .lt. 2) b = sum(a)

flang/test/Semantics/omp-workshare05.f90

program omp_workshare
   !$omp parallel
   p => t
   a = l
   !$omp single
   ee = ff
   !$omp end single
   !$omp end parallel
 
+  !ERROR: A worksharing region may not be closely nested inside a worksharing, explicit task, taskloop, critical, ordered, atomic, or master region
   !$omp parallel sections
   !$omp section
   aa = my_func()
   !$omp end parallel sections
 
   !$omp parallel do
   do i = 1, 10
     b(i) = my_func() + i