[flang][OpenMP] Add semantic checks for occurrence of nested Barrier regions

[arnamoy10]

Summary

This patch adds the following nesting check for barrier constructs:

A barrier region may not be closely nested inside a worksharing, loop, task, taskloop, critical, ordered, atomic, or master region.

Also adds a test case for the check,

[kiranchandramohan]

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

808

Definition of closely nested region is
"A region nested inside another region with no parallel region nested between them."

Does this check capture this definition correctly?

815–828

Nit: Remove commented code.

[arnamoy10]

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

808

Thanks for pointing in out.

1. Just double checking -- is parallel region a region enclosed by the !$omp parallel construct?
2. IIUC, this check (by which my code was inspired) also does not cover the definition of "close nesting"? Please confirm.

[kiranchandramohan]

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

808

1. Yes. But it is a dynamic concept, i.e it is the body of code executed during the execution.
2. Yes that code also does not cover the definition. Will appreciate a fix.

[arnamoy10]

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

808

I need one more clarification before implementing the fix.

From the definition:

A region nested inside another region with no parallel region nested between them

What does nesting "between" mean? I can think of two cases.

Case 1:

!$omp ordered
  !$omp parallel <-- Child of ordered
    !$omp barrier <-- Child of parallel, grandchild of ordered

Case 2:

!$omp ordered
  !$omp parallel <-- Child of ordered
  !$omp barrier <-- Child of ordered

Can you comment on which one of the above two cases has a barrier closely nested inside the ordered construct?

[kiranchandramohan]

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

808

Case 2 has a barrier closely nested inside ordered.

[arnamoy10]

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

808

Thanks.

So IIUC, basically if your immediate parent (say region1) is not a !$omp parallel, then you are closely nested inside region1.

If that is the case, IMO we do not need a fix. Because GetContextParent() gives us our immediate parent.

[kiranchandramohan]

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

808

Question is can there be a situation like,

construct 1
   construct 2
        barrier

The current check is only for barrier and construct 2 and you might conclude that there is no error. If construct 2 is not parallel and barrier and construct 1 is not allowed to be nested then you will miss that error, isn't it?

[arnamoy10]

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

808

I see that makes sense.

I think I can restrict the check until the grandparent in the construct stack? For example, I do not need to flag the following case:

not allowed construct 1
  allowed construct 2
    allowed construct 3
       barrier

Or do I?

[kiranchandramohan]

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

808

Why do you think so?

You might be right for real world benchmarks, since many constructs don't allow other constructs to nest. But theoretically you should.

[arnamoy10]

Updating patch to use newly landed API for checking close nesting.

Also added a few more test cases when there is violation for more than 1 level of nesting.

[kiranchandramohan]

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

94

Barriers are allowed in a parallel region. It is probably not allowed in parallel do, do simd etc.

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

45

This is wrong.

111

This and further tests below that use distribute are going to run into the following restriction when it is implemented.

"The region associated with the distribute construct must be strictly nested inside a teams region"

[arnamoy10]

Using the recently introduced OmpDirectiveSet workShareSet to do the nesting check.

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

94

Thanks, addressed.

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

45

Thanks, updated

111

Thanks, they are currently shown.

[kiranchandramohan]

LGTM.

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

 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 OmpDirectiveSet nestedBarrierErrSet{
+    OmpDirectiveSet{Directive::OMPD_critical, Directive::OMPD_ordered,
+        Directive::OMPD_atomic, Directive::OMPD_master} |
+    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 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 CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &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 &);
   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::Leave(const parser::OpenMPExecutableAllocate &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   const auto &objectList{std::get<std::optional<parser::OmpObjectList>>(x.t)};
   if (objectList)
     CheckPredefinedAllocatorRestriction(dir.source, *objectList);
   dirContext_.pop_back();
 }
 
+void OmpStructureChecker::CheckBarrierNesting(
+    const parser::OpenMPSimpleStandaloneConstruct &x) {
+  // A barrier region may not be `closely nested` inside a worksharing, loop,
+  // task, taskloop, critical, ordered, atomic, or master region.
+  // TODO:  Expand the check to include `LOOP` construct as well when it is
+  // supported.
+  if (GetContext().directive == llvm::omp::Directive::OMPD_barrier) {
+    if (IsCloselyNestedRegion(llvm::omp::nestedBarrierErrSet)) {
+      context_.Say(parser::FindSourceLocation(x),
+          "`BARRIER` region may not be closely nested inside of `WORKSHARING`, "
+          "`LOOP`, `TASK`, `TASKLOOP`,"
+          "`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region."_err_en_US);
+    }
+  }
+}
+
 void OmpStructureChecker::Enter(
     const parser::OpenMPSimpleStandaloneConstruct &x) {
   const auto &dir{std::get<parser::OmpSimpleStandaloneDirective>(x.t)};
   PushContextAndClauseSets(dir.source, dir.v);
+  CheckBarrierNesting(x);
 }
 
 void OmpStructureChecker::Leave(
     const parser::OpenMPSimpleStandaloneConstruct &) {
   dirContext_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPFlushConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_flush);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPFlushConstruct &x) {
   if (FindClause(llvm::omp::Clause::OMPC_acquire) ||
       FindClause(llvm::omp::Clause::OMPC_release) ||
       FindClause(llvm::omp::Clause::OMPC_acq_rel)) {

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

+! RUN: %S/test_errors.sh %s %t %flang_fc1 -fopenmp
+! OpenMP Version 4.5
+! Various checks with the nesting of BARRIER construct
+
+program omp_nest_barrier
+  integer i, k, j
+  k = 0;
+
+  !$omp do
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+
+  !$omp 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: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+
+  !$omp parallel do
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+
+  !$omp 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: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+
+  !$omp parallel
+  do i = 1, 10
+    k = k + 1
+    !$omp barrier
+    j = j -1
+  end do
+  !$omp end parallel
+
+  !$omp task
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+  !$omp end task
+
+  !$omp taskloop
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+  !$omp end taskloop
+
+  !$omp critical
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+  !$omp end critical
+
+  !$omp master
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+  !$omp end master
+
+  !$omp ordered
+  do i = 1, 10
+    k = k + 1
+    !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+    !$omp barrier
+    j = j -1
+  end do
+  !$omp end ordered
+
+  !$omp ordered
+  do i = 1, 10
+    !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+      !$omp barrier
+      j = j -1
+    end do
+    !$omp end distribute
+  end do
+  !$omp end ordered
+
+  !$omp master
+  do i = 1, 10
+    !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+      !$omp barrier
+      j = j -1
+    end do
+    !$omp end distribute
+  end do
+  !$omp end master
+
+  !$omp critical
+  do i = 1, 10
+    !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+      !$omp barrier
+      j = j -1
+    end do
+    !$omp end distribute
+  end do
+  !$omp end critical
+
+  !$omp taskloop
+  do i = 1, 10
+    !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+      !$omp barrier
+      j = j -1
+    end do
+    !$omp end distribute
+  end do
+  !$omp end taskloop
+
+  !$omp task
+  do i = 1, 10
+    !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+    !$omp distribute
+    do k =1, 10
+      print *, "hello"
+      !ERROR: `BARRIER` region may not be closely nested inside of `WORKSHARING`, `LOOP`, `TASK`, `TASKLOOP`,`CRITICAL`, `ORDERED`, `ATOMIC` or `MASTER` region.
+      !$omp barrier
+      j = j -1
+    end do
+    !$omp end distribute
+  end do
+  !$omp end task
+
+end program omp_nest_barrier