[flang-commits] [flang] 2221b75 - [Flang][OpenMP][Sema] Add semantics checks for REQUIRES directive

Sergio Afonso via flang-commits flang-commits at lists.llvm.org
Tue Aug 15 04:38:34 PDT 2023


Author: Sergio Afonso
Date: 2023-08-15T12:38:14+01:00
New Revision: 2221b758d2fbd518dca05be9b45d22848e961a48

URL: https://github.com/llvm/llvm-project/commit/2221b758d2fbd518dca05be9b45d22848e961a48
DIFF: https://github.com/llvm/llvm-project/commit/2221b758d2fbd518dca05be9b45d22848e961a48.diff

LOG: [Flang][OpenMP][Sema] Add semantics checks for REQUIRES directive

This patch adds semantics checks for REQUIRES directives appearing after other
directives that are affected by them. In particular, it adds checks for device
constructs appearing after device-related REQUIRES directives and for the
`atomic_default_mem_order` clause appearing after atomic operations where the
memory order is not explicitly specified.

This is patch 2/5 of a series splitting D149337 to simplify review.

Depends on D157710.

Differential Revision: https://reviews.llvm.org/D157722

Added: 
    flang/test/Semantics/OpenMP/requires01.f90
    flang/test/Semantics/OpenMP/requires02.f90
    flang/test/Semantics/OpenMP/requires03.f90
    flang/test/Semantics/OpenMP/requires04.f90
    flang/test/Semantics/OpenMP/requires05.f90
    flang/test/Semantics/OpenMP/requires06.f90
    flang/test/Semantics/OpenMP/requires07.f90
    flang/test/Semantics/OpenMP/requires08.f90

Modified: 
    flang/lib/Semantics/check-omp-structure.cpp
    flang/lib/Semantics/check-omp-structure.h

Removed: 
    flang/test/Semantics/OpenMP/requires.f90


################################################################################
diff  --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 7a04d28bfa8ad1..92706aeac9c717 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -400,6 +400,13 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
     EnterDirectiveNest(SIMDNest);
   }
 
+  // Combined target loop constructs are target device constructs. Keep track of
+  // whether any such construct has been visited to later check that REQUIRES
+  // directives for target-related options don't appear after them.
+  if (llvm::omp::allTargetSet.test(beginDir.v)) {
+    deviceConstructFound_ = true;
+  }
+
   if (beginDir.v == llvm::omp::Directive::OMPD_do) {
     // 2.7.1 do-clause -> private-clause |
     //                    firstprivate-clause |
@@ -791,6 +798,13 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
 
   CheckNoBranching(block, beginDir.v, beginDir.source);
 
+  // Target block constructs are target device constructs. Keep track of
+  // whether any such construct has been visited to later check that REQUIRES
+  // directives for target-related options don't appear after them.
+  if (llvm::omp::allTargetSet.test(beginDir.v)) {
+    deviceConstructFound_ = true;
+  }
+
   switch (beginDir.v) {
   case llvm::omp::Directive::OMPD_target:
     if (CheckTargetBlockOnlyTeams(block)) {
@@ -1189,22 +1203,47 @@ void OmpStructureChecker::CheckSymbolNames(
 void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
   const auto &dir{std::get<parser::Verbatim>(x.t)};
   const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
+  // Handle both forms of DECLARE TARGET.
+  // - Extended list: It behaves as if there was an ENTER/TO clause with the
+  //   list of objects as argument. It accepts no explicit clauses.
+  // - With clauses.
   if (const auto *objectList{parser::Unwrap<parser::OmpObjectList>(spec.u)}) {
+    deviceConstructFound_ = true;
     CheckSymbolNames(dir.source, *objectList);
     CheckIsVarPartOfAnotherVar(dir.source, *objectList);
     CheckThreadprivateOrDeclareTargetVar(*objectList);
   } else if (const auto *clauseList{
                  parser::Unwrap<parser::OmpClauseList>(spec.u)}) {
+    bool toClauseFound{false}, deviceTypeClauseFound{false};
     for (const auto &clause : clauseList->v) {
-      if (const auto *toClause{std::get_if<parser::OmpClause::To>(&clause.u)}) {
-        CheckSymbolNames(dir.source, toClause->v);
-        CheckIsVarPartOfAnotherVar(dir.source, toClause->v);
-        CheckThreadprivateOrDeclareTargetVar(toClause->v);
-      } else if (const auto *linkClause{
-                     std::get_if<parser::OmpClause::Link>(&clause.u)}) {
-        CheckSymbolNames(dir.source, linkClause->v);
-        CheckIsVarPartOfAnotherVar(dir.source, linkClause->v);
-        CheckThreadprivateOrDeclareTargetVar(linkClause->v);
+      common::visit(
+          common::visitors{
+              [&](const parser::OmpClause::To &toClause) {
+                toClauseFound = true;
+                CheckSymbolNames(dir.source, toClause.v);
+                CheckIsVarPartOfAnotherVar(dir.source, toClause.v);
+                CheckThreadprivateOrDeclareTargetVar(toClause.v);
+              },
+              [&](const parser::OmpClause::Link &linkClause) {
+                CheckSymbolNames(dir.source, linkClause.v);
+                CheckIsVarPartOfAnotherVar(dir.source, linkClause.v);
+                CheckThreadprivateOrDeclareTargetVar(linkClause.v);
+              },
+              [&](const parser::OmpClause::DeviceType &deviceTypeClause) {
+                deviceTypeClauseFound = true;
+                if (deviceTypeClause.v.v !=
+                    parser::OmpDeviceTypeClause::Type::Host) {
+                  // Function / subroutine explicitly marked as runnable by the
+                  // target device.
+                  deviceConstructFound_ = true;
+                }
+              },
+              [&](const auto &) {},
+          },
+          clause.u);
+
+      if (toClauseFound && !deviceTypeClauseFound) {
+        deviceConstructFound_ = true;
       }
     }
   }
@@ -1721,6 +1760,9 @@ void OmpStructureChecker::CheckAtomicMemoryOrderClause(
   if (rightHandClauseList) {
     checkForValidMemoryOrderClause(rightHandClauseList);
   }
+  if (numMemoryOrderClause == 0) {
+    atomicDirectiveDefaultOrderFound_ = true;
+  }
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
@@ -1917,7 +1959,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause &x) {
 // Following clauses do not have a separate node in parse-tree.h.
 CHECK_SIMPLE_CLAUSE(AcqRel, OMPC_acq_rel)
 CHECK_SIMPLE_CLAUSE(Acquire, OMPC_acquire)
-CHECK_SIMPLE_CLAUSE(AtomicDefaultMemOrder, OMPC_atomic_default_mem_order)
 CHECK_SIMPLE_CLAUSE(Affinity, OMPC_affinity)
 CHECK_SIMPLE_CLAUSE(Capture, OMPC_capture)
 CHECK_SIMPLE_CLAUSE(Default, OMPC_default)
@@ -1926,7 +1967,6 @@ 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(DynamicAllocators, OMPC_dynamic_allocators)
 CHECK_SIMPLE_CLAUSE(Exclusive, OMPC_exclusive)
 CHECK_SIMPLE_CLAUSE(Final, OMPC_final)
 CHECK_SIMPLE_CLAUSE(Flush, OMPC_flush)
@@ -1939,7 +1979,6 @@ CHECK_SIMPLE_CLAUSE(Match, OMPC_match)
 CHECK_SIMPLE_CLAUSE(Nontemporal, OMPC_nontemporal)
 CHECK_SIMPLE_CLAUSE(Order, OMPC_order)
 CHECK_SIMPLE_CLAUSE(Read, OMPC_read)
-CHECK_SIMPLE_CLAUSE(ReverseOffload, OMPC_reverse_offload)
 CHECK_SIMPLE_CLAUSE(Threadprivate, OMPC_threadprivate)
 CHECK_SIMPLE_CLAUSE(Threads, OMPC_threads)
 CHECK_SIMPLE_CLAUSE(Inbranch, OMPC_inbranch)
@@ -1960,8 +1999,6 @@ CHECK_SIMPLE_CLAUSE(Simd, OMPC_simd)
 CHECK_SIMPLE_CLAUSE(Sizes, OMPC_sizes)
 CHECK_SIMPLE_CLAUSE(TaskReduction, OMPC_task_reduction)
 CHECK_SIMPLE_CLAUSE(To, OMPC_to)
-CHECK_SIMPLE_CLAUSE(UnifiedAddress, OMPC_unified_address)
-CHECK_SIMPLE_CLAUSE(UnifiedSharedMemory, OMPC_unified_shared_memory)
 CHECK_SIMPLE_CLAUSE(Uniform, OMPC_uniform)
 CHECK_SIMPLE_CLAUSE(Unknown, OMPC_unknown)
 CHECK_SIMPLE_CLAUSE(Untied, OMPC_untied)
@@ -2962,4 +2999,49 @@ const parser::OmpObjectList *OmpStructureChecker::GetOmpObjectList(
       clause.u);
 }
 
+void OmpStructureChecker::Enter(
+    const parser::OmpClause::AtomicDefaultMemOrder &x) {
+  CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_atomic_default_mem_order);
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::DynamicAllocators &x) {
+  CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_dynamic_allocators);
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::ReverseOffload &x) {
+  CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_reverse_offload);
+}
+
+void OmpStructureChecker::Enter(const parser::OmpClause::UnifiedAddress &x) {
+  CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_unified_address);
+}
+
+void OmpStructureChecker::Enter(
+    const parser::OmpClause::UnifiedSharedMemory &x) {
+  CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_unified_shared_memory);
+}
+
+void OmpStructureChecker::CheckAllowedRequiresClause(llvmOmpClause clause) {
+  CheckAllowed(clause);
+
+  if (clause == llvm::omp::Clause::OMPC_atomic_default_mem_order) {
+    // Check that it does not appear after an atomic operation without memory
+    // order
+    if (atomicDirectiveDefaultOrderFound_) {
+      context_.Say(GetContext().clauseSource,
+          "REQUIRES directive with '%s' clause found lexically after atomic "
+          "operation without a memory order clause"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()));
+    }
+  } else {
+    // Check that it does not appear after a device construct
+    if (deviceConstructFound_) {
+      context_.Say(GetContext().clauseSource,
+          "REQUIRES directive with '%s' clause found lexically after device "
+          "construct"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()));
+    }
+  }
+}
+
 } // namespace Fortran::semantics

diff  --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index d165834196aefc..7e9c0a80300e26 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -205,6 +205,11 @@ class OmpStructureChecker
   void CheckPredefinedAllocatorRestriction(
       const parser::CharBlock &source, const parser::Name &name);
   bool isPredefinedAllocator{false};
+
+  void CheckAllowedRequiresClause(llvmOmpClause clause);
+  bool deviceConstructFound_{false};
+  bool atomicDirectiveDefaultOrderFound_{false};
+
   void EnterDirectiveNest(const int index) { directiveNest_[index]++; }
   void ExitDirectiveNest(const int index) { directiveNest_[index]--; }
   int GetDirectiveNest(const int index) { return directiveNest_[index]; }

diff  --git a/flang/test/Semantics/OpenMP/requires.f90 b/flang/test/Semantics/OpenMP/requires01.f90
similarity index 100%
rename from flang/test/Semantics/OpenMP/requires.f90
rename to flang/test/Semantics/OpenMP/requires01.f90

diff  --git a/flang/test/Semantics/OpenMP/requires02.f90 b/flang/test/Semantics/OpenMP/requires02.f90
new file mode 100644
index 00000000000000..974bcceb10c6f1
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires02.f90
@@ -0,0 +1,17 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! All atomic_default_mem_order clauses in 'requires' directives must come
+! strictly before any atomic directives on which the memory_order clause is not
+! specified.
+
+subroutine f
+  integer :: a = 0
+  !$omp atomic
+  a = a + 1
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'ATOMIC_DEFAULT_MEM_ORDER' clause found lexically after atomic operation without a memory order clause
+  !$omp requires atomic_default_mem_order(relaxed)
+end subroutine g

diff  --git a/flang/test/Semantics/OpenMP/requires03.f90 b/flang/test/Semantics/OpenMP/requires03.f90
new file mode 100644
index 00000000000000..4a23a6a4105fe2
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires03.f90
@@ -0,0 +1,21 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in 'requires' directives must come strictly before any
+! device constructs, such as target regions.
+
+subroutine f
+  !$omp target
+  !$omp end target
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'DYNAMIC_ALLOCATORS' clause found lexically after device construct
+  !$omp requires dynamic_allocators
+  !ERROR: REQUIRES directive with 'REVERSE_OFFLOAD' clause found lexically after device construct
+  !$omp requires reverse_offload
+  !ERROR: REQUIRES directive with 'UNIFIED_ADDRESS' clause found lexically after device construct
+  !$omp requires unified_address
+  !ERROR: REQUIRES directive with 'UNIFIED_SHARED_MEMORY' clause found lexically after device construct
+  !$omp requires unified_shared_memory
+end subroutine g

diff  --git a/flang/test/Semantics/OpenMP/requires04.f90 b/flang/test/Semantics/OpenMP/requires04.f90
new file mode 100644
index 00000000000000..ae1c2c08aa1c7e
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires04.f90
@@ -0,0 +1,20 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in 'requires' directives must come strictly before any
+! device constructs, such as declare target with device_type=nohost|any.
+
+subroutine f
+  !$omp declare target device_type(nohost)
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'DYNAMIC_ALLOCATORS' clause found lexically after device construct
+  !$omp requires dynamic_allocators
+  !ERROR: REQUIRES directive with 'REVERSE_OFFLOAD' clause found lexically after device construct
+  !$omp requires reverse_offload
+  !ERROR: REQUIRES directive with 'UNIFIED_ADDRESS' clause found lexically after device construct
+  !$omp requires unified_address
+  !ERROR: REQUIRES directive with 'UNIFIED_SHARED_MEMORY' clause found lexically after device construct
+  !$omp requires unified_shared_memory
+end subroutine g

diff  --git a/flang/test/Semantics/OpenMP/requires05.f90 b/flang/test/Semantics/OpenMP/requires05.f90
new file mode 100644
index 00000000000000..9281f8bab4a169
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires05.f90
@@ -0,0 +1,20 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in 'requires' directives must come strictly before any
+! device constructs, such as declare target with 'to' clause and no device_type.
+
+subroutine f
+  !$omp declare target to(f)
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'DYNAMIC_ALLOCATORS' clause found lexically after device construct
+  !$omp requires dynamic_allocators
+  !ERROR: REQUIRES directive with 'REVERSE_OFFLOAD' clause found lexically after device construct
+  !$omp requires reverse_offload
+  !ERROR: REQUIRES directive with 'UNIFIED_ADDRESS' clause found lexically after device construct
+  !$omp requires unified_address
+  !ERROR: REQUIRES directive with 'UNIFIED_SHARED_MEMORY' clause found lexically after device construct
+  !$omp requires unified_shared_memory
+end subroutine g

diff  --git a/flang/test/Semantics/OpenMP/requires06.f90 b/flang/test/Semantics/OpenMP/requires06.f90
new file mode 100644
index 00000000000000..ba9bbf31b6e070
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires06.f90
@@ -0,0 +1,20 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in 'requires' directives must come strictly before any
+! device constructs, such as declare target with extended list.
+
+subroutine f
+  !$omp declare target (f)
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'DYNAMIC_ALLOCATORS' clause found lexically after device construct
+  !$omp requires dynamic_allocators
+  !ERROR: REQUIRES directive with 'REVERSE_OFFLOAD' clause found lexically after device construct
+  !$omp requires reverse_offload
+  !ERROR: REQUIRES directive with 'UNIFIED_ADDRESS' clause found lexically after device construct
+  !$omp requires unified_address
+  !ERROR: REQUIRES directive with 'UNIFIED_SHARED_MEMORY' clause found lexically after device construct
+  !$omp requires unified_shared_memory
+end subroutine g

diff  --git a/flang/test/Semantics/OpenMP/requires07.f90 b/flang/test/Semantics/OpenMP/requires07.f90
new file mode 100644
index 00000000000000..2a36b4def9199c
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires07.f90
@@ -0,0 +1,21 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in 'requires' directives must come strictly before any
+! device constructs, such as target parallel regions.
+
+subroutine f
+  !$omp target parallel
+  !$omp end target parallel
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'DYNAMIC_ALLOCATORS' clause found lexically after device construct
+  !$omp requires dynamic_allocators
+  !ERROR: REQUIRES directive with 'REVERSE_OFFLOAD' clause found lexically after device construct
+  !$omp requires reverse_offload
+  !ERROR: REQUIRES directive with 'UNIFIED_ADDRESS' clause found lexically after device construct
+  !$omp requires unified_address
+  !ERROR: REQUIRES directive with 'UNIFIED_SHARED_MEMORY' clause found lexically after device construct
+  !$omp requires unified_shared_memory
+end subroutine g

diff  --git a/flang/test/Semantics/OpenMP/requires08.f90 b/flang/test/Semantics/OpenMP/requires08.f90
new file mode 100644
index 00000000000000..5f3b084078ccfd
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/requires08.f90
@@ -0,0 +1,23 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in 'requires' directives must come strictly before any
+! device constructs, such as target teams distribute parallel do loops.
+
+subroutine f
+  !$omp target teams distribute parallel do
+  do i=1, 10
+  end do
+  !$omp end target teams distribute parallel do
+end subroutine f
+
+subroutine g
+  !ERROR: REQUIRES directive with 'DYNAMIC_ALLOCATORS' clause found lexically after device construct
+  !$omp requires dynamic_allocators
+  !ERROR: REQUIRES directive with 'REVERSE_OFFLOAD' clause found lexically after device construct
+  !$omp requires reverse_offload
+  !ERROR: REQUIRES directive with 'UNIFIED_ADDRESS' clause found lexically after device construct
+  !$omp requires unified_address
+  !ERROR: REQUIRES directive with 'UNIFIED_SHARED_MEMORY' clause found lexically after device construct
+  !$omp requires unified_shared_memory
+end subroutine g


        


More information about the flang-commits mailing list