R6: Reworked to add a few more semantic checks and moved checks to ch…

…eck-omp-structure
llvm · Sep 9, 2024 · 7e1d4cf · 7e1d4cf
1 parent 945fd5d
commit 7e1d4cf
Show file tree

Hide file tree

Showing 7 changed files with 156 additions and 35 deletions.
diff --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
@@ -198,6 +198,10 @@ class DirectiveStructureChecker : public virtual BaseChecker {
     ClauseMapTy clauseInfo;
     std::list<C> actualClauses;
     std::list<C> crtGroup;
+    std::set<std::string> usedInScanDirective;
+
+    using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
+    std::map<const std::string, ReductionModifier> reductionMod;
     Symbol *loopIV{nullptr};
   };
 

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
@@ -759,7 +759,43 @@ void OmpStructureChecker::CheckDistLinear(
   }
 }
 
-void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) {
+void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
+  const auto &beginLoopDir = std::get<parser::OmpBeginLoopDirective>(x.t);
+  const auto &clauseList{std::get<parser::OmpClauseList>(beginLoopDir.t)};
+  for (const auto &clause : clauseList.v) {
+    if (const auto *reductionClause{
+            std::get_if<parser::OmpClause::Reduction>(&clause.u)}) {
+      using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
+      const auto &maybeModifier{
+          std::get<std::optional<ReductionModifier>>(reductionClause->v.t)};
+      if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
+
+        const auto &objectList{
+            std::get<parser::OmpObjectList>(reductionClause->v.t)};
+        for (const auto &ompObj : objectList.v) {
+          common::visit(
+              common::visitors{
+                  [&](const parser::Designator &designator) {
+                    if (const auto *name{semantics::getDesignatorNameIfDataRef(
+                            designator)}) {
+                      std::string nameStr = name->symbol->name().ToString();
+                      if (GetContext().usedInScanDirective.find(nameStr) ==
+                          GetContext().usedInScanDirective.end()) {
+                        context_.Say(name->source,
+                            "List item %s must appear in 'inclusive' or "
+                            "'exclusive' clause of an "
+                            "enclosed scan directive"_err_en_US,
+                            nameStr);
+                      }
+                    }
+                  },
+                  [&](const auto &name) {},
+              },
+              ompObj.u);
+        }
+      }
+    }
+  }
   if (llvm::omp::allSimdSet.test(GetContext().directive)) {
     ExitDirectiveNest(SIMDNest);
   }
@@ -1391,19 +1427,32 @@ void OmpStructureChecker::Leave(const parser::OpenMPAllocatorsConstruct &x) {
   dirContext_.pop_back();
 }
 
+void OmpStructureChecker::CheckScan(
+    const parser::OpenMPSimpleStandaloneConstruct &x) {
+  if (std::get<parser::OmpClauseList>(x.t).v.size() != 1) {
+    context_.Say(x.source,
+        "Exactly one of `exclusive` or `inclusive` clause is expected"_err_en_US);
+  }
+  if (!CurrentDirectiveIsNested() ||
+      !llvm::omp::scanParentAllowedSet.test(GetContextParent().directive)) {
+    context_.Say(x.source,
+        "Orphaned `omp scan` directives are prohibited; perhaps you forgot "
+        "to enclose the directive in to a worksharing loop, a worksharing "
+        "loop simd or a simd directive."_err_en_US);
+  }
+}
+
 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);
-    }
+  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);
   }
 }
 
@@ -1524,7 +1573,16 @@ void OmpStructureChecker::Enter(
     const parser::OpenMPSimpleStandaloneConstruct &x) {
   const auto &dir{std::get<parser::OmpSimpleStandaloneDirective>(x.t)};
   PushContextAndClauseSets(dir.source, dir.v);
-  CheckBarrierNesting(x);
+  switch (dir.v) {
+  case llvm::omp::Directive::OMPD_barrier:
+    CheckBarrierNesting(x);
+    break;
+  case llvm::omp::Directive::OMPD_scan:
+    CheckScan(x);
+    break;
+  default:
+    break;
+  }
 }
 
 void OmpStructureChecker::Leave(
@@ -2240,15 +2298,13 @@ CHECK_SIMPLE_CLAUSE(Destroy, OMPC_destroy)
 CHECK_SIMPLE_CLAUSE(Detach, OMPC_detach)
 CHECK_SIMPLE_CLAUSE(DeviceType, OMPC_device_type)
 CHECK_SIMPLE_CLAUSE(DistSchedule, OMPC_dist_schedule)
-CHECK_SIMPLE_CLAUSE(Exclusive, OMPC_exclusive)
 CHECK_SIMPLE_CLAUSE(Final, OMPC_final)
 CHECK_SIMPLE_CLAUSE(Flush, OMPC_flush)
 CHECK_SIMPLE_CLAUSE(From, OMPC_from)
 CHECK_SIMPLE_CLAUSE(Full, OMPC_full)
 CHECK_SIMPLE_CLAUSE(Hint, OMPC_hint)
 CHECK_SIMPLE_CLAUSE(Holds, OMPC_holds)
 CHECK_SIMPLE_CLAUSE(InReduction, OMPC_in_reduction)
-CHECK_SIMPLE_CLAUSE(Inclusive, OMPC_inclusive)
 CHECK_SIMPLE_CLAUSE(Match, OMPC_match)
 CHECK_SIMPLE_CLAUSE(Nontemporal, OMPC_nontemporal)
 CHECK_SIMPLE_CLAUSE(Order, OMPC_order)
@@ -2321,7 +2377,24 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
   if (CheckReductionOperators(x)) {
     CheckReductionTypeList(x);
   }
-  CheckReductionModifier(x);
+  using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
+  if (const auto &maybeModifier{
+          std::get<std::optional<ReductionModifier>>(x.v.t)}) {
+    ReductionModifier modifier{*maybeModifier};
+    const auto &ompObjectList{std::get<parser::OmpObjectList>(x.v.t)};
+    addModifiertoMap(ompObjectList, modifier);
+    CheckReductionModifier(modifier);
+  }
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::Inclusive &x) {
+  CheckAllowed(llvm::omp::Clause::OMPC_inclusive);
+  checkAndAddSymbolsToUsedInScanList(x.v);
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::Exclusive &x) {
+  CheckAllowed(llvm::omp::Clause::OMPC_exclusive);
+  checkAndAddSymbolsToUsedInScanList(x.v);
 }
 
 bool OmpStructureChecker::CheckReductionOperators(
@@ -2364,6 +2437,49 @@ bool OmpStructureChecker::CheckReductionOperators(
 
   return ok;
 }
+
+void OmpStructureChecker::addModifiertoMap(const parser::OmpObjectList &x,
+    parser::OmpReductionClause::ReductionModifier &modifier) {
+  for (const auto &ompObject : x.v) {
+    if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
+      if (const auto *symbol{name->symbol}) {
+        GetContext().reductionMod[symbol->name().ToString()] = modifier;
+      }
+    }
+  }
+}
+
+void OmpStructureChecker::checkAndAddSymbolsToUsedInScanList(
+    const parser::OmpObjectList &x) {
+  for (const auto &ompObj : x.v) {
+    common::visit(
+        common::visitors{
+            [&](const parser::Designator &designator) {
+              if (const auto *name{
+                      semantics::getDesignatorNameIfDataRef(designator)}) {
+                if (name->symbol) {
+                  if (CurrentDirectiveIsNested()) {
+                    std::string nameStr = name->symbol->name().ToString();
+                    if (GetContextParent().reductionMod.find(nameStr) ==
+                        GetContextParent().reductionMod.end()) {
+
+                      context_.Say(name->source,
+                          "List item %s must appear in 'reduction' clause "
+                          "with the 'inscan' modifier of the parent "
+                          "directive"_err_en_US,
+                          nameStr);
+                    }
+                    GetContextParent().usedInScanDirective.insert(nameStr);
+                  }
+                }
+              }
+            },
+            [&](const auto &name) {},
+        },
+        ompObj.u);
+  }
+}
+
 bool OmpStructureChecker::CheckIntrinsicOperator(
     const parser::DefinedOperator::IntrinsicOperator &op) {
 
@@ -2498,14 +2614,12 @@ void OmpStructureChecker::CheckReductionTypeList(
 }
 
 void OmpStructureChecker::CheckReductionModifier(
-    const parser::OmpClause::Reduction &x) {
+    const parser::OmpReductionClause::ReductionModifier &modifier) {
   using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
-  const auto &maybeModifier{std::get<std::optional<ReductionModifier>>(x.v.t)};
-  if (!maybeModifier || *maybeModifier == ReductionModifier::Default) {
-    // No modifier, or the default one is always ok.
+  if (modifier == ReductionModifier::Default) {
+    // the default one is always ok.
     return;
   }
-  ReductionModifier modifier{*maybeModifier};
   const DirectiveContext &dirCtx{GetContext()};
   if (dirCtx.directive == llvm::omp::Directive::OMPD_loop) {
     // [5.2:257:33-34]

diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
@@ -205,10 +205,12 @@ class OmpStructureChecker
   bool CheckIntrinsicOperator(
       const parser::DefinedOperator::IntrinsicOperator &);
   void CheckReductionTypeList(const parser::OmpClause::Reduction &);
-  void CheckReductionModifier(const parser::OmpClause::Reduction &);
+  void CheckReductionModifier(
+      const parser::OmpReductionClause::ReductionModifier &);
   void CheckMasterNesting(const parser::OpenMPBlockConstruct &x);
   void ChecksOnOrderedAsBlock();
   void CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &x);
+  void CheckScan(const parser::OpenMPSimpleStandaloneConstruct &x);
   void ChecksOnOrderedAsStandalone();
   void CheckOrderedDependClause(std::optional<std::int64_t> orderedValue);
   void CheckReductionArraySection(const parser::OmpObjectList &ompObjectList);
@@ -223,6 +225,9 @@ class OmpStructureChecker
       const parser::OmpObjectList &ompObjectList);
   void CheckPredefinedAllocatorRestriction(
       const parser::CharBlock &source, const parser::Name &name);
+  void checkAndAddSymbolsToUsedInScanList(const parser::OmpObjectList &x);
+  void addModifiertoMap(const parser::OmpObjectList &x,
+      parser::OmpReductionClause::ReductionModifier &modifier);
   bool isPredefinedAllocator{false};
 
   void CheckAllowedRequiresClause(llvmOmpClause clause);

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
@@ -1578,7 +1578,6 @@ bool OmpAttributeVisitor::Pre(
     const parser::OpenMPSimpleStandaloneConstruct &x) {
   const auto &standaloneDir{
       std::get<parser::OmpSimpleStandaloneDirective>(x.t)};
-  const auto &parentContext{GetContextIf()};
   switch (standaloneDir.v) {
   case llvm::omp::Directive::OMPD_barrier:
   case llvm::omp::Directive::OMPD_ordered:
@@ -1593,19 +1592,6 @@ bool OmpAttributeVisitor::Pre(
   default:
     break;
   }
-  if (standaloneDir.v == llvm::omp::Directive::OMPD_scan) {
-    if (std::get<parser::OmpClauseList>(x.t).v.size() != 1) {
-      context_.Say(standaloneDir.source,
-          "Exactly one of `exclusive` or `inclusive` clause is expected"_err_en_US);
-    }
-    if (!parentContext ||
-        !llvm::omp::scanParentAllowedSet.test(parentContext->directive)) {
-      context_.Say(standaloneDir.source,
-          "Orphaned `omp scan` directives are prohibited; perhaps you forgot "
-          "to enclose the directive in to a worksharing loop, a worksharing "
-          "loop simd or a simd directive."_err_en_US);
-    }
-  }
   ClearDataSharingAttributeObjects();
   return true;
 }

diff --git a/flang/test/Lower/OpenMP/Todo/reduction-modifiers.f90 b/flang/test/Lower/OpenMP/Todo/reduction-modifiers.f90
@@ -8,6 +8,7 @@ subroutine foo()
   j = 0
   !$omp do reduction (inscan, *: j)
   do i = 1, 10
+    !$omp scan inclusive(j)
     j = j + 1
   end do
 end subroutine
diff --git a/flang/test/Semantics/OpenMP/reduction-modifiers.f90 b/flang/test/Semantics/OpenMP/reduction-modifiers.f90
@@ -39,6 +39,7 @@ subroutine mod_inscan1(x)
   !Correct: worksharing-loop directive
   !$omp do reduction(inscan, +:x)
   do i = 1, 100
+    !$omp scan inclusive(x)
     x = foo(i)
   enddo
   !$omp end do
@@ -50,6 +51,7 @@ subroutine mod_inscan2(x)
   !Correct: worksharing-loop simd directive
   !$omp do simd reduction(inscan, +:x)
   do i = 1, 100
+    !$omp scan inclusive(x)
     x = foo(i)
   enddo
   !$omp end do simd
@@ -61,6 +63,7 @@ subroutine mod_inscan3(x)
   !Correct: "simd" directive
   !$omp simd reduction(inscan, +:x)
   do i = 1, 100
+    !$omp scan inclusive(x)
     x = foo(i)
   enddo
   !$omp end simd

diff --git a/flang/test/Semantics/OpenMP/scan.f90 b/flang/test/Semantics/OpenMP/scan.f90
@@ -1,17 +1,17 @@
 ! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
 
 subroutine test_scan()
- integer x, y, k
+ integer x, y, k, z
 
  !ERROR: Orphaned `omp scan` directives are prohibited; perhaps you forgot to enclose the directive in to a worksharing loop, a worksharing loop simd or a simd directive.
  !$omp scan inclusive(x)
- !$omp parallel do simd reduction(inscan,+: x)
+ !$omp parallel do simd
  do k = 1, n
  !ERROR: UNTIED clause is not allowed on the SCAN directive
    !$omp scan untied
  end do
 
- !$omp parallel do simd reduction(inscan,+: x)
+ !$omp parallel do simd
  do k = 1, n
  !ERROR: Exactly one of `exclusive` or `inclusive` clause is expected
    !$omp scan
@@ -22,4 +22,12 @@ subroutine test_scan()
  !ERROR: Exactly one of `exclusive` or `inclusive` clause is expected
    !$omp scan inclusive(x) exclusive(y)
  end do
+
+!ERROR: List item y must appear in 'inclusive' or 'exclusive' clause of an enclosed scan directive
+!$omp parallel do simd reduction(inscan,+: x, y)
+ do k = 1, n
+ !ERROR: Exactly one of `exclusive` or `inclusive` clause is expected
+ !ERROR: List item z must appear in 'reduction' clause with the 'inscan' modifier of the parent directive
+   !$omp scan inclusive(x) exclusive(z)
+ end do
 end subroutine