diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -569,7 +569,7 @@ // 2.4 Requires construct TYPE_PARSER(sourced(construct( - verbatim("REQUIRES"_tok), some(Parser{} / maybe(","_tok))))) + verbatim("REQUIRES"_tok), Parser{}))) // 2.15.2 Threadprivate directive TYPE_PARSER(sourced(construct( diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp --- a/flang/lib/Semantics/canonicalize-omp.cpp +++ b/flang/lib/Semantics/canonicalize-omp.cpp @@ -17,7 +17,10 @@ // after this pass. // 2. Associate declarative OMP allocation directives with their // respective executable allocation directive -// 3. TBD +// 3. set the default memory order, defined by any 'requires' directive +// present in the compilation unit, as explicit memory order of all atomic +// operations that don't already have it specified. +// 4. TBD namespace Fortran::semantics { using namespace parser::literals; @@ -50,6 +53,26 @@ void Post(parser::ExecutionPart &body) { RewriteOmpAllocations(body); } + // Assume at this point that any such clauses appear only on 'requires' + // directives that are visited before any atomic operations, and that all + // instances (if present) specify the same ordering. Other semantic checks + // ensure that these restrictions are met. + void Post(parser::OmpClause::AtomicDefaultMemOrder &x) { + switch (x.v.v) { + case parser::OmpAtomicDefaultMemOrderClause::Type::AcqRel: + atomicDefaultMemOrder_ = llvm::omp::Clause::OMPC_acq_rel; + break; + case parser::OmpAtomicDefaultMemOrderClause::Type::Relaxed: + atomicDefaultMemOrder_ = llvm::omp::Clause::OMPC_relaxed; + break; + case parser::OmpAtomicDefaultMemOrderClause::Type::SeqCst: + atomicDefaultMemOrder_ = llvm::omp::Clause::OMPC_seq_cst; + break; + } + } + + void Post(parser::OpenMPAtomicConstruct &atomic) { RewriteOmpAtomic(atomic); } + private: template T *GetConstructIf(parser::ExecutionPartConstruct &x) { if (auto *y{std::get_if(&x.u)}) { @@ -149,7 +172,68 @@ } } + void RewriteOmpAtomic(parser::OpenMPAtomicConstruct &atomic) { + // Rewrite atomic constructs to add an explicit memory ordering to all + // that do not specify it, honoring in this way the + // `atomic_default_mem_order` clause of the 'requires' directive. + if (!atomicDefaultMemOrder_) { + return; + } + + auto findMemOrderClause = + [](const std::list &clauses) { + return std::find_if( + clauses.begin(), clauses.end(), [](const auto &clause) { + return std::get_if( + &clause.u); + }) != clauses.end(); + }; + + // Get the clause list to which the new memory order clause must be added, + // only if there are no other memory order clauses present for this atomic + // directive. + auto *clauseList = common::visit( + common::visitors{ + [&](parser::OmpAtomic &atomicConstruct) { + // OmpAtomic only has a single list of clauses. + auto &clauses = + std::get(atomicConstruct.t); + return !findMemOrderClause(clauses.v) ? &clauses.v : nullptr; + }, + [&](auto &atomicConstruct) { + // All other atomic constructs have two lists of clauses. + auto &clausesLhs = std::get<0>(atomicConstruct.t); + auto &clausesRhs = std::get<2>(atomicConstruct.t); + return !findMemOrderClause(clausesLhs.v) && + !findMemOrderClause(clausesRhs.v) + ? &clausesRhs.v + : nullptr; + }}, + atomic.u); + + if (clauseList) { + switch (*atomicDefaultMemOrder_) { + case llvm::omp::Clause::OMPC_acq_rel: + clauseList->push_back(parser::OmpMemoryOrderClause( + parser::OmpClause(parser::OmpClause::AcqRel()))); + break; + case llvm::omp::Clause::OMPC_relaxed: + clauseList->push_back(parser::OmpMemoryOrderClause( + parser::OmpClause(parser::OmpClause::Relaxed()))); + break; + case llvm::omp::Clause::OMPC_seq_cst: + clauseList->push_back(parser::OmpMemoryOrderClause( + parser::OmpClause(parser::OmpClause::SeqCst()))); + break; + default: + llvm_unreachable("Unexpected clause used as memory ordering"); + break; + } + } + } + parser::Messages &messages_; + std::optional atomicDefaultMemOrder_; }; bool CanonicalizeOmp(parser::Messages &messages, parser::Program &program) { diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -268,6 +268,7 @@ const parser::OmpObjectList &ompObjectList); void CheckPredefinedAllocatorRestriction( const parser::CharBlock &source, const parser::Name &name); + void CheckAllowedRequiresClause(llvmOmpClause clause); bool isPredefinedAllocator{false}; void EnterDirectiveNest(const int index) { directiveNest_[index]++; } void ExitDirectiveNest(const int index) { directiveNest_[index]--; } @@ -281,6 +282,11 @@ LastType }; int directiveNest_[LastType + 1] = {0}; + + bool deviceConstructFound = false; + bool atomicDirectiveDefaultOrderFound = false; + std::optional + atomicDefaultMemoryOrder; }; } // namespace Fortran::semantics #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_ diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -789,6 +789,7 @@ switch (beginDir.v) { case llvm::omp::Directive::OMPD_target: + deviceConstructFound = true; if (CheckTargetBlockOnlyTeams(block)) { EnterDirectiveNest(TargetBlockOnlyTeams); } @@ -1155,21 +1156,40 @@ const auto &dir{std::get(x.t)}; const auto &spec{std::get(x.t)}; if (const auto *objectList{parser::Unwrap(spec.u)}) { + deviceConstructFound = true; CheckSymbolNames(dir.source, *objectList); CheckIsVarPartOfAnotherVar(dir.source, *objectList); CheckThreadprivateOrDeclareTargetVar(*objectList); } else if (const auto *clauseList{ parser::Unwrap(spec.u)}) { + bool toClauseFound = false, deviceTypeClauseFound = false; for (const auto &clause : clauseList->v) { - if (const auto *toClause{std::get_if(&clause.u)}) { - CheckSymbolNames(dir.source, toClause->v); - CheckIsVarPartOfAnotherVar(dir.source, toClause->v); - CheckThreadprivateOrDeclareTargetVar(toClause->v); - } else if (const auto *linkClause{ - std::get_if(&clause.u)}) { - CheckSymbolNames(dir.source, linkClause->v); - CheckIsVarPartOfAnotherVar(dir.source, linkClause->v); - CheckThreadprivateOrDeclareTargetVar(linkClause->v); + common::visit( + common::visitors{ + [&](const parser::OmpClause::To &toClause) { + CheckSymbolNames(dir.source, toClause.v); + CheckIsVarPartOfAnotherVar(dir.source, toClause.v); + CheckThreadprivateOrDeclareTargetVar(toClause.v); + toClauseFound = true; + }, + [&](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) { + deviceConstructFound = true; + } + }, + [&](const auto &) {}, + }, + clause.u); + + if (toClauseFound && !deviceTypeClauseFound) { + deviceConstructFound = true; } } } @@ -1671,6 +1691,9 @@ if (rightHandClauseList) { checkForValidMemoryOrderClause(rightHandClauseList); } + if (numMemoryOrderClause == 0) { + atomicDirectiveDefaultOrderFound = true; + } } void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { @@ -1867,7 +1890,6 @@ // 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(Allocate, OMPC_allocate) CHECK_SIMPLE_CLAUSE(Capture, OMPC_capture) @@ -1877,7 +1899,6 @@ 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) @@ -1890,7 +1911,6 @@ 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) @@ -1911,8 +1931,6 @@ 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) @@ -2836,4 +2854,61 @@ clause.u); } +void OmpStructureChecker::Enter( + const parser::OmpClause::AtomicDefaultMemOrder &x) { + CheckAllowed(llvm::omp::Clause::OMPC_atomic_default_mem_order); + + // Check that multiple occurrences of this clause match + parser::OmpAtomicDefaultMemOrderClause::Type memOrder = x.v.v; + if (atomicDefaultMemoryOrder && atomicDefaultMemoryOrder != memOrder) { + context_.Say(GetContext().clauseSource, + "Conflicting '%s' REQUIRES clauses found in compilation " + "unit"_err_en_US, + parser::ToUpperCaseLetters( + getClauseName(llvmOmpClause::OMPC_atomic_default_mem_order).str())); + return; + } + atomicDefaultMemoryOrder = memOrder; + // Check that it does not appear after an atomic operation without + // memory_order defined + if (atomicDirectiveDefaultOrderFound) { + context_.Say(GetContext().clauseSource, + "'%s' REQUIRES clause found lexically after atomic " + "operation with the '%s' clause not defined"_err_en_US, + parser::ToUpperCaseLetters( + getClauseName(llvmOmpClause::OMPC_atomic_default_mem_order).str()), + parser::ToUpperCaseLetters( + getClauseName(llvmOmpClause::OMPC_memory_order).str())); + } +} + +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); + + // Check that it does not appear after a device construct + if (deviceConstructFound) { + context_.Say(GetContext().clauseSource, + "'%s' REQUIRES clause found lexically after device " + "construct"_err_en_US, + parser::ToUpperCaseLetters(getClauseName(clause).str())); + } +} + } // namespace Fortran::semantics diff --git a/flang/test/Semantics/OpenMP/requires-rewrite.f90 b/flang/test/Semantics/OpenMP/requires-rewrite.f90 new file mode 100644 --- /dev/null +++ b/flang/test/Semantics/OpenMP/requires-rewrite.f90 @@ -0,0 +1,76 @@ +! RUN: %flang_fc1 -fopenmp -fdebug-dump-parse-tree %s 2>&1 | FileCheck %s +! Ensure that requires atomic_default_mem_order is used to update atomic +! operations with no explicit memory order set. +program requires + !$omp requires atomic_default_mem_order(seq_cst) + implicit none + integer :: i, j, k + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomic + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic relaxed + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomic + ! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst + !$omp atomic + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicUpdate + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic relaxed update + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicUpdate + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic update relaxed + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicUpdate + ! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst + !$omp atomic update + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic relaxed capture + i = j + j = k + !$omp end atomic + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic capture relaxed + i = j + j = k + !$omp end atomic + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture + ! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst + !$omp atomic capture + i = j + j = k + !$omp end atomic + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicWrite + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic relaxed write + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicWrite + ! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst + ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed + !$omp atomic write relaxed + i = j + + ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicWrite + ! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst + !$omp atomic write + i = j +end program requires diff --git a/flang/test/Semantics/OpenMP/requires01.f90 b/flang/test/Semantics/OpenMP/requires01.f90 new file mode 100644 --- /dev/null +++ b/flang/test/Semantics/OpenMP/requires01.f90 @@ -0,0 +1,14 @@ +! 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 found within a +! compilation unit must specify the same ordering. + +subroutine f + !$omp requires atomic_default_mem_order(seq_cst) +end subroutine f + +subroutine g + !ERROR: Conflicting 'ATOMIC_DEFAULT_MEM_ORDER' REQUIRES clauses found in compilation unit + !$omp requires atomic_default_mem_order(relaxed) +end subroutine g diff --git a/flang/test/Semantics/OpenMP/requires02.f90 b/flang/test/Semantics/OpenMP/requires02.f90 new file mode 100644 --- /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: 'ATOMIC_DEFAULT_MEM_ORDER' REQUIRES clause found lexically after atomic operation with the 'MEMORY_ORDER' clause not defined + !$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 --- /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: 'DYNAMIC_ALLOCATORS' REQUIRES clause found lexically after device construct + !$omp requires dynamic_allocators + !ERROR: 'REVERSE_OFFLOAD' REQUIRES clause found lexically after device construct + !$omp requires reverse_offload + !ERROR: 'UNIFIED_ADDRESS' REQUIRES clause found lexically after device construct + !$omp requires unified_address + !ERROR: 'UNIFIED_SHARED_MEMORY' REQUIRES 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 --- /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: 'DYNAMIC_ALLOCATORS' REQUIRES clause found lexically after device construct + !$omp requires dynamic_allocators + !ERROR: 'REVERSE_OFFLOAD' REQUIRES clause found lexically after device construct + !$omp requires reverse_offload + !ERROR: 'UNIFIED_ADDRESS' REQUIRES clause found lexically after device construct + !$omp requires unified_address + !ERROR: 'UNIFIED_SHARED_MEMORY' REQUIRES 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 --- /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: 'DYNAMIC_ALLOCATORS' REQUIRES clause found lexically after device construct + !$omp requires dynamic_allocators + !ERROR: 'REVERSE_OFFLOAD' REQUIRES clause found lexically after device construct + !$omp requires reverse_offload + !ERROR: 'UNIFIED_ADDRESS' REQUIRES clause found lexically after device construct + !$omp requires unified_address + !ERROR: 'UNIFIED_SHARED_MEMORY' REQUIRES 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 --- /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: 'DYNAMIC_ALLOCATORS' REQUIRES clause found lexically after device construct + !$omp requires dynamic_allocators + !ERROR: 'REVERSE_OFFLOAD' REQUIRES clause found lexically after device construct + !$omp requires reverse_offload + !ERROR: 'UNIFIED_ADDRESS' REQUIRES clause found lexically after device construct + !$omp requires unified_address + !ERROR: 'UNIFIED_SHARED_MEMORY' REQUIRES clause found lexically after device construct + !$omp requires unified_shared_memory +end subroutine g