[flang][OpenMP] Add semantic check for close nesting of `master` regions

[arnamoy10]

Summary

This patch implements the following semantic check:

A master region may not be closely nested inside a work-sharing, loop, atomic, task, or taskloop region.

Adds a test case and also modifies a couple of existing test cases to include the check.

[kiranchandramohan]

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

504

Nit: Can we check whether it is the master construct before calling the check function?

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

91

Not for this patch. It will be good to generate these Sets from the OMP.td file.

92

Why is parallelSet here? Master is allowed inside parallel.

[arnamoy10]

Addressing reviewers comments.

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

504

Done, thanks!

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

92

Good catch, updated the code and removed it. Now the set is inspired from here

[kiranchandramohan]

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

92

Can you add all those combined constructs also. Those combined constructs which will lead to a nested master violating this constraint, like parallel do.

[arnamoy10]

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

92

Can you please review this patch? Then I should be able to use the combined constructs for worksharing from the patch here.

[kiranchandramohan]

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

92

Apologies, I missed it completely.

[arnamoy10]

Updating the patch to use the recent merged workShareSet (that included tests for combined constructs for worksharing loops).

[kiranchandramohan]

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

98

Is a master construct not allowed in a master construct?

[arnamoy10]

Removing master construct from the error set.

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

98

Thanks for the catch, just updated the code.

[kiranchandramohan]

LGTM.

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

static OmpDirectiveSet teamSet{Directive::OMPD_teams,
     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 OmpDirectiveSet nestedWorkshareErrSet{
     OmpDirectiveSet{Directive::OMPD_task, Directive::OMPD_taskloop,
         Directive::OMPD_critical, Directive::OMPD_ordered,
         Directive::OMPD_atomic, Directive::OMPD_master} |
     workShareSet};
+static OmpDirectiveSet nestedMasterErrSet{
+    OmpDirectiveSet{llvm::omp::Directive::OMPD_atomic} | taskGeneratingSet |
+    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 {
private:
   void CheckCycleConstraints(const parser::OpenMPLoopConstruct &x);
   void CheckSIMDNest(const parser::OpenMPConstruct &x);
   std::int64_t GetOrdCollapseLevel(const parser::OpenMPLoopConstruct &x);
   void CheckIfDoOrderedClause(const parser::OmpBlockDirective &blkDirectiv);
   bool CheckReductionOperators(const parser::OmpClause::Reduction &);
   bool CheckIntrinsicOperator(
       const parser::DefinedOperator::IntrinsicOperator &);
   void CheckReductionTypeList(const parser::OmpClause::Reduction &);
+  void CheckMasterNesting(const parser::OpenMPBlockConstruct &x);
   void CheckReductionArraySection(const parser::OmpObjectList &ompObjectList);
   void CheckIntentInPointerAndDefinable(
       const parser::OmpObjectList &, const llvm::omp::Clause);
   void CheckArraySection(const parser::ArrayElement &arrayElement,
       const parser::Name &name, const llvm::omp::Clause clause);
   void CheckMultipleAppearanceAcrossContext(
       const parser::OmpObjectList &ompObjectList);
   const parser::OmpObjectList *GetOmpObjectList(const parser::OmpClause &);
 };
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_

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

void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   const parser::Block &block{std::get<parser::Block>(x.t)};
 
   CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
 
   PushContextAndClauseSets(beginDir.source, beginDir.v);
 
   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);
+    }
   }
 
   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::CheckMasterNesting(
+    const parser::OpenMPBlockConstruct &x) {
+  // A MASTER region may not be `closely nested` inside a worksharing, loop,
+  // task, taskloop, or atomic region.
+  // TODO:  Expand the check to include `LOOP` construct as well when it is
+  // supported.
+  if (IsCloselyNestedRegion(llvm::omp::nestedMasterErrSet)) {
+    context_.Say(parser::FindSourceLocation(x),
+        "`MASTER` region may not be closely nested inside of `WORKSHARING`, "
+        "`LOOP`, `TASK`, `TASKLOOP`,"
+        " or `ATOMIC` region."_err_en_US);
+  }
+}
+
 void OmpStructureChecker::CheckIfDoOrderedClause(
     const parser::OmpBlockDirective &blkDirective) {
   if (blkDirective.v == llvm::omp::OMPD_ordered) {
     // Loops
     if (llvm::omp::doSet.test(GetContextParent().directive) &&
         !FindClauseParent(llvm::omp::Clause::OMPC_ordered)) {
       context_.Say(blkDirective.source,
           "The ORDERED clause must be present on the loop"

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

+! RUN: %S/test_errors.sh %s %t %flang_fc1 -fopenmp
+! OpenMP Version 4.5
+! Various checks with the nesting of MASTER construct
+
+program omp_nest_master
+  integer i, k, j
+  k = 0;
+
+  !$omp do
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+    !$omp master
+    j = j -1
+    !$omp end master
+  end do
+
+  !$omp sections 
+  !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+    !$omp master
+    do i = 1, 10
+      k = k + 1
+    end do
+    !$omp end master
+  !$omp end sections
+
+  !$omp single 
+  !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+    !$omp master
+    do i = 1, 10
+      k = k + 1
+    end do
+    !$omp end master
+  !$omp end single
+
+
+
+  !$omp task
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+    !$omp master
+    j = j -1
+    !$omp end master
+  end do
+  !$omp end task
+
+  !$omp taskloop
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+    !$omp master
+    j = j -1
+    !$omp end master
+  end do
+  !$omp end taskloop
+
+  !$omp target parallel do simd
+  do i = 1, 10
+    k = k + 1
+    !ERROR: The only OpenMP constructs that can be encountered during execution of a 'SIMD' region are the `ATOMIC` construct, the `LOOP` construct, the `SIMD` construct and the `ORDERED` construct with the `SIMD` clause.
+    !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+    !$omp master
+    j = j -1
+    !$omp end master
+  end do
+  !$omp end target parallel do simd
+
+  !$omp critical
+  do i = 1, 10
+    k = k + 1
+    !$omp master
+    j = j -1
+    !$omp end master
+  end do
+  !$omp end critical
+
+  !$omp ordered
+  do i = 1, 10
+    k = k + 1
+    !$omp master
+    j = j -1
+    !$omp end master
+  end do
+  !$omp end ordered
+
+  !$omp ordered
+  do i = 1, 10
+    !$omp teams
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !$omp master
+      j = j -1
+      !$omp end master
+    end do
+    !$omp end distribute
+    !$omp end teams
+  end do
+  !$omp end ordered
+
+  !$omp critical
+  do i = 1, 10
+    !$omp teams
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !$omp master
+      j = j -1
+      !$omp end master
+    end do
+    !$omp end distribute
+    !$omp end teams
+  end do
+  !$omp end critical
+
+  !$omp taskloop
+  do i = 1, 10
+    !$omp teams
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+      !$omp master
+      j = j -1
+      !$omp end master
+    end do
+    !$omp end distribute
+    !$omp end teams
+  end do
+  !$omp end taskloop
+
+  !$omp task
+  do i = 1, 10
+    !$omp teams
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
+      !$omp master
+      j = j -1
+      !$omp end master
+    end do
+    !$omp end distribute
+    !$omp end teams
+  end do
+  !$omp end task
+
+end program omp_nest_master

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

DO I = 1,N
       CALL WORK(I)
       !$OMP END ORDERED
     END DO
     !$OMP END MASTER
   !$OMP END ORDERED
 
   !$OMP TASK  
     C =  C - A * B
+    !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
     !$OMP MASTER
     DO I = 1,N
       !ERROR: `ORDERED` region may not be closely nested inside of `CRITICAL`, `ORDERED`, explicit `TASK` or `TASKLOOP` region.
       !$OMP ORDERED 
       CALL WORK(I)
       !$OMP END ORDERED
     END DO
     !$OMP END MASTER
   !$OMP END TASK
 
   !$OMP TASKLOOP
   DO J= 1,N  
     C =  C - A * B
+    !ERROR: `MASTER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`, or `ATOMIC` region.
     !$OMP MASTER
     DO I = 1,N
       !ERROR: `ORDERED` region may not be closely nested inside of `CRITICAL`, `ORDERED`, explicit `TASK` or `TASKLOOP` region.
       !$OMP ORDERED 
       CALL WORK(I)
       !$OMP END ORDERED
     END DO
     !$OMP END MASTER
   END DO
   !$OMP END TASKLOOP
 
 END SUBROUTINE ORDERED_BAD