From af48237ec9c683bf3cf9c4cb70f4d5c79012c700 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 18 Apr 2025 08:39:36 -0500 Subject: [PATCH 1/2] [flang][OpenMP] Introduce OmpHintClause, simplify OmpAtomicClause The OmpAtomicClause is a variant of a few specific clauses that are used on the ATOMIC construct. The HINT clause, however, was represented as a generic OmpClause, which somewhat complicated the analysis of an OmpAtomicClause. Introduce OmpHintClause to represent the contents of the HINT clause, and use it on OmpAtomicClause similarly to how OmpFailClause is used. --- flang/include/flang/Lower/DirectivesCommon.h | 13 ++-- flang/include/flang/Parser/dump-parse-tree.h | 1 + flang/include/flang/Parser/parse-tree.h | 7 +- flang/lib/Lower/OpenMP/Clauses.cpp | 4 +- flang/lib/Parser/openmp-parsers.cpp | 11 +-- flang/lib/Parser/unparse.cpp | 7 +- flang/lib/Semantics/check-omp-structure.cpp | 67 +++++++++++-------- flang/lib/Semantics/check-omp-structure.h | 2 +- .../Semantics/OpenMP/atomic-hint-clause.f90 | 1 + .../Semantics/OpenMP/critical-hint-clause.f90 | 1 + llvm/include/llvm/Frontend/OpenMP/OMP.td | 2 +- 11 files changed, 69 insertions(+), 47 deletions(-) diff --git a/flang/include/flang/Lower/DirectivesCommon.h b/flang/include/flang/Lower/DirectivesCommon.h index 6e24343cebd3a..b37097289eb83 100644 --- a/flang/include/flang/Lower/DirectivesCommon.h +++ b/flang/include/flang/Lower/DirectivesCommon.h @@ -55,14 +55,11 @@ static inline void genOmpAtomicHintAndMemoryOrderClauses( mlir::omp::ClauseMemoryOrderKindAttr &memoryOrder) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); for (const Fortran::parser::OmpAtomicClause &clause : clauseList.v) { - if (const auto *ompClause = - std::get_if(&clause.u)) { - if (const auto *hintClause = - std::get_if(&ompClause->u)) { - const auto *expr = Fortran::semantics::GetExpr(hintClause->v); - uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr); - hint = firOpBuilder.getI64IntegerAttr(hintExprValue); - } + if (const auto *hintClause = + std::get_if(&clause.u)) { + const auto *expr = Fortran::semantics::GetExpr(hintClause->v); + uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr); + hint = firOpBuilder.getI64IntegerAttr(hintExprValue); } else if (const auto *ompMemoryOrderClause = std::get_if( &clause.u)) { diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index c66f0735f33f0..42f2ff376c650 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -591,6 +591,7 @@ class ParseTreeDumper { NODE(OmpFromClause, Modifier) NODE(parser, OmpExpectation) NODE_ENUM(OmpExpectation, Value) + NODE(parser, OmpHintClause) NODE(parser, OmpHoldsClause) NODE(parser, OmpIfClause) NODE(OmpIfClause, Modifier) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 0c2a5de3b71d2..2f45faec42c8f 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -4272,6 +4272,11 @@ struct OmpGrainsizeClause { std::tuple t; }; +// Ref: [5.0:234-242], [5.1:266-275], [5.2:299], [6.0:472-473] +struct OmpHintClause { + WRAPPER_CLASS_BOILERPLATE(OmpHintClause, ScalarIntConstantExpr); +}; + // Ref: [5.2: 214] // // holds-clause -> @@ -4832,7 +4837,7 @@ struct OmpMemoryOrderClause { struct OmpAtomicClause { UNION_CLASS_BOILERPLATE(OmpAtomicClause); CharBlock source; - std::variant u; + std::variant u; }; // atomic-clause-list -> [atomic-clause, [atomic-clause], ...] diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 3ffdf7138b035..3f0748382116b 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -863,8 +863,8 @@ HasDeviceAddr make(const parser::OmpClause::HasDeviceAddr &inp, Hint make(const parser::OmpClause::Hint &inp, semantics::SemanticsContext &semaCtx) { - // inp.v -> parser::ConstantExpr - return Hint{/*HintExpr=*/makeExpr(inp.v, semaCtx)}; + // inp.v -> parser::OmpHintClause + return Hint{/*HintExpr=*/makeExpr(inp.v.v, semaCtx)}; } Holds make(const parser::OmpClause::Holds &inp, diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index e84abf87c1cb6..57324810eac1e 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -804,6 +804,8 @@ TYPE_PARSER( // OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle) TYPE_PARSER(construct(Parser{})) +TYPE_PARSER(construct(scalarIntConstantExpr)) + // init clause TYPE_PARSER(construct( maybe(nonemptyList(Parser{}) / ":"), @@ -941,8 +943,8 @@ TYPE_PARSER( // "HAS_DEVICE_ADDR" >> construct(construct( parenthesized(Parser{}))) || - "HINT" >> construct( - construct(parenthesized(constantExpr))) || + "HINT" >> construct(construct( + parenthesized(Parser{}))) || "HOLDS" >> construct(construct( parenthesized(Parser{}))) || "IF" >> construct(construct( @@ -1217,9 +1219,8 @@ TYPE_PARSER(construct( TYPE_PARSER(sourced(construct( construct(Parser{}) || construct("FAIL" >> Parser{}) || - construct("HINT" >> - sourced(construct( - construct(parenthesized(constantExpr)))))))) + construct( + "HINT" >> parenthesized(Parser{}))))) // atomic-clause-list -> [atomic-clause, [atomic-clause], ...] TYPE_PARSER(sourced(construct( diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 47dae0ae753d2..d18f3e43d27d2 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2890,12 +2890,17 @@ class UnparseVisitor { Walk(x.v); Put(")"); } + void Unparse(const OmpHintClause &x) { + Word("HINT("); + Walk(x.v); + Put(")"); + } void Unparse(const OmpMemoryOrderClause &x) { Walk(x.v); } void Unparse(const OmpAtomicClause &x) { common::visit(common::visitors{ [&](const OmpMemoryOrderClause &y) { Walk(y); }, [&](const OmpFailClause &y) { Walk(y); }, - [&](const OmpClause &z) { Walk(z); }, + [&](const OmpHintClause &y) { Walk(y); }, }, x.u); } diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index bc4651584b9a8..c8a905451cd03 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -586,33 +586,40 @@ void OmpStructureChecker::CheckPredefinedAllocatorRestriction( template void OmpStructureChecker::CheckHintClause( - D *leftOmpClauseList, D *rightOmpClauseList) { + D *leftOmpClauseList, D *rightOmpClauseList, std::string_view dirName) { + bool foundHint{false}; + auto checkForValidHintClause = [&](const D *clauseList) { for (const auto &clause : clauseList->v) { - const parser::OmpClause *ompClause = nullptr; + const parser::OmpHintClause *ompHintClause = nullptr; if constexpr (std::is_same_v) { - ompClause = std::get_if(&clause.u); - if (!ompClause) - continue; + ompHintClause = std::get_if(&clause.u); } else if constexpr (std::is_same_v) { - ompClause = &clause; + if (auto *hint{std::get_if(&clause.u)}) { + ompHintClause = &hint->v; + } } - if (const parser::OmpClause::Hint *hintClause{ - std::get_if(&ompClause->u)}) { - std::optional hintValue = GetIntValue(hintClause->v); - if (hintValue && *hintValue >= 0) { - /*`omp_sync_hint_nonspeculative` and `omp_lock_hint_speculative`*/ - if ((*hintValue & 0xC) == 0xC - /*`omp_sync_hint_uncontended` and omp_sync_hint_contended*/ - || (*hintValue & 0x3) == 0x3) - context_.Say(clause.source, - "Hint clause value " - "is not a valid OpenMP synchronization value"_err_en_US); - } else { + if (!ompHintClause) + continue; + if (foundHint) { + context_.Say(clause.source, + "At most one HINT clause can appear on the %s directive"_err_en_US, + parser::ToUpperCaseLetters(dirName)); + } + foundHint = true; + std::optional hintValue = GetIntValue(ompHintClause->v); + if (hintValue && *hintValue >= 0) { + /*`omp_sync_hint_nonspeculative` and `omp_lock_hint_speculative`*/ + if ((*hintValue & 0xC) == 0xC + /*`omp_sync_hint_uncontended` and omp_sync_hint_contended*/ + || (*hintValue & 0x3) == 0x3) context_.Say(clause.source, - "Hint clause must have non-negative constant " - "integer expression"_err_en_US); - } + "Hint clause value " + "is not a valid OpenMP synchronization value"_err_en_US); + } else { + context_.Say(clause.source, + "Hint clause must have non-negative constant " + "integer expression"_err_en_US); } } }; @@ -2375,7 +2382,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPCriticalConstruct &x) { "Hint clause other than omp_sync_hint_none cannot be specified for " "an unnamed CRITICAL directive"_err_en_US}); } - CheckHintClause(&ompClause, nullptr); + CheckHintClause(&ompClause, nullptr, "CRITICAL"); } void OmpStructureChecker::Leave(const parser::OpenMPCriticalConstruct &) { @@ -2950,7 +2957,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { nullptr); CheckHintClause( &std::get(atomicConstruct.t), - nullptr); + nullptr, "ATOMIC"); }, [&](const parser::OmpAtomicUpdate &atomicUpdate) { const auto &dir{std::get(atomicUpdate.t)}; @@ -2963,7 +2970,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { CheckAtomicMemoryOrderClause( &std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t)); CheckHintClause( - &std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t)); + &std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t), + "UPDATE"); }, [&](const parser::OmpAtomicRead &atomicRead) { const auto &dir{std::get(atomicRead.t)}; @@ -2972,7 +2980,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { CheckAtomicMemoryOrderClause( &std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t)); CheckHintClause( - &std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t)); + &std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t), "READ"); CheckAtomicCaptureStmt( std::get>( atomicRead.t) @@ -2985,7 +2993,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { CheckAtomicMemoryOrderClause( &std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t)); CheckHintClause( - &std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t)); + &std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t), + "WRITE"); CheckAtomicWriteStmt( std::get>( atomicWrite.t) @@ -2998,7 +3007,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { CheckAtomicMemoryOrderClause( &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t)); CheckHintClause( - &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t)); + &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t), + "CAPTURE"); CheckAtomicCaptureConstruct(atomicCapture); }, [&](const parser::OmpAtomicCompare &atomicCompare) { @@ -3008,7 +3018,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) { CheckAtomicMemoryOrderClause( &std::get<0>(atomicCompare.t), &std::get<2>(atomicCompare.t)); CheckHintClause( - &std::get<0>(atomicCompare.t), &std::get<2>(atomicCompare.t)); + &std::get<0>(atomicCompare.t), &std::get<2>(atomicCompare.t), + "CAPTURE"); CheckAtomicCompareConstruct(atomicCompare); }, }, diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index 5af3b505937c1..87130f51b85f6 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -323,7 +323,7 @@ class OmpStructureChecker void EnterDirectiveNest(const int index) { directiveNest_[index]++; } void ExitDirectiveNest(const int index) { directiveNest_[index]--; } int GetDirectiveNest(const int index) { return directiveNest_[index]; } - template void CheckHintClause(D *, D *); + template void CheckHintClause(D *, D *, std::string_view); inline void ErrIfAllocatableVariable(const parser::Variable &); inline void ErrIfLHSAndRHSSymbolsMatch( const parser::Variable &, const parser::Expr &); diff --git a/flang/test/Semantics/OpenMP/atomic-hint-clause.f90 b/flang/test/Semantics/OpenMP/atomic-hint-clause.f90 index f724a69345f6e..c13a11a8dd5dc 100644 --- a/flang/test/Semantics/OpenMP/atomic-hint-clause.f90 +++ b/flang/test/Semantics/OpenMP/atomic-hint-clause.f90 @@ -78,6 +78,7 @@ program sample y = y * 9 !ERROR: Hint clause must have non-negative constant integer expression + !ERROR: Must have INTEGER type, but is REAL(4) !$omp atomic hint(1.0) read y = x diff --git a/flang/test/Semantics/OpenMP/critical-hint-clause.f90 b/flang/test/Semantics/OpenMP/critical-hint-clause.f90 index 419187fa3bbfb..7ca8c858239f7 100644 --- a/flang/test/Semantics/OpenMP/critical-hint-clause.f90 +++ b/flang/test/Semantics/OpenMP/critical-hint-clause.f90 @@ -95,6 +95,7 @@ program sample !$omp end critical (name) !ERROR: Hint clause must have non-negative constant integer expression + !ERROR: Must have INTEGER type, but is REAL(4) !$omp critical (name) hint(1.0) y = 2 !$omp end critical (name) diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td index e2a1449d8cc76..6e0fb10fabe29 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.td +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td @@ -224,7 +224,7 @@ def OMPC_HasDeviceAddr : Clause<"has_device_addr"> { } def OMPC_Hint : Clause<"hint"> { let clangClass = "OMPHintClause"; - let flangClass = "ConstantExpr"; + let flangClass = "OmpHintClause"; } def OMPC_Holds : Clause<"holds"> { let clangClass = "OMPHoldsClause"; From 8df469018b0a1c09f45636364f96226b7c82d2c3 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 18 Apr 2025 09:06:37 -0500 Subject: [PATCH 2/2] [flang][OpenMP] Extend common::AtomicDefaultMemOrderType enumeration Add "Acquire" and "Release", and rename it to OmpMemoryOrderType, since memory order type is a concept extending beyond the ATOMIC_DEFAULT_MEM_ORDER clause. When processing a REQUIRES directive (in rewrite-directives.cpp), do not add Acquire or Release to ATOMIC constructs, because handling of those types depends on the OpenMP version, which is not available in that file. This issue will be addressed later. --- flang/examples/FeatureList/FeatureList.cpp | 2 +- flang/include/flang/Lower/DirectivesCommon.h | 66 +++++++++++--------- flang/include/flang/Parser/dump-parse-tree.h | 2 +- flang/include/flang/Parser/parse-tree.h | 6 +- flang/include/flang/Semantics/symbol.h | 2 +- flang/include/flang/Support/Fortran.h | 4 +- flang/lib/Lower/OpenMP/Clauses.cpp | 9 +-- flang/lib/Parser/openmp-parsers.cpp | 36 ++++++----- flang/lib/Parser/unparse.cpp | 4 +- flang/lib/Semantics/resolve-directives.cpp | 10 +-- flang/lib/Semantics/rewrite-directives.cpp | 12 ++-- 11 files changed, 84 insertions(+), 69 deletions(-) diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp index 94fdfa3e4dea9..d1407cf0ef239 100644 --- a/flang/examples/FeatureList/FeatureList.cpp +++ b/flang/examples/FeatureList/FeatureList.cpp @@ -564,11 +564,11 @@ struct NodeVisitor { READ_FEATURE(OpenMPDeclareReductionConstruct) READ_FEATURE(OpenMPDeclareSimdConstruct) READ_FEATURE(OpenMPDeclareTargetConstruct) + READ_FEATURE(OmpMemoryOrderType) READ_FEATURE(OmpMemoryOrderClause) READ_FEATURE(OmpAtomicClause) READ_FEATURE(OmpAtomicClauseList) READ_FEATURE(OmpAtomicDefaultMemOrderClause) - READ_FEATURE(OmpAtomicDefaultMemOrderType) READ_FEATURE(OpenMPFlushConstruct) READ_FEATURE(OpenMPLoopConstruct) READ_FEATURE(OpenMPExecutableAllocate) diff --git a/flang/include/flang/Lower/DirectivesCommon.h b/flang/include/flang/Lower/DirectivesCommon.h index b37097289eb83..df84c7d46d231 100644 --- a/flang/include/flang/Lower/DirectivesCommon.h +++ b/flang/include/flang/Lower/DirectivesCommon.h @@ -55,36 +55,42 @@ static inline void genOmpAtomicHintAndMemoryOrderClauses( mlir::omp::ClauseMemoryOrderKindAttr &memoryOrder) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); for (const Fortran::parser::OmpAtomicClause &clause : clauseList.v) { - if (const auto *hintClause = - std::get_if(&clause.u)) { - const auto *expr = Fortran::semantics::GetExpr(hintClause->v); - uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr); - hint = firOpBuilder.getI64IntegerAttr(hintExprValue); - } else if (const auto *ompMemoryOrderClause = - std::get_if( - &clause.u)) { - if (std::get_if( - &ompMemoryOrderClause->v.u)) { - memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get( - firOpBuilder.getContext(), - mlir::omp::ClauseMemoryOrderKind::Acquire); - } else if (std::get_if( - &ompMemoryOrderClause->v.u)) { - memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get( - firOpBuilder.getContext(), - mlir::omp::ClauseMemoryOrderKind::Relaxed); - } else if (std::get_if( - &ompMemoryOrderClause->v.u)) { - memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get( - firOpBuilder.getContext(), - mlir::omp::ClauseMemoryOrderKind::Seq_cst); - } else if (std::get_if( - &ompMemoryOrderClause->v.u)) { - memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get( - firOpBuilder.getContext(), - mlir::omp::ClauseMemoryOrderKind::Release); - } - } + common::visit( + common::visitors{ + [&](const parser::OmpMemoryOrderClause &s) { + auto kind = common::visit( + common::visitors{ + [&](const parser::OmpClause::AcqRel &) { + return mlir::omp::ClauseMemoryOrderKind::Acq_rel; + }, + [&](const parser::OmpClause::Acquire &) { + return mlir::omp::ClauseMemoryOrderKind::Acquire; + }, + [&](const parser::OmpClause::Relaxed &) { + return mlir::omp::ClauseMemoryOrderKind::Relaxed; + }, + [&](const parser::OmpClause::Release &) { + return mlir::omp::ClauseMemoryOrderKind::Release; + }, + [&](const parser::OmpClause::SeqCst &) { + return mlir::omp::ClauseMemoryOrderKind::Seq_cst; + }, + [&](auto &&) -> mlir::omp::ClauseMemoryOrderKind { + llvm_unreachable("Unexpected clause"); + }, + }, + s.v.u); + memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get( + firOpBuilder.getContext(), kind); + }, + [&](const parser::OmpHintClause &s) { + const auto *expr = Fortran::semantics::GetExpr(s.v); + uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr); + hint = firOpBuilder.getI64IntegerAttr(hintExprValue); + }, + [&](const parser::OmpFailClause &) {}, + }, + clause.u); } } diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 42f2ff376c650..c0cf90c4696b6 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -707,11 +707,11 @@ class ParseTreeDumper { NODE(parser, OpenMPDeclareSimdConstruct) NODE(parser, OpenMPDeclareTargetConstruct) NODE(parser, OpenMPDeclareMapperConstruct) + NODE_ENUM(common, OmpMemoryOrderType) NODE(parser, OmpMemoryOrderClause) NODE(parser, OmpAtomicClause) NODE(parser, OmpAtomicClauseList) NODE(parser, OmpAtomicDefaultMemOrderClause) - NODE_ENUM(common, OmpAtomicDefaultMemOrderType) NODE(parser, OpenMPDepobjConstruct) NODE(parser, OpenMPUtilityConstruct) NODE(parser, OpenMPDispatchConstruct) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 2f45faec42c8f..9061130202b08 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -4071,7 +4071,7 @@ struct OmpAtClause { // SEQ_CST | ACQ_REL | RELAXED | // since 5.0 // ACQUIRE | RELEASE // since 5.2 struct OmpAtomicDefaultMemOrderClause { - using MemoryOrder = common::OmpAtomicDefaultMemOrderType; + using MemoryOrder = common::OmpMemoryOrderType; WRAPPER_CLASS_BOILERPLATE(OmpAtomicDefaultMemOrderClause, MemoryOrder); }; @@ -4822,10 +4822,10 @@ struct OpenMPAllocatorsConstruct { // 2.17.7 Atomic construct/2.17.8 Flush construct [OpenMP 5.0] // memory-order-clause -> acq_rel -// release // acquire -// seq_cst +// release // relaxed +// seq_cst struct OmpMemoryOrderClause { WRAPPER_CLASS_BOILERPLATE(OmpMemoryOrderClause, OmpClause); CharBlock source; diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h index 715811885c219..36d926a8a4bc5 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -48,7 +48,7 @@ using MutableSymbolVector = std::vector; // Mixin for details with OpenMP declarative constructs. class WithOmpDeclarative { - using OmpAtomicOrderType = common::OmpAtomicDefaultMemOrderType; + using OmpAtomicOrderType = common::OmpMemoryOrderType; public: ENUM_CLASS(RequiresFlag, ReverseOffload, UnifiedAddress, UnifiedSharedMemory, diff --git a/flang/include/flang/Support/Fortran.h b/flang/include/flang/Support/Fortran.h index 6ce053926c1e7..0b4fc1a608e9e 100644 --- a/flang/include/flang/Support/Fortran.h +++ b/flang/include/flang/Support/Fortran.h @@ -72,8 +72,8 @@ ENUM_CLASS( ENUM_CLASS( OpenACCDeviceType, Star, Default, Nvidia, Radeon, Host, Multicore, None) -// OpenMP atomic_default_mem_order clause allowed values -ENUM_CLASS(OmpAtomicDefaultMemOrderType, SeqCst, AcqRel, Relaxed) +// OpenMP memory-order types +ENUM_CLASS(OmpMemoryOrderType, Acq_Rel, Acquire, Relaxed, Release, Seq_Cst) // Fortran names may have up to 63 characters (See Fortran 2018 C601). static constexpr int maxNameLen{63}; diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 3f0748382116b..57c2870f8d293 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -494,12 +494,13 @@ AtomicDefaultMemOrder make(const parser::OmpClause::AtomicDefaultMemOrder &inp, semantics::SemanticsContext &semaCtx) { // inp.v -> parser::OmpAtomicDefaultMemOrderClause CLAUSET_ENUM_CONVERT( // - convert, common::OmpAtomicDefaultMemOrderType, - AtomicDefaultMemOrder::MemoryOrder, + convert, common::OmpMemoryOrderType, AtomicDefaultMemOrder::MemoryOrder, // clang-format off - MS(AcqRel, AcqRel) + MS(Acq_Rel, AcqRel) + MS(Acquire, Acquire) MS(Relaxed, Relaxed) - MS(SeqCst, SeqCst) + MS(Release, Release) + MS(Seq_Cst, SeqCst) // clang-format on ); diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 57324810eac1e..0d20cce1b0371 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -636,6 +636,20 @@ TYPE_PARSER(construct( maybe(nonemptyList(Parser{}) / ":"), Parser{})) +// 2.4 Requires construct [OpenMP 5.0] +// atomic-default-mem-order-clause -> +// acq_rel +// acquire +// relaxed +// release +// seq_cst +TYPE_PARSER(construct( + "ACQ_REL" >> pure(common::OmpMemoryOrderType::Acq_Rel) || + "ACQUIRE" >> pure(common::OmpMemoryOrderType::Acquire) || + "RELAXED" >> pure(common::OmpMemoryOrderType::Relaxed) || + "RELEASE" >> pure(common::OmpMemoryOrderType::Release) || + "SEQ_CST" >> pure(common::OmpMemoryOrderType::Seq_Cst))) + TYPE_PARSER(construct( OmpDirectiveNameParser{}, maybe(parenthesized(scalarLogicalExpr)))) @@ -1192,27 +1206,17 @@ TYPE_PARSER(sourced(construct( // 2.17.7 Atomic construct/2.17.8 Flush construct [OpenMP 5.0] // memory-order-clause -> -// seq_cst // acq_rel -// release // acquire // relaxed +// release +// seq_cst TYPE_PARSER(sourced(construct( - sourced("SEQ_CST" >> construct(construct()) || - "ACQ_REL" >> construct(construct()) || - "RELEASE" >> construct(construct()) || + sourced("ACQ_REL" >> construct(construct()) || "ACQUIRE" >> construct(construct()) || - "RELAXED" >> construct(construct()))))) - -// 2.4 Requires construct [OpenMP 5.0] -// atomic-default-mem-order-clause -> -// seq_cst -// acq_rel -// relaxed -TYPE_PARSER(construct( - "SEQ_CST" >> pure(common::OmpAtomicDefaultMemOrderType::SeqCst) || - "ACQ_REL" >> pure(common::OmpAtomicDefaultMemOrderType::AcqRel) || - "RELAXED" >> pure(common::OmpAtomicDefaultMemOrderType::Relaxed))) + "RELAXED" >> construct(construct()) || + "RELEASE" >> construct(construct()) || + "SEQ_CST" >> construct(construct()))))) // 2.17.7 Atomic construct // atomic-clause -> memory-order-clause | HINT(hint-expression) diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index d18f3e43d27d2..74b3f7b9dda22 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2558,8 +2558,8 @@ class UnparseVisitor { } } - void Unparse(const OmpAtomicDefaultMemOrderClause &x) { - Word(ToUpperCaseLetters(common::EnumToString(x.v))); + void Unparse(const common::OmpMemoryOrderType &x) { + Word(ToUpperCaseLetters(common::EnumToString(x))); } void Unparse(const OmpAtomicClauseList &x) { Walk(" ", x.v, " "); } diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index c50724fe9d35d..620a37cb40231 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -416,7 +416,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { // Gather information from the clauses. Flags flags; - std::optional memOrder; + std::optional memOrder; for (const auto &clause : std::get(x.t).v) { flags |= common::visit( common::visitors{ @@ -799,7 +799,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { std::int64_t ordCollapseLevel{0}; void AddOmpRequiresToScope(Scope &, WithOmpDeclarative::RequiresFlags, - std::optional); + std::optional); void IssueNonConformanceWarning( llvm::omp::Directive D, parser::CharBlock source); @@ -2721,7 +2721,7 @@ void ResolveOmpTopLevelParts( // program units. Modules are skipped because their REQUIRES clauses should be // propagated via USE statements instead. WithOmpDeclarative::RequiresFlags combinedFlags; - std::optional combinedMemOrder; + std::optional combinedMemOrder; // Function to go through non-module top level program units and extract // REQUIRES information to be processed by a function-like argument. @@ -2764,7 +2764,7 @@ void ResolveOmpTopLevelParts( flags{details.ompRequires()}) { combinedFlags |= *flags; } - if (const common::OmpAtomicDefaultMemOrderType * + if (const common::OmpMemoryOrderType * memOrder{details.ompAtomicDefaultMemOrder()}) { if (combinedMemOrder && *combinedMemOrder != *memOrder) { context.Say(symbol.scope()->sourceRange(), @@ -2983,7 +2983,7 @@ void OmpAttributeVisitor::CheckNameInAllocateStmt( void OmpAttributeVisitor::AddOmpRequiresToScope(Scope &scope, WithOmpDeclarative::RequiresFlags flags, - std::optional memOrder) { + std::optional memOrder) { Scope *scopeIter = &scope; do { if (Symbol * symbol{scopeIter->symbol()}) { diff --git a/flang/lib/Semantics/rewrite-directives.cpp b/flang/lib/Semantics/rewrite-directives.cpp index c94d0f3855bee..104a77885d276 100644 --- a/flang/lib/Semantics/rewrite-directives.cpp +++ b/flang/lib/Semantics/rewrite-directives.cpp @@ -70,7 +70,7 @@ bool OmpRewriteMutator::Pre(parser::OpenMPAtomicConstruct &x) { x.u)}; // Get the `atomic_default_mem_order` clause from the top-level parent. - std::optional defaultMemOrder; + std::optional defaultMemOrder; common::visit( [&](auto &details) { if constexpr (std::is_convertible_vemplace_back(common::visit( common::visitors{[](parser::OmpAtomicRead &) -> parser::OmpClause { return parser::OmpClause::Acquire{}; @@ -133,14 +133,18 @@ bool OmpRewriteMutator::Pre(parser::OpenMPAtomicConstruct &x) { }}, x.u)); break; - case common::OmpAtomicDefaultMemOrderType::Relaxed: + case common::OmpMemoryOrderType::Relaxed: clauseList->emplace_back( parser::OmpClause{parser::OmpClause::Relaxed{}}); break; - case common::OmpAtomicDefaultMemOrderType::SeqCst: + case common::OmpMemoryOrderType::Seq_Cst: clauseList->emplace_back( parser::OmpClause{parser::OmpClause::SeqCst{}}); break; + default: + // FIXME: Don't process other values at the moment since their validity + // depends on the OpenMP version (which is unavailable here). + break; } }