[flang][OpenMP] Add semantic check for occurrence of constructs nested inside a SIMD region

[arnamoy10]

Summary

This patch adds the semantic check for occurrence of constructs nested inside a SIMD region.

To make the test generalized for multiple constructs, to avoid repeats, the check is done while Enter()-ing the top level OpenMPConstruct

Also adds a test case.

It also modifies an existing test case for nested ordered clause check because with the current check introduced, the previous test shows two sema check failures instead of one.

PS: The check is currently supporting SIMD loops, but not declare SIMD directive.

[clementval]

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

191

It is also considered nested is it's bigger than 1. So it should be >= 1?

311

Apply clang-format please.

[arnamoy10]

1. Modifying the if-check to trigger the test.
2. Clang-formatting.

[kiranchandramohan]

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

308

Nit: use braced initializers.

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

76

Which revision of the standard are you following here?

4.5:
An ordered construct with the simd clause is the only OpenMP construct that can be encountered during execution of a simd region.

5.0, 5.1:
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.

[arnamoy10]

Updating the checks (and test cases) to be compliant with the 5.0/5.1 standard.

[arnamoy10]

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

76

Thanks for the comment. Initially the check I implemented was based on gfortran (latest) error messages. I updated the checks to reflect the OpenMP 5.0/5.1 standard.

[kiranchandramohan]

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

192

Can you check for the SIMD construct before calling this function?

313

Nit: use braced initializer.

314

Will simd be always the parent construct? Are there cases where it can become an ancestor but not the immediate parent?

[arnamoy10]

Addressing review comment to check for SIMD construct in the whole context chain instead of only searching at the immediate parent level.

Also added a test accordingly.

[kiranchandramohan]

Have a question here.

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

303

Nit: A TODO for the loop construct.

321

Here ordered is a block-directive. But below it is a standalone directive. Is it parsed both ways?

335

Here ordered is a standalone directive. See above.

[arnamoy10]

Small change in function docstring as per reviewers suggestion.

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

303

Thanks, added.

321

Yes, according to here,

If the depend clause is specified, the ordered construct is a stand-alone directive.

[kiranchandramohan]

LGTM. I have a Nit comment about a counter. Leaving the decision to you on whether to consider it or not.

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

224 ↗

(On Diff #341246)

Nit: gven -> given

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

192

Nit: Would it be better to keep a counter which counts the constructs in the simdSet? The counter can be incremented on Entering a simdSet construct and decremented on leaving. If counter is greater than 1 then FindDirInWholeContext is true. This way we avoid going through all the entries in dirContext_ everytime we see a construct.

311

Nit: not -> note

[arnamoy10]

Using a variable to identify SIMD nesting as per suggestion to avoid expensive traversals.

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

192

Thanks for the suggestion. Updated

[kiranchandramohan]

LGTM.

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

323 ↗

(On Diff #343150)

Nit: Braced initialization.

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

private:
   void SetLoopInfo(const parser::OpenMPLoopConstruct &x);
   void CheckIsLoopIvPartOfClause(
       llvmOmpClause clause, const parser::OmpObjectList &ompObjectList);
   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 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 CheckReductionArraySection(const parser::OmpObjectList &ompObjectList);
   void CheckIntentInPointerAndDefinable(

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 &) {
-  // 2.8.1 TODO: Simd Construct with Ordered Construct Nesting check
+void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
+  // Simd Construct with Ordered Construct Nesting check
+  // We cannot use CurrentDirectiveIsNested() here because
+  // PushContextAndClauseSets() has not been called yet, it is
+  // called individually for each construct.  Therefore a
+  // dirContext_ size `1` means the current construct is nested
+  if (dirContext_.size() >= 1) {
+    CheckSIMDNest(x);
+  }
 }
 
 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
   if (const auto &endLoopDir{
for (const parser::DoConstruct *loop{&*loopConstruct}; loop;) {
       const auto &block{std::get<parser::Block>(loop->t)};
       const auto it{block.begin()};
       loop = it != block.end() ? parser::Unwrap<parser::DoConstruct>(*it)
                                : nullptr;
     }
   }
 }
 
+void OmpStructureChecker::CheckSIMDNest(const parser::OpenMPConstruct &c) {
+  // Check the following:
+  //  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.
+
+  // Check if the parent context has the SIMD clause
+  // Please not that we use GetContext() instead of GetContextParent()
+  // because PushContextAndClauseSets() has not been called on the
+  // current context yet.
+  // TODO: Check for declare simd regions.
+  bool eligibleSIMD = false;
+  if (llvm::omp::simdSet.test(GetContext().directive)) {
+    std::visit(Fortran::common::visitors{
+                   // Allow `!$OMP ORDERED SIMD`
+                   [&](const parser::OpenMPBlockConstruct &c) {
+                     const auto &beginBlockDir{
+                         std::get<parser::OmpBeginBlockDirective>(c.t)};
+                     const auto &beginDir{
+                         std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
+                     if (beginDir.v == llvm::omp::Directive::OMPD_ordered) {
+                       const auto &clauses{
+                           std::get<parser::OmpClauseList>(beginBlockDir.t)};
+                       for (const auto &clause : clauses.v) {
+                         if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
+                           eligibleSIMD = true;
+                           break;
+                         }
+                       }
+                     }
+                   },
+                   [&](const parser::OpenMPSimpleStandaloneConstruct &c) {
+                     const auto &dir{
+                         std::get<parser::OmpSimpleStandaloneDirective>(c.t)};
+                     if (dir.v == llvm::omp::Directive::OMPD_ordered) {
+                       const auto &clauses{
+                           std::get<parser::OmpClauseList>(c.t)};
+                       for (const auto &clause : clauses.v) {
+                         if (std::get_if<parser::OmpClause::Simd>(&clause.u)) {
+                           eligibleSIMD = true;
+                           break;
+                         }
+                       }
+                     }
+                   },
+                   // Allowing SIMD construct
+                   [&](const parser::OpenMPLoopConstruct &c) {
+                     const auto &beginLoopDir{
+                         std::get<parser::OmpBeginLoopDirective>(c.t)};
+                     const auto &beginDir{
+                         std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
+                     if ((beginDir.v == llvm::omp::Directive::OMPD_simd) ||
+                         (beginDir.v == llvm::omp::Directive::OMPD_do_simd)) {
+                       eligibleSIMD = true;
+                     }
+                   },
+                   [&](const parser::OpenMPAtomicConstruct &c) {
+                     // Allow `!$OMP ATOMIC`
+                     eligibleSIMD = true;
+                   },
+                   [&](const auto &c) {},
+               },
+        c.u);
+    if (!eligibleSIMD) {
+      context_.Say(parser::FindSourceLocation(c),
+          "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."_err_en_US);
+    }
+  }
+}
+
 std::int64_t OmpStructureChecker::GetOrdCollapseLevel(
     const parser::OpenMPLoopConstruct &x) {
   const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
   const auto &clauseList{std::get<parser::OmpClauseList>(beginLoopDir.t)};
   std::int64_t orderedCollapseLevel{1};
   std::int64_t orderedLevel{0};
   std::int64_t collapseLevel{0};
 

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

+! RUN: %S/test_errors.sh %s %t %flang_fc1 -fopenmp
+! OpenMP Version 4.5
+! Various checks with the nesting of SIMD construct
+
+SUBROUTINE NESTED_GOOD(N)
+  INTEGER N, I, J, K, A(10), B(10)
+  !$OMP SIMD
+  DO I = 1,N
+    !$OMP ATOMIC
+    K =  K + 1
+    IF (I <= 10) THEN
+      !$OMP ORDERED SIMD
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END ORDERED
+    ENDIF
+  END DO
+  !$OMP END SIMD
+
+  !$OMP SIMD
+  DO I = 1,N
+    IF (I <= 10) THEN
+      !$OMP SIMD
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END SIMD
+    ENDIF
+  END DO
+  !$OMP END SIMD
+END SUBROUTINE NESTED_GOOD
+
+SUBROUTINE NESTED_BAD(N)
+  INTEGER N, I, J, K, A(10), B(10)
+
+  !$OMP SIMD
+  DO I = 1,N
+    !$OMP ATOMIC
+    K =  K + 1
+    IF (I <= 10) THEN
+      !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.
+      !$omp task 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end task
+      !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.
+      !$omp teams 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end teams
+      !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.
+      !$omp target 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end target
+      !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.
+      !$OMP DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END DO
+      !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.
+      !$OMP PARALLEL DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END PARALLEL DO
+    ENDIF
+  END DO
+  !$OMP END SIMD
+
+  !$OMP DO SIMD
+  DO I = 1,N
+    !$OMP ATOMIC
+    K =  K + 1
+    IF (I <= 10) THEN
+      !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.
+      !$omp task 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end task
+      !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.
+      !$omp teams 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end teams
+      !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.
+      !$omp target 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end target
+      !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.
+      !$OMP DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END DO
+      !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.
+      !$OMP PARALLEL DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END PARALLEL DO
+    ENDIF
+  END DO
+  !$OMP END DO SIMD
+
+  !$OMP PARALLEL DO SIMD
+  DO I = 1,N
+    !$OMP ATOMIC
+    K =  K + 1
+    IF (I <= 10) THEN
+      !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.
+      !$omp task 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end task
+      !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.
+      !$omp teams 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end teams
+      !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.
+      !$omp target 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end target
+      !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.
+      !$OMP DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END DO
+      !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.
+      !$OMP PARALLEL DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END PARALLEL DO
+    ENDIF
+  END DO
+  !$OMP END PARALLEL DO SIMD
+
+  !$OMP TARGET SIMD
+  DO I = 1,N
+    !$OMP ATOMIC
+    K =  K + 1
+    IF (I <= 10) THEN
+      !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.
+      !$omp task 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end task
+      !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.
+      !$omp teams 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end teams
+      !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.
+      !$omp target 
+      do J = 1, N
+        K = 2
+      end do
+      !$omp end target
+      !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.
+      !$OMP DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END DO
+      !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.
+      !$OMP PARALLEL DO
+      DO J = 1,N
+        A(J) = J
+      END DO
+      !$OMP END PARALLEL DO
+    ENDIF
+  END DO
+  !$OMP END TARGET SIMD
+
+
+END SUBROUTINE NESTED_BAD

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

 END SUBROUTINE ORDERED_GOOD
 
 SUBROUTINE ORDERED_BAD(N)
   INTEGER N, I, A(10), B(10), C(10)
 
   !$OMP DO SIMD
   DO I = 1,N
     IF (I <= 10) THEN
+      !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: The ORDERED clause must be present on the loop construct if any ORDERED region ever binds to a loop region arising from the loop construct.
       !$OMP ORDERED 
       CALL WORK(I)
       !$OMP END ORDERED
     ENDIF
   END DO
   !$OMP END DO SIMD