[flang][OpenMP] Extend common::AtomicDefaultMemOrderType enumeration #136312
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-fir-hlfir Author: Krzysztof Parzyszek (kparzysz) ChangesAdd "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. Full diff: https://github.com/llvm/llvm-project/pull/136312.diff 11 Files Affected:
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<Fortran::parser::OmpHintClause>(&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<Fortran::parser::OmpMemoryOrderClause>(
- &clause.u)) {
- if (std::get_if<Fortran::parser::OmpClause::Acquire>(
- &ompMemoryOrderClause->v.u)) {
- memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get(
- firOpBuilder.getContext(),
- mlir::omp::ClauseMemoryOrderKind::Acquire);
- } else if (std::get_if<Fortran::parser::OmpClause::Relaxed>(
- &ompMemoryOrderClause->v.u)) {
- memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get(
- firOpBuilder.getContext(),
- mlir::omp::ClauseMemoryOrderKind::Relaxed);
- } else if (std::get_if<Fortran::parser::OmpClause::SeqCst>(
- &ompMemoryOrderClause->v.u)) {
- memoryOrder = mlir::omp::ClauseMemoryOrderKindAttr::get(
- firOpBuilder.getContext(),
- mlir::omp::ClauseMemoryOrderKind::Seq_cst);
- } else if (std::get_if<Fortran::parser::OmpClause::Release>(
- &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<MutableSymbolRef>;
// 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<OmpAffinityClause>(
maybe(nonemptyList(Parser<OmpAffinityClause::Modifier>{}) / ":"),
Parser<OmpObjectList>{}))
+// 2.4 Requires construct [OpenMP 5.0]
+// atomic-default-mem-order-clause ->
+// acq_rel
+// acquire
+// relaxed
+// release
+// seq_cst
+TYPE_PARSER(construct<OmpAtomicDefaultMemOrderClause>(
+ "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<OmpCancellationConstructTypeClause>(
OmpDirectiveNameParser{}, maybe(parenthesized(scalarLogicalExpr))))
@@ -1192,27 +1206,17 @@ TYPE_PARSER(sourced(construct<OmpFailClause>(
// 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<OmpMemoryOrderClause>(
- sourced("SEQ_CST" >> construct<OmpClause>(construct<OmpClause::SeqCst>()) ||
- "ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
- "RELEASE" >> construct<OmpClause>(construct<OmpClause::Release>()) ||
+ sourced("ACQ_REL" >> construct<OmpClause>(construct<OmpClause::AcqRel>()) ||
"ACQUIRE" >> construct<OmpClause>(construct<OmpClause::Acquire>()) ||
- "RELAXED" >> construct<OmpClause>(construct<OmpClause::Relaxed>())))))
-
-// 2.4 Requires construct [OpenMP 5.0]
-// atomic-default-mem-order-clause ->
-// seq_cst
-// acq_rel
-// relaxed
-TYPE_PARSER(construct<OmpAtomicDefaultMemOrderClause>(
- "SEQ_CST" >> pure(common::OmpAtomicDefaultMemOrderType::SeqCst) ||
- "ACQ_REL" >> pure(common::OmpAtomicDefaultMemOrderType::AcqRel) ||
- "RELAXED" >> pure(common::OmpAtomicDefaultMemOrderType::Relaxed)))
+ "RELAXED" >> construct<OmpClause>(construct<OmpClause::Relaxed>()) ||
+ "RELEASE" >> construct<OmpClause>(construct<OmpClause::Release>()) ||
+ "SEQ_CST" >> construct<OmpClause>(construct<OmpClause::SeqCst>())))))
// 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..4ffe20afc704f 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -416,7 +416,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
// Gather information from the clauses.
Flags flags;
- std::optional<common::OmpAtomicDefaultMemOrderType> memOrder;
+ std::optional<common::OmpMemoryOrderType> memOrder;
for (const auto &clause : std::get<parser::OmpClauseList>(x.t).v) {
flags |= common::visit(
common::visitors{
@@ -799,7 +799,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
std::int64_t ordCollapseLevel{0};
void AddOmpRequiresToScope(Scope &, WithOmpDeclarative::RequiresFlags,
- std::optional<common::OmpAtomicDefaultMemOrderType>);
+ std::optional<common::OmpMemoryOrderType>);
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<common::OmpAtomicDefaultMemOrderType> combinedMemOrder;
+ std::optional<common::OmpMemoryOrderType> combinedMemOrder;
// Function to go through non-module top level program units and extract
// REQUIRES information to be processed by a function-like argument.
@@ -2764,8 +2764,8 @@ void ResolveOmpTopLevelParts(
flags{details.ompRequires()}) {
combinedFlags |= *flags;
}
- if (const common::OmpAtomicDefaultMemOrderType *
- memOrder{details.ompAtomicDefaultMemOrder()}) {
+ if (const common::OmpMemoryOrderType *memOrder{
+ details.ompAtomicDefaultMemOrder()}) {
if (combinedMemOrder && *combinedMemOrder != *memOrder) {
context.Say(symbol.scope()->sourceRange(),
"Conflicting '%s' REQUIRES clauses found in compilation "
@@ -2983,7 +2983,7 @@ void OmpAttributeVisitor::CheckNameInAllocateStmt(
void OmpAttributeVisitor::AddOmpRequiresToScope(Scope &scope,
WithOmpDeclarative::RequiresFlags flags,
- std::optional<common::OmpAtomicDefaultMemOrderType> memOrder) {
+ std::optional<common::OmpMemoryOrderType> 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<common::OmpAtomicDefaultMemOrderType> defaultMemOrder;
+ std::optional<common::OmpMemoryOrderType> defaultMemOrder;
common::visit(
[&](auto &details) {
if constexpr (std::is_convertible_v<decltype(&details),
@@ -119,7 +119,7 @@ bool OmpRewriteMutator::Pre(parser::OpenMPAtomicConstruct &x) {
if (clauseList) {
atomicDirectiveDefaultOrderFound_ = true;
switch (*defaultMemOrder) {
- case common::OmpAtomicDefaultMemOrderType::AcqRel:
+ case common::OmpMemoryOrderType::Acq_Rel:
clauseList->emplace_back<parser::OmpMemoryOrderClause>(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::OmpMemoryOrderClause>(
parser::OmpClause{parser::OmpClause::Relaxed{}});
break;
- case common::OmpAtomicDefaultMemOrderType::SeqCst:
+ case common::OmpMemoryOrderType::Seq_Cst:
clauseList->emplace_back<parser::OmpMemoryOrderClause>(
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;
}
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
kparzysz
force-pushed
the
users/kparzysz/spr/h02-memory-order
branch
from
8e62278 to
8df4690
Compare
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…lvm#136312) 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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…lvm#136312) 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.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…lvm#136312) 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.