[flang][OpenMP][Semantics] improve semantic checks for array sections

[tblah]

I'm not sure why strides were not allowed in array sections: the stride is explicitly allowed by the standard from the first version where array sections were introduced. The limitation is that the stride must not be negative.

Here I have added the check for a negative stride and updated the test for a zero length section to take account of the stride.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-semantics

Author: Tom Eccles (tblah)

Changes

I'm not sure why strides were not allowed in array sections: the stride is explicitly allowed by the standard from the first version where array sections were introduced. The limitation is that the stride must not be negative.

Here I have added the check for a negative stride and updated the test for a zero length section to take account of the stride.

---

Full diff: https://github.com/llvm/llvm-project/pull/132230.diff

2 Files Affected:

• (modified) flang/lib/Semantics/check-omp-structure.cpp (+29-16)
• (modified) flang/test/Semantics/OpenMP/depend01.f90 (+9-2)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9c9d666b5e8d5..57de3de8b8ef7 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -5304,27 +5304,40 @@ void OmpStructureChecker::CheckArraySection(
       if (const auto *triplet{
               std::get_if<parser::SubscriptTriplet>(&subscript.u)}) {
         if (std::get<0>(triplet->t) && std::get<1>(triplet->t)) {
+          std::optional<int64_t> strideVal{std::nullopt};
+          if (std::get<2>(triplet->t)) {
+            const auto &strideExpr{std::get<2>(triplet->t)};
+            if (strideExpr) {
+              // OpenMP 6.0 Section 5.2.5: Array Sections
+              // Restrictions: if a stride expression is specified it must be
+              // positive. A stride of 0 doesn't make sense.
+              strideVal = GetIntValue(strideExpr);
+              if (strideVal && *strideVal < 1) {
+                context_.Say(GetContext().clauseSource,
+                    "'%s' in %s clause must have a positive stride"_err_en_US,
+                    name.ToString(),
+                    parser::ToUpperCaseLetters(getClauseName(clause).str()));
+              }
+            }
+          }
           const auto &lower{std::get<0>(triplet->t)};
           const auto &upper{std::get<1>(triplet->t)};
           if (lower && upper) {
             const auto lval{GetIntValue(lower)};
             const auto uval{GetIntValue(upper)};
-            if (lval && uval && *uval < *lval) {
-              context_.Say(GetContext().clauseSource,
-                  "'%s' in %s clause"
-                  " is a zero size array section"_err_en_US,
-                  name.ToString(),
-                  parser::ToUpperCaseLetters(getClauseName(clause).str()));
-              break;
-            } else if (std::get<2>(triplet->t)) {
-              const auto &strideExpr{std::get<2>(triplet->t)};
-              if (strideExpr) {
-                if (clause == llvm::omp::Clause::OMPC_depend) {
-                  context_.Say(GetContext().clauseSource,
-                      "Stride should not be specified for array section in "
-                      "DEPEND "
-                      "clause"_err_en_US);
-                }
+            if (lval && uval) {
+              int64_t sectionLen = *uval - *lval;
+              if (strideVal) {
+                sectionLen = sectionLen / *strideVal;
+              }
+
+              if (sectionLen < 1) {
+                context_.Say(GetContext().clauseSource,
+                    "'%s' in %s clause"
+                    " is a zero size array section"_err_en_US,
+                    name.ToString(),
+                    parser::ToUpperCaseLetters(getClauseName(clause).str()));
+                break;
               }
             }
           }
diff --git a/flang/test/Semantics/OpenMP/depend01.f90 b/flang/test/Semantics/OpenMP/depend01.f90
index 29468f4358855..19fcfbf64bebd 100644
--- a/flang/test/Semantics/OpenMP/depend01.f90
+++ b/flang/test/Semantics/OpenMP/depend01.f90
@@ -17,8 +17,15 @@ program omp_depend
   a(2:1) = b(2, 2)
   !$omp end task
 
-  !ERROR: Stride should not be specified for array section in DEPEND clause
-  !$omp task shared(x) depend(in: a(5:10:1))
+  !ERROR: 'a' in DEPEND clause must have a positive stride
+  !ERROR: 'b' in DEPEND clause must have a positive stride
+  !ERROR: 'b' in DEPEND clause is a zero size array section
+  !$omp task shared(x) depend(in: a(10:5:-1)) depend(in: b(5:10:-1))
+  print *, a(5:10), b
+  !$omp end task
+
+  !ERROR: 'a' in DEPEND clause is a zero size array section
+  !$omp task shared(x) depend(in: a(1:5:10))
   print *, a(5:10), b
   !$omp end task
 

[Leporacanthicus]

Overall looks OK.

[Leporacanthicus]

LGTM