[flang-commits] [flang] [flang][OpenMP][Semantics] improve semantic checks for array sections (PR #132230)

Tom Eccles via flang-commits flang-commits at lists.llvm.org
Thu Mar 20 09:57:45 PDT 2025


https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/132230

>From 666e0960f1bc0f7e5d1a0afff9aa91ca9ba74c29 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 20 Mar 2025 14:50:29 +0000
Subject: [PATCH 1/2] [flang][OpenMP][Semantics] improve semantic checks for
 array sections

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.
---
 flang/lib/Semantics/check-omp-structure.cpp | 45 +++++++++++++--------
 flang/test/Semantics/OpenMP/depend01.f90    | 11 ++++-
 2 files changed, 38 insertions(+), 18 deletions(-)

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
 

>From 52c542d6c2ec5714ccf7f203eead58c1605405eb Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 20 Mar 2025 16:56:17 +0000
Subject: [PATCH 2/2] Simplify if statements

---
 flang/lib/Semantics/check-omp-structure.cpp | 23 +++++++++------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 57de3de8b8ef7..ab38a95a38871 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -5305,19 +5305,16 @@ void OmpStructureChecker::CheckArraySection(
               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()));
-              }
+          if (const auto &strideExpr = std::get<2>(triplet->t)) {
+            // 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)};



More information about the flang-commits mailing list