[llvm-branch-commits] [flang] [flang][OpenMP] Semantic checks for DOACROSS clause (PR #115397)

Krzysztof Parzyszek via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Nov 11 04:39:24 PST 2024


https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/115397

>From 4165254e8c4a7b572e741cb62d632b462537dc0f Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 5 Nov 2024 12:01:43 -0600
Subject: [PATCH 1/6] [flang][OpenMP] Semantic checks for DOACROSS clause

Keep track of loop constructs and OpenMP loop constructs that have
been entered. Use the information to validate the variables in the
SINK loop iteration vector.
---
 flang/lib/Lower/OpenMP/Clauses.cpp          |  28 ++--
 flang/lib/Semantics/check-omp-structure.cpp | 141 ++++++++++++++++++--
 flang/lib/Semantics/check-omp-structure.h   |  25 +++-
 flang/lib/Semantics/resolve-directives.cpp  |  12 +-
 flang/test/Lower/OpenMP/Todo/ordered.f90    |  20 +++
 flang/test/Semantics/OpenMP/doacross.f90    |  28 ++++
 flang/test/Semantics/OpenMP/ordered01.f90   |   4 +-
 flang/test/Semantics/OpenMP/ordered03.f90   |   2 +
 8 files changed, 230 insertions(+), 30 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/Todo/ordered.f90
 create mode 100644 flang/test/Semantics/OpenMP/doacross.f90

diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index f6633dd53f6f23..1764b3b79b4e34 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -574,20 +574,17 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
                      /*VariableCategory=*/maybeApply(convert2, t1)}};
 }
 
-Depend make(const parser::OmpClause::Depend &inp,
-            semantics::SemanticsContext &semaCtx) {
-  // inp.v -> parser::OmpDependClause
-  using wrapped = parser::OmpDependClause;
-  using Variant = decltype(Depend::u);
+Doacross makeDoacross(const parser::OmpDoacross &doa,
+                      semantics::SemanticsContext &semaCtx) {
   // Iteration is the equivalent of parser::OmpIteration
   using Iteration = Doacross::Vector::value_type; // LoopIterationT
 
-  auto visitSource = [&](const parser::OmpDoacross::Source &) -> Variant {
+  auto visitSource = [&](const parser::OmpDoacross::Source &) {
     return Doacross{{/*DependenceType=*/Doacross::DependenceType::Source,
                      /*Vector=*/{}}};
   };
 
-  auto visitSink = [&](const parser::OmpDoacross::Sink &s) -> Variant {
+  auto visitSink = [&](const parser::OmpDoacross::Sink &s) {
     using IterOffset = parser::OmpIterationOffset;
     auto convert2 = [&](const parser::OmpIteration &v) {
       auto &t0 = std::get<parser::Name>(v.t);
@@ -605,6 +602,15 @@ Depend make(const parser::OmpClause::Depend &inp,
                      /*Vector=*/makeList(s.v.v, convert2)}};
   };
 
+  return common::visit(common::visitors{visitSink, visitSource}, doa.u);
+}
+
+Depend make(const parser::OmpClause::Depend &inp,
+            semantics::SemanticsContext &semaCtx) {
+  // inp.v -> parser::OmpDependClause
+  using wrapped = parser::OmpDependClause;
+  using Variant = decltype(Depend::u);
+
   auto visitTaskDep = [&](const wrapped::TaskDep &s) -> Variant {
     auto &t0 = std::get<std::optional<parser::OmpIteratorModifier>>(s.t);
     auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
@@ -617,11 +623,11 @@ Depend make(const parser::OmpClause::Depend &inp,
                             /*LocatorList=*/makeObjects(t2, semaCtx)}};
   };
 
-  return Depend{Fortran::common::visit( //
+  return Depend{common::visit( //
       common::visitors{
           // Doacross
           [&](const parser::OmpDoacross &s) -> Variant {
-            return common::visit(common::visitors{visitSink, visitSource}, s.u);
+            return makeDoacross(s, semaCtx);
           },
           // Depend::TaskDep
           visitTaskDep,
@@ -692,8 +698,8 @@ DistSchedule make(const parser::OmpClause::DistSchedule &inp,
 
 Doacross make(const parser::OmpClause::Doacross &inp,
               semantics::SemanticsContext &semaCtx) {
-  // inp -> empty
-  llvm_unreachable("Empty: doacross");
+  // inp.v -> OmpDoacrossClause
+  return makeDoacross(inp.v.v, semaCtx);
 }
 
 // DynamicAllocators: empty
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 132fb6484bcfc5..67360b983a7d19 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -541,6 +541,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPConstruct &) {
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
+  loopStack_.push_back(&x);
   const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
   const auto &beginDir{std::get<parser::OmpLoopDirective>(beginLoopDir.t)};
 
@@ -933,11 +934,19 @@ void OmpStructureChecker::CheckDistLinear(
   }
 }
 
-void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
+void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
   if (llvm::omp::allSimdSet.test(GetContext().directive)) {
     ExitDirectiveNest(SIMDNest);
   }
   dirContext_.pop_back();
+
+  assert(!loopStack_.empty() && "Expecting non-empty loop stack");
+  const LoopConstruct &top = loopStack_.back();
+#ifndef NDEBUG
+  auto *loopc = std::get_if<const parser::OpenMPLoopConstruct *>(&top);
+  assert(loopc != nullptr && *loopc == &x && "Mismatched loop constructs");
+#endif
+  loopStack_.pop_back();
 }
 
 void OmpStructureChecker::Enter(const parser::OmpEndLoopDirective &x) {
@@ -1068,8 +1077,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) {
 void OmpStructureChecker::ChecksOnOrderedAsBlock() {
   if (FindClause(llvm::omp::Clause::OMPC_depend)) {
     context_.Say(GetContext().clauseSource,
-        "DEPEND(*) clauses are not allowed when ORDERED construct is a block"
-        " construct with an ORDERED region"_err_en_US);
+        "DEPEND clauses are not allowed when ORDERED construct is a block construct with an ORDERED region"_err_en_US);
     return;
   }
 
@@ -1618,7 +1626,8 @@ void OmpStructureChecker::CheckBarrierNesting(
 void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
   if (FindClause(llvm::omp::Clause::OMPC_threads) ||
       FindClause(llvm::omp::Clause::OMPC_simd)) {
-    context_.Say(GetContext().clauseSource,
+    context_.Say(
+        GetContext().clauseSource,
         "THREADS, SIMD clauses are not allowed when ORDERED construct is a "
         "standalone construct with no ORDERED region"_err_en_US);
   }
@@ -1645,8 +1654,9 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
     }
   };
 
-  auto clauseAll = FindClauses(llvm::omp::Clause::OMPC_depend);
-  for (auto itr = clauseAll.first; itr != clauseAll.second; ++itr) {
+  // Visit the DEPEND and DOACROSS clauses.
+  auto depClauses = FindClauses(llvm::omp::Clause::OMPC_depend);
+  for (auto itr = depClauses.first; itr != depClauses.second; ++itr) {
     const auto &dependClause{
         std::get<parser::OmpClause::Depend>(itr->second->u)};
     if (auto *doAcross{std::get_if<parser::OmpDoacross>(&dependClause.v.u)}) {
@@ -1656,6 +1666,11 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
           "Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US);
     }
   }
+  auto doaClauses = FindClauses(llvm::omp::Clause::OMPC_doacross);
+  for (auto itr = doaClauses.first; itr != doaClauses.second; ++itr) {
+    auto &doaClause{std::get<parser::OmpClause::Doacross>(itr->second->u)};
+    visitDoacross(doaClause.v.v, itr->second->source);
+  }
 
   bool isNestedInDoOrderedWithPara{false};
   if (CurrentDirectiveIsNested() &&
@@ -1693,13 +1708,18 @@ void OmpStructureChecker::CheckOrderedDependClause(
       }
     }
   };
-  auto clauseAll{FindClauses(llvm::omp::Clause::OMPC_depend)};
-  for (auto itr = clauseAll.first; itr != clauseAll.second; ++itr) {
+  auto depClauses{FindClauses(llvm::omp::Clause::OMPC_depend)};
+  for (auto itr = depClauses.first; itr != depClauses.second; ++itr) {
     auto &dependClause{std::get<parser::OmpClause::Depend>(itr->second->u)};
     if (auto *doAcross{std::get_if<parser::OmpDoacross>(&dependClause.v.u)}) {
       visitDoacross(*doAcross, itr->second->source);
     }
   }
+  auto doaClauses = FindClauses(llvm::omp::Clause::OMPC_doacross);
+  for (auto itr = doaClauses.first; itr != doaClauses.second; ++itr) {
+    auto &doaClause{std::get<parser::OmpClause::Doacross>(itr->second->u)};
+    visitDoacross(doaClause.v.v, itr->second->source);
+  }
 }
 
 void OmpStructureChecker::CheckTargetUpdate() {
@@ -2677,7 +2697,6 @@ CHECK_SIMPLE_CLAUSE(Bind, OMPC_bind)
 CHECK_SIMPLE_CLAUSE(Align, OMPC_align)
 CHECK_SIMPLE_CLAUSE(Compare, OMPC_compare)
 CHECK_SIMPLE_CLAUSE(CancellationConstructType, OMPC_cancellation_construct_type)
-CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross)
 CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute)
 CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare)
 CHECK_SIMPLE_CLAUSE(Fail, OMPC_fail)
@@ -3458,6 +3477,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
       "Unexpected alternative in update clause");
 
   if (doaDep) {
+    CheckDoacross(*doaDep);
     CheckDependenceType(doaDep->GetDepType());
   } else {
     CheckTaskDependenceType(taskDep->GetTaskDepType());
@@ -3537,6 +3557,93 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
   }
 }
 
+void OmpStructureChecker::Enter(const parser::OmpClause::Doacross &x) {
+  CheckAllowedClause(llvm::omp::Clause::OMPC_doacross);
+  CheckDoacross(x.v.v);
+}
+
+void OmpStructureChecker::CheckDoacross(const parser::OmpDoacross &doa) {
+  if (std::holds_alternative<parser::OmpDoacross::Source>(doa.u)) {
+    // Nothing to check here.
+    return;
+  }
+
+  // Process SINK dependence type. SINK may only appear in an ORDER construct,
+  // which references a prior ORDERED(n) clause on a DO or SIMD construct
+  // that marks the top of the loop nest.
+
+  auto &sink{std::get<parser::OmpDoacross::Sink>(doa.u)};
+  const std::list<parser::OmpIteration> &vec{sink.v.v};
+
+  // Check if the variables in the iteration vector are unique.
+  struct Less {
+    bool operator()(
+        const parser::OmpIteration *a, const parser::OmpIteration *b) const {
+      auto namea{std::get<parser::Name>(a->t)};
+      auto nameb{std::get<parser::Name>(b->t)};
+      assert(namea.symbol && nameb.symbol && "Unresolved symbols");
+      // The non-determinism of the "<" doesn't matter, we only care about
+      // equality, i.e.  a == b  <=>  !(a < b) && !(b < a)
+      return reinterpret_cast<uintptr_t>(namea.symbol) <
+          reinterpret_cast<uintptr_t>(nameb.symbol);
+    }
+  };
+  if (auto *duplicate{FindDuplicateEntry<parser::OmpIteration, Less>(vec)}) {
+    auto name{std::get<parser::Name>(duplicate->t)};
+    context_.Say(name.source,
+        "Duplicate variable '%s' in the iteration vector"_err_en_US,
+        name.ToString());
+  }
+
+  // Check if the variables in the iteration vector are induction variables.
+  // Ignore any mismatch between the size of the iteration vector and the
+  // number of DO constructs on the stack. This is checked elsewhere.
+
+  auto GetLoopDirective{[](const parser::OpenMPLoopConstruct &x) {
+    auto &begin{std::get<parser::OmpBeginLoopDirective>(x.t)};
+    return std::get<parser::OmpLoopDirective>(begin.t).v;
+  }};
+  auto GetLoopClauses{[](const parser::OpenMPLoopConstruct &x)
+                          -> const std::list<parser::OmpClause> & {
+    auto &begin{std::get<parser::OmpBeginLoopDirective>(x.t)};
+    return std::get<parser::OmpClauseList>(begin.t).v;
+  }};
+
+  std::set<const Symbol *> inductionVars;
+  for (const LoopConstruct &loop : llvm::reverse(loopStack_)) {
+    if (auto *doc{std::get_if<const parser::DoConstruct *>(&loop)}) {
+      // Do-construct, collect the induction variable.
+      if (auto &control{(*doc)->GetLoopControl()}) {
+        if (auto *b{std::get_if<parser::LoopControl::Bounds>(&control->u)}) {
+          inductionVars.insert(b->name.thing.symbol);
+        }
+      }
+    } else {
+      // Omp-loop-construct, check if it's do/simd with an ORDERED clause.
+      auto *loopc{std::get_if<const parser::OpenMPLoopConstruct *>(&loop)};
+      assert(loopc && "Expecting OpenMPLoopConstruct");
+      llvm::omp::Directive loopDir{GetLoopDirective(**loopc)};
+      if (loopDir == llvm::omp::OMPD_do || loopDir == llvm::omp::OMPD_simd) {
+        auto IsOrdered{[](const parser::OmpClause &c) {
+          return c.Id() == llvm::omp::OMPC_ordered;
+        }};
+        // If it has ORDERED clause, stop the traversal.
+        if (llvm::any_of(GetLoopClauses(**loopc), IsOrdered)) {
+          break;
+        }
+      }
+    }
+  }
+  for (const parser::OmpIteration &iter : vec) {
+    auto &name{std::get<parser::Name>(iter.t)};
+    if (!inductionVars.count(name.symbol)) {
+      context_.Say(name.source,
+          "The iteration vector element '%s' is not an induction variable within the ORDERED loop nest"_err_en_US,
+          name.ToString());
+    }
+  }
+}
+
 void OmpStructureChecker::CheckCopyingPolymorphicAllocatable(
     SymbolSourceMap &symbols, const llvm::omp::Clause clause) {
   if (context_.ShouldWarn(common::UsageWarning::Portability)) {
@@ -4291,6 +4398,22 @@ void OmpStructureChecker::Enter(
   CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_unified_shared_memory);
 }
 
+void OmpStructureChecker::Enter(const parser::DoConstruct &x) {
+  Base::Enter(x);
+  loopStack_.push_back(&x);
+}
+
+void OmpStructureChecker::Leave(const parser::DoConstruct &x) {
+  assert(!loopStack_.empty() && "Expecting non-empty loop stack");
+  const LoopConstruct &top = loopStack_.back();
+#ifndef NDEBUG
+  auto *doc = std::get_if<const parser::DoConstruct *>(&top);
+  assert(doc != nullptr && *doc == &x && "Mismatched loop constructs");
+#endif
+  loopStack_.pop_back();
+  Base::Leave(x);
+}
+
 void OmpStructureChecker::CheckAllowedRequiresClause(llvmOmpClause clause) {
   CheckAllowedClause(clause);
 
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 9efacaa9710084..57796ad32de698 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -60,6 +60,9 @@ class OmpStructureChecker
     : public DirectiveStructureChecker<llvm::omp::Directive, llvm::omp::Clause,
           parser::OmpClause, llvm::omp::Clause_enumSize> {
 public:
+  using Base = DirectiveStructureChecker<llvm::omp::Directive,
+      llvm::omp::Clause, parser::OmpClause, llvm::omp::Clause_enumSize>;
+
   OmpStructureChecker(SemanticsContext &context)
       : DirectiveStructureChecker(context,
 #define GEN_FLANG_DIRECTIVE_CLAUSE_MAP
@@ -131,6 +134,9 @@ class OmpStructureChecker
   void Enter(const parser::OmpAtomicCapture &);
   void Leave(const parser::OmpAtomic &);
 
+  void Enter(const parser::DoConstruct &);
+  void Leave(const parser::DoConstruct &);
+
 #define GEN_FLANG_CLAUSE_CHECK_ENTER
 #include "llvm/Frontend/OpenMP/OMP.inc"
 
@@ -156,13 +162,19 @@ class OmpStructureChecker
       const parser::OmpScheduleModifierType::ModType &);
   void CheckAllowedMapTypes(const parser::OmpMapClause::Type &,
       const std::list<parser::OmpMapClause::Type> &);
-  template <typename T> const T *FindDuplicateEntry(const std::list<T> &);
   llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
 
+  template <typename T> struct DefaultLess {
+    bool operator()(const T *a, const T *b) const { return *a < *b; }
+  };
+  template <typename T, typename Less = DefaultLess<T>>
+  const T *FindDuplicateEntry(const std::list<T> &);
+
   void CheckDependList(const parser::DataRef &);
   void CheckDependArraySection(
       const common::Indirection<parser::ArrayElement> &, const parser::Name &);
+  void CheckDoacross(const parser::OmpDoacross &doa);
   bool IsDataRefTypeParamInquiry(const parser::DataRef *dataRef);
   void CheckIsVarPartOfAnotherVar(const parser::CharBlock &source,
       const parser::OmpObjectList &objList, llvm::StringRef clause = "");
@@ -254,9 +266,13 @@ class OmpStructureChecker
   int directiveNest_[LastType + 1] = {0};
 
   SymbolSourceMap deferredNonVariables_;
+
+  using LoopConstruct = std::variant<const parser::DoConstruct *,
+      const parser::OpenMPLoopConstruct *>;
+  std::vector<LoopConstruct> loopStack_;
 };
 
-template <typename T>
+template <typename T, typename Less>
 const T *OmpStructureChecker::FindDuplicateEntry(const std::list<T> &list) {
   // Add elements of the list to a set. If the insertion fails, return
   // the address of the failing element.
@@ -264,10 +280,7 @@ const T *OmpStructureChecker::FindDuplicateEntry(const std::list<T> &list) {
   // The objects of type T may not be copyable, so add their addresses
   // to the set. The set will need to compare the actual objects, so
   // the custom comparator is provided.
-  struct less {
-    bool operator()(const T *a, const T *b) const { return *a < *b; }
-  };
-  std::set<const T *, less> uniq;
+  std::set<const T *, Less> uniq;
 
   for (const T &item : list) {
     if (!uniq.insert(&item).second) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 632d7e918ac64f..9267262d4718e4 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -554,8 +554,16 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
   }
 
   void Post(const parser::OmpIteration &x) {
-    const auto &name{std::get<parser::Name>(x.t)};
-    ResolveName(&name);
+    if (const auto &name{std::get<parser::Name>(x.t)}; !name.symbol) {
+      auto *symbol{currScope().FindSymbol(name.source)};
+      if (!symbol) {
+        // OmpIteration must use an existing object. If there isn't one,
+        // create a fake one and flag an error later.
+        symbol = &currScope().MakeSymbol(
+            name.source, Attrs{}, EntityDetails(/*isDummy=*/true));
+      }
+      Resolve(name, symbol);
+    }
   }
 
   bool Pre(const parser::OmpClause::UseDevicePtr &x) {
diff --git a/flang/test/Lower/OpenMP/Todo/ordered.f90 b/flang/test/Lower/OpenMP/Todo/ordered.f90
new file mode 100644
index 00000000000000..2f91e5ed28a1a0
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/ordered.f90
@@ -0,0 +1,20 @@
+!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+
+!CHECK: not yet implemented: OMPD_ordered
+subroutine f00(x)
+  integer :: a(10)
+
+  do i = 1, 10
+    !$omp do ordered(3)
+    do j = 1, 10
+      do k = 1, 10
+        do m = 1, 10
+          !$omp ordered doacross(sink: m+1, k+0, j-2)
+          a(i) = i
+        enddo
+      enddo
+    enddo
+    !$omp end do
+  enddo
+end
diff --git a/flang/test/Semantics/OpenMP/doacross.f90 b/flang/test/Semantics/OpenMP/doacross.f90
new file mode 100644
index 00000000000000..381a4118ce7bfd
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/doacross.f90
@@ -0,0 +1,28 @@
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=52
+
+subroutine f00(x)
+  integer :: x(10, 10)
+  !$omp do ordered(2)
+  do i = 1, 10
+    do j = 1, 10
+!ERROR: Duplicate variable 'i' in the iteration vector
+      !$omp ordered doacross(sink: i+1, i-2)
+      x(i, j) = 0
+    enddo
+  enddo
+  !$omp end do
+end
+
+subroutine f01(x)
+  integer :: x(10, 10)
+  do i = 1, 10
+    !$omp do ordered(1)
+    do j = 1, 10
+!ERROR: The iteration vector element 'i' is not an induction variable within the ORDERED loop nest
+      !$omp ordered doacross(sink: i+1)
+      x(i, j) = 0
+    enddo
+    !$omp end do
+  enddo
+end
+
diff --git a/flang/test/Semantics/OpenMP/ordered01.f90 b/flang/test/Semantics/OpenMP/ordered01.f90
index 9f3a258d470a6f..661b3939b3e626 100644
--- a/flang/test/Semantics/OpenMP/ordered01.f90
+++ b/flang/test/Semantics/OpenMP/ordered01.f90
@@ -54,11 +54,11 @@ program main
 
   !$omp do ordered(1)
   do i = 2, N
-    !ERROR: DEPEND(*) clauses are not allowed when ORDERED construct is a block construct with an ORDERED region
+    !ERROR: DEPEND clauses are not allowed when ORDERED construct is a block construct with an ORDERED region
     !$omp ordered depend(source)
     arrayA(i) = foo(i)
     !$omp end ordered
-    !ERROR: DEPEND(*) clauses are not allowed when ORDERED construct is a block construct with an ORDERED region
+    !ERROR: DEPEND clauses are not allowed when ORDERED construct is a block construct with an ORDERED region
     !$omp ordered depend(sink: i - 1)
     arrayB(i) = bar(arrayA(i), arrayB(i-1))
     !$omp end ordered
diff --git a/flang/test/Semantics/OpenMP/ordered03.f90 b/flang/test/Semantics/OpenMP/ordered03.f90
index e96d4557f8f18b..6a7037e2b750c5 100644
--- a/flang/test/Semantics/OpenMP/ordered03.f90
+++ b/flang/test/Semantics/OpenMP/ordered03.f90
@@ -100,6 +100,7 @@ subroutine sub1()
   !$omp do ordered(1)
   do i = 1, N
     !ERROR: The number of variables in the SINK iteration vector does not match the parameter specified in ORDERED clause
+    !ERROR: The iteration vector element 'j' is not an induction variable within the ORDERED loop nest
     !$omp ordered depend(sink: i - 1) depend(sink: i - 1, j)
     arrayB(i) = bar(i - 1, j)
   end do
@@ -119,5 +120,6 @@ subroutine sub1()
   !$omp ordered depend(source)
 
   !ERROR: An ORDERED construct with the DEPEND clause must be closely nested in a worksharing-loop (or parallel worksharing-loop) construct with ORDERED clause with a parameter
+  !ERROR: The iteration vector element 'i' is not an induction variable within the ORDERED loop nest
   !$omp ordered depend(sink: i - 1)
 end

>From ecde8f5cfacdec0daf6e223143dca374bc954cbc Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 7 Nov 2024 17:34:46 -0600
Subject: [PATCH 2/6] format

---
 flang/lib/Semantics/check-omp-structure.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 67360b983a7d19..cfa6155b5b713d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1626,10 +1626,8 @@ void OmpStructureChecker::CheckBarrierNesting(
 void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
   if (FindClause(llvm::omp::Clause::OMPC_threads) ||
       FindClause(llvm::omp::Clause::OMPC_simd)) {
-    context_.Say(
-        GetContext().clauseSource,
-        "THREADS, SIMD clauses are not allowed when ORDERED construct is a "
-        "standalone construct with no ORDERED region"_err_en_US);
+    context_.Say(GetContext().clauseSource,
+        "THREADS and SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US);
   }
 
   int dependSinkCount{0}, dependSourceCount{0};

>From 49d5e9c878987c1e553fb4d2837c85a62a99f5a0 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 7 Nov 2024 17:38:50 -0600
Subject: [PATCH 3/6] restart build


>From 9ab7cd0b0d359d98d5adf530edd0aee901ca0c23 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 7 Nov 2024 19:27:04 -0600
Subject: [PATCH 4/6] fix error message in test

---
 flang/test/Semantics/OpenMP/ordered01.f90 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/flang/test/Semantics/OpenMP/ordered01.f90 b/flang/test/Semantics/OpenMP/ordered01.f90
index 661b3939b3e626..12543acb2916b3 100644
--- a/flang/test/Semantics/OpenMP/ordered01.f90
+++ b/flang/test/Semantics/OpenMP/ordered01.f90
@@ -67,12 +67,12 @@ program main
 
 contains
   subroutine work1()
-    !ERROR: THREADS, SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region
+    !ERROR: THREADS and SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region
     !$omp ordered simd
   end subroutine work1
 
   subroutine work2()
-    !ERROR: THREADS, SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region
+    !ERROR: THREADS and SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region
     !$omp ordered threads
   end subroutine work2
 

>From d72089c5f0eba49c05fac95646e59aff0ec3ac4b Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 11 Nov 2024 06:16:32 -0600
Subject: [PATCH 5/6] Update flang/lib/Semantics/check-omp-structure.cpp

Co-authored-by: Tom Eccles <tom.eccles at arm.com>
---
 flang/lib/Semantics/check-omp-structure.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index cfa6155b5b713d..6af402e707c3b1 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1664,7 +1664,7 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
           "Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US);
     }
   }
-  auto doaClauses = FindClauses(llvm::omp::Clause::OMPC_doacross);
+  auto doaClauses{FindClauses(llvm::omp::Clause::OMPC_doacross)};
   for (auto itr = doaClauses.first; itr != doaClauses.second; ++itr) {
     auto &doaClause{std::get<parser::OmpClause::Doacross>(itr->second->u)};
     visitDoacross(doaClause.v.v, itr->second->source);

>From 2a94fd732c37d23d09bb6272574eb9d3e789d659 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 11 Nov 2024 06:35:15 -0600
Subject: [PATCH 6/6] use {} initialization in more places

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

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 6af402e707c3b1..2f8625746d366f 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -941,9 +941,9 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
   dirContext_.pop_back();
 
   assert(!loopStack_.empty() && "Expecting non-empty loop stack");
-  const LoopConstruct &top = loopStack_.back();
+  const LoopConstruct &top{loopStack_.back()};
 #ifndef NDEBUG
-  auto *loopc = std::get_if<const parser::OpenMPLoopConstruct *>(&top);
+  auto *loopc{std::get_if<const parser::OpenMPLoopConstruct *>(&top)};
   assert(loopc != nullptr && *loopc == &x && "Mismatched loop constructs");
 #endif
   loopStack_.pop_back();
@@ -1633,8 +1633,8 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
   int dependSinkCount{0}, dependSourceCount{0};
   bool exclusiveShown{false}, duplicateSourceShown{false};
 
-  auto visitDoacross = [&](const parser::OmpDoacross &doa,
-                           const parser::CharBlock &src) {
+  auto visitDoacross{[&](const parser::OmpDoacross &doa,
+                         const parser::CharBlock &src) {
     common::visit(
         common::visitors{
             [&](const parser::OmpDoacross::Source &) { dependSourceCount++; },
@@ -1650,11 +1650,11 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
       context_.Say(src,
           "At most one SOURCE dependence type can appear on the ORDERED directive"_err_en_US);
     }
-  };
+  }};
 
   // Visit the DEPEND and DOACROSS clauses.
-  auto depClauses = FindClauses(llvm::omp::Clause::OMPC_depend);
-  for (auto itr = depClauses.first; itr != depClauses.second; ++itr) {
+  auto depClauses{FindClauses(llvm::omp::Clause::OMPC_depend)};
+  for (auto itr{depClauses.first}; itr != depClauses.second; ++itr) {
     const auto &dependClause{
         std::get<parser::OmpClause::Depend>(itr->second->u)};
     if (auto *doAcross{std::get_if<parser::OmpDoacross>(&dependClause.v.u)}) {
@@ -1665,7 +1665,7 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
     }
   }
   auto doaClauses{FindClauses(llvm::omp::Clause::OMPC_doacross)};
-  for (auto itr = doaClauses.first; itr != doaClauses.second; ++itr) {
+  for (auto itr{doaClauses.first}; itr != doaClauses.second; ++itr) {
     auto &doaClause{std::get<parser::OmpClause::Doacross>(itr->second->u)};
     visitDoacross(doaClause.v.v, itr->second->source);
   }
@@ -4405,7 +4405,7 @@ void OmpStructureChecker::Leave(const parser::DoConstruct &x) {
   assert(!loopStack_.empty() && "Expecting non-empty loop stack");
   const LoopConstruct &top = loopStack_.back();
 #ifndef NDEBUG
-  auto *doc = std::get_if<const parser::DoConstruct *>(&top);
+  auto *doc{std::get_if<const parser::DoConstruct *>(&top)};
   assert(doc != nullptr && *doc == &x && "Mismatched loop constructs");
 #endif
   loopStack_.pop_back();



More information about the llvm-branch-commits mailing list