[flang-commits] [flang] [flang][OpenMP] Handle REQUIRES ADMO in lowering (PR #144362)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Mon Jun 16 07:29:58 PDT 2025
https://github.com/kparzysz created https://github.com/llvm/llvm-project/pull/144362
The previous approach rewrote the atomic constructs in the AST based on the REQUIRES ATOMIC_DEFAULT_MEM_ORDER directives. The new approach checks for incorrect uses of REQUIRED ADMO in the semantic analysis, and applies it in lowering, eliminating the need for a separate tree-rewriting procedure.
>From af0e929d760fb75ef4c9ba5b4cf6e269a9d8e018 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Fri, 13 Jun 2025 13:53:06 -0500
Subject: [PATCH] [flang][OpenMP] Handle REQUIRES ADMO in lowering
The previous approach rewrote the atomic constructs in the AST based
on the REQUIRES ATOMIC_DEFAULT_MEM_ORDER directives.
The new approach checks for incorrect uses of REQUIRED ADMO in the
semantic analysis, and applies it in lowering, eliminating the need
for a separate tree-rewriting procedure.
---
flang/lib/Lower/OpenMP/OpenMP.cpp | 206 +++++++++++++-----
flang/lib/Semantics/CMakeLists.txt | 1 -
flang/lib/Semantics/check-omp-structure.cpp | 19 ++
flang/lib/Semantics/check-omp-structure.h | 1 +
flang/lib/Semantics/rewrite-directives.cpp | 172 ---------------
flang/lib/Semantics/rewrite-directives.h | 24 --
flang/lib/Semantics/rewrite-parse-tree.cpp | 4 +-
.../Lower/OpenMP/requires-admo-acqrel.f90 | 19 ++
.../Lower/OpenMP/requires-admo-invalid1.f90 | 16 ++
.../Lower/OpenMP/requires-admo-invalid2.f90 | 16 ++
.../Semantics/OpenMP/requires-atomic01.f90 | 121 ----------
.../Semantics/OpenMP/requires-atomic02.f90 | 121 ----------
12 files changed, 220 insertions(+), 500 deletions(-)
delete mode 100644 flang/lib/Semantics/rewrite-directives.cpp
delete mode 100644 flang/lib/Semantics/rewrite-directives.h
create mode 100644 flang/test/Lower/OpenMP/requires-admo-acqrel.f90
create mode 100644 flang/test/Lower/OpenMP/requires-admo-invalid1.f90
create mode 100644 flang/test/Lower/OpenMP/requires-admo-invalid2.f90
delete mode 100644 flang/test/Semantics/OpenMP/requires-atomic01.f90
delete mode 100644 flang/test/Semantics/OpenMP/requires-atomic02.f90
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 82673f0948a5b..ed495257bdb73 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2715,58 +2715,129 @@ static mlir::IntegerAttr getAtomicHint(lower::AbstractConverter &converter,
return nullptr;
}
-static mlir::omp::ClauseMemoryOrderKindAttr
-getAtomicMemoryOrder(lower::AbstractConverter &converter,
- semantics::SemanticsContext &semaCtx,
- const List<Clause> &clauses) {
- std::optional<mlir::omp::ClauseMemoryOrderKind> kind;
+static mlir::omp::ClauseMemoryOrderKind
+getMemoryOrderKind(common::OmpMemoryOrderType kind) {
+ switch (kind) {
+ case common::OmpMemoryOrderType::Acq_Rel:
+ return mlir::omp::ClauseMemoryOrderKind::Acq_rel;
+ case common::OmpMemoryOrderType::Acquire:
+ return mlir::omp::ClauseMemoryOrderKind::Acquire;
+ case common::OmpMemoryOrderType::Relaxed:
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ case common::OmpMemoryOrderType::Release:
+ return mlir::omp::ClauseMemoryOrderKind::Release;
+ case common::OmpMemoryOrderType::Seq_Cst:
+ return mlir::omp::ClauseMemoryOrderKind::Seq_cst;
+ }
+ llvm_unreachable("Unexpected kind");
+}
+
+static std::optional<mlir::omp::ClauseMemoryOrderKind>
+getMemoryOrderKind(llvm::omp::Clause clauseId) {
+ switch (clauseId) {
+ case llvm::omp::Clause::OMPC_acq_rel:
+ return mlir::omp::ClauseMemoryOrderKind::Acq_rel;
+ case llvm::omp::Clause::OMPC_acquire:
+ return mlir::omp::ClauseMemoryOrderKind::Acquire;
+ case llvm::omp::Clause::OMPC_relaxed:
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ case llvm::omp::Clause::OMPC_release:
+ return mlir::omp::ClauseMemoryOrderKind::Release;
+ case llvm::omp::Clause::OMPC_seq_cst:
+ return mlir::omp::ClauseMemoryOrderKind::Seq_cst;
+ default:
+ return std::nullopt;
+ }
+}
+
+static std::optional<mlir::omp::ClauseMemoryOrderKind>
+getMemoryOrderFromRequires(const semantics::Scope &scope) {
+ // The REQUIRES construct is only allowed in the main program scope
+ // and module scope, but seems like we also accept it in a subprogram
+ // scope.
+ // For safety, traverse all enclosing scopes and check if their symbol
+ // contains REQUIRES.
+ for (const auto *sc{&scope}; sc->kind() != semantics::Scope::Kind::Global;
+ sc = &sc->parent()) {
+ const semantics::Symbol *sym = sc->symbol();
+ if (!sym)
+ continue;
+
+ const common::OmpMemoryOrderType *admo = common::visit(
+ [](auto &&s) {
+ using WithOmpDeclarative = semantics::WithOmpDeclarative;
+ if constexpr (std::is_convertible_v<decltype(s),
+ const WithOmpDeclarative &>) {
+ return s.ompAtomicDefaultMemOrder();
+ }
+ return static_cast<const common::OmpMemoryOrderType *>(nullptr);
+ },
+ sym->details());
+ if (admo)
+ return getMemoryOrderKind(*admo);
+ }
+
+ return std::nullopt;
+}
+
+static std::optional<mlir::omp::ClauseMemoryOrderKind>
+getDefaultAtomicMemOrder(semantics::SemanticsContext &semaCtx) {
unsigned version = semaCtx.langOptions().OpenMPVersion;
+ if (version > 50)
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ return std::nullopt;
+}
+static std::optional<mlir::omp::ClauseMemoryOrderKind>
+getAtomicMemoryOrder(semantics::SemanticsContext &semaCtx,
+ const List<Clause> &clauses,
+ const semantics::Scope &scope) {
for (const Clause &clause : clauses) {
- switch (clause.id) {
- case llvm::omp::Clause::OMPC_acq_rel:
- kind = mlir::omp::ClauseMemoryOrderKind::Acq_rel;
- break;
- case llvm::omp::Clause::OMPC_acquire:
- kind = mlir::omp::ClauseMemoryOrderKind::Acquire;
- break;
- case llvm::omp::Clause::OMPC_relaxed:
- kind = mlir::omp::ClauseMemoryOrderKind::Relaxed;
- break;
- case llvm::omp::Clause::OMPC_release:
- kind = mlir::omp::ClauseMemoryOrderKind::Release;
- break;
- case llvm::omp::Clause::OMPC_seq_cst:
- kind = mlir::omp::ClauseMemoryOrderKind::Seq_cst;
- break;
- default:
- break;
- }
+ if (auto maybeKind = getMemoryOrderKind(clause.id))
+ return *maybeKind;
}
- // Starting with 5.1, if no memory-order clause is present, the effect
- // is as if "relaxed" was present.
- if (!kind) {
- if (version <= 50)
- return nullptr;
- kind = mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ if (auto maybeKind = getMemoryOrderFromRequires(scope))
+ return *maybeKind;
+
+ return getDefaultAtomicMemOrder(semaCtx);
+}
+
+static mlir::omp::ClauseMemoryOrderKindAttr
+makeMemOrderAttr(lower::AbstractConverter &converter,
+ std::optional<mlir::omp::ClauseMemoryOrderKind> maybeKind) {
+ if (maybeKind) {
+ return mlir::omp::ClauseMemoryOrderKindAttr::get(
+ converter.getFirOpBuilder().getContext(), *maybeKind);
}
- fir::FirOpBuilder &builder = converter.getFirOpBuilder();
- return mlir::omp::ClauseMemoryOrderKindAttr::get(builder.getContext(), *kind);
+ return nullptr;
}
static mlir::Operation * //
-genAtomicRead(lower::AbstractConverter &converter, mlir::Location loc,
+genAtomicRead(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx, mlir::Location loc,
lower::StatementContext &stmtCtx, mlir::Value atomAddr,
const semantics::SomeExpr &atom,
const evaluate::Assignment &assign, mlir::IntegerAttr hint,
- mlir::omp::ClauseMemoryOrderKindAttr memOrder,
+ std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder,
fir::FirOpBuilder::InsertPoint preAt,
fir::FirOpBuilder::InsertPoint atomicAt,
fir::FirOpBuilder::InsertPoint postAt) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
builder.restoreInsertionPoint(preAt);
+ // If the atomic clause is read then the memory-order clause must
+ // not be release.
+ if (memOrder) {
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Release) {
+ // Reset it back to the default.
+ memOrder = getDefaultAtomicMemOrder(semaCtx);
+ } else if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acq_rel) {
+ // The MLIR verifier doesn't like acq_rel either.
+ memOrder = mlir::omp::ClauseMemoryOrderKind::Acquire;
+ }
+ }
+
mlir::Value storeAddr =
fir::getBase(converter.genExprAddr(assign.lhs, stmtCtx, &loc));
mlir::Type atomType = fir::unwrapRefType(atomAddr.getType());
@@ -2780,7 +2851,8 @@ genAtomicRead(lower::AbstractConverter &converter, mlir::Location loc,
builder.restoreInsertionPoint(atomicAt);
mlir::Operation *op = builder.create<mlir::omp::AtomicReadOp>(
- loc, atomAddr, toAddr, mlir::TypeAttr::get(atomType), hint, memOrder);
+ loc, atomAddr, toAddr, mlir::TypeAttr::get(atomType), hint,
+ makeMemOrderAttr(converter, memOrder));
if (atomType != storeType) {
lower::ExprToValueMap overrides;
@@ -2801,17 +2873,30 @@ genAtomicRead(lower::AbstractConverter &converter, mlir::Location loc,
}
static mlir::Operation * //
-genAtomicWrite(lower::AbstractConverter &converter, mlir::Location loc,
+genAtomicWrite(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx, mlir::Location loc,
lower::StatementContext &stmtCtx, mlir::Value atomAddr,
const semantics::SomeExpr &atom,
const evaluate::Assignment &assign, mlir::IntegerAttr hint,
- mlir::omp::ClauseMemoryOrderKindAttr memOrder,
+ std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder,
fir::FirOpBuilder::InsertPoint preAt,
fir::FirOpBuilder::InsertPoint atomicAt,
fir::FirOpBuilder::InsertPoint postAt) {
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
builder.restoreInsertionPoint(preAt);
+ // If the atomic clause is write then the memory-order clause must
+ // not be acquire.
+ if (memOrder) {
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acquire) {
+ // Reset it back to the default.
+ memOrder = getDefaultAtomicMemOrder(semaCtx);
+ } else if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acq_rel) {
+ // The MLIR verifier doesn't like acq_rel either.
+ memOrder = mlir::omp::ClauseMemoryOrderKind::Release;
+ }
+ }
+
mlir::Value value =
fir::getBase(converter.genExprValue(assign.rhs, stmtCtx, &loc));
mlir::Type atomType = fir::unwrapRefType(atomAddr.getType());
@@ -2819,16 +2904,17 @@ genAtomicWrite(lower::AbstractConverter &converter, mlir::Location loc,
builder.restoreInsertionPoint(atomicAt);
mlir::Operation *op = builder.create<mlir::omp::AtomicWriteOp>(
- loc, atomAddr, converted, hint, memOrder);
+ loc, atomAddr, converted, hint, makeMemOrderAttr(converter, memOrder));
return op;
}
static mlir::Operation *
-genAtomicUpdate(lower::AbstractConverter &converter, mlir::Location loc,
+genAtomicUpdate(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx, mlir::Location loc,
lower::StatementContext &stmtCtx, mlir::Value atomAddr,
const semantics::SomeExpr &atom,
const evaluate::Assignment &assign, mlir::IntegerAttr hint,
- mlir::omp::ClauseMemoryOrderKindAttr memOrder,
+ std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder,
fir::FirOpBuilder::InsertPoint preAt,
fir::FirOpBuilder::InsertPoint atomicAt,
fir::FirOpBuilder::InsertPoint postAt) {
@@ -2851,8 +2937,8 @@ genAtomicUpdate(lower::AbstractConverter &converter, mlir::Location loc,
}
builder.restoreInsertionPoint(atomicAt);
- auto updateOp =
- builder.create<mlir::omp::AtomicUpdateOp>(loc, atomAddr, hint, memOrder);
+ auto updateOp = builder.create<mlir::omp::AtomicUpdateOp>(
+ loc, atomAddr, hint, makeMemOrderAttr(converter, memOrder));
mlir::Region ®ion = updateOp->getRegion(0);
mlir::Block *block = builder.createBlock(®ion, {}, {atomType}, {loc});
@@ -2871,11 +2957,12 @@ genAtomicUpdate(lower::AbstractConverter &converter, mlir::Location loc,
}
static mlir::Operation *
-genAtomicOperation(lower::AbstractConverter &converter, mlir::Location loc,
+genAtomicOperation(lower::AbstractConverter &converter,
+ semantics::SemanticsContext &semaCtx, mlir::Location loc,
lower::StatementContext &stmtCtx, int action,
mlir::Value atomAddr, const semantics::SomeExpr &atom,
const evaluate::Assignment &assign, mlir::IntegerAttr hint,
- mlir::omp::ClauseMemoryOrderKindAttr memOrder,
+ std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder,
fir::FirOpBuilder::InsertPoint preAt,
fir::FirOpBuilder::InsertPoint atomicAt,
fir::FirOpBuilder::InsertPoint postAt) {
@@ -2887,14 +2974,14 @@ genAtomicOperation(lower::AbstractConverter &converter, mlir::Location loc,
// builder's insertion point, or set it to anything specific.
switch (action) {
case parser::OpenMPAtomicConstruct::Analysis::Read:
- return genAtomicRead(converter, loc, stmtCtx, atomAddr, atom, assign, hint,
- memOrder, preAt, atomicAt, postAt);
+ return genAtomicRead(converter, semaCtx, loc, stmtCtx, atomAddr, atom,
+ assign, hint, memOrder, preAt, atomicAt, postAt);
case parser::OpenMPAtomicConstruct::Analysis::Write:
- return genAtomicWrite(converter, loc, stmtCtx, atomAddr, atom, assign, hint,
- memOrder, preAt, atomicAt, postAt);
+ return genAtomicWrite(converter, semaCtx, loc, stmtCtx, atomAddr, atom,
+ assign, hint, memOrder, preAt, atomicAt, postAt);
case parser::OpenMPAtomicConstruct::Analysis::Update:
- return genAtomicUpdate(converter, loc, stmtCtx, atomAddr, atom, assign,
- hint, memOrder, preAt, atomicAt, postAt);
+ return genAtomicUpdate(converter, semaCtx, loc, stmtCtx, atomAddr, atom,
+ assign, hint, memOrder, preAt, atomicAt, postAt);
default:
return nullptr;
}
@@ -3894,8 +3981,9 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
mlir::Value atomAddr =
fir::getBase(converter.genExprAddr(atom, stmtCtx, &loc));
mlir::IntegerAttr hint = getAtomicHint(converter, clauses);
- mlir::omp::ClauseMemoryOrderKindAttr memOrder =
- getAtomicMemoryOrder(converter, semaCtx, clauses);
+ std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder =
+ getAtomicMemoryOrder(semaCtx, clauses,
+ semaCtx.FindScope(construct.source));
if (auto *cond = get(analysis.cond)) {
(void)cond;
@@ -3913,8 +4001,8 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
"Expexcing two actions");
(void)action0;
(void)action1;
- captureOp =
- builder.create<mlir::omp::AtomicCaptureOp>(loc, hint, memOrder);
+ captureOp = builder.create<mlir::omp::AtomicCaptureOp>(
+ loc, hint, makeMemOrderAttr(converter, memOrder));
// Set the non-atomic insertion point to before the atomic.capture.
preAt = getInsertionPointBefore(captureOp);
@@ -3926,7 +4014,7 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
atomicAt = getInsertionPointBefore(term);
postAt = getInsertionPointAfter(captureOp);
hint = nullptr;
- memOrder = nullptr;
+ memOrder = std::nullopt;
} else {
// Non-capturing operation.
assert(action0 != analysis.None && action1 == analysis.None &&
@@ -3938,16 +4026,16 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
// The builder's insertion point needs to be specifically set before
// each call to `genAtomicOperation`.
mlir::Operation *firstOp = genAtomicOperation(
- converter, loc, stmtCtx, analysis.op0.what, atomAddr, atom,
+ converter, semaCtx, loc, stmtCtx, analysis.op0.what, atomAddr, atom,
*get(analysis.op0.assign), hint, memOrder, preAt, atomicAt, postAt);
assert(firstOp && "Should have created an atomic operation");
atomicAt = getInsertionPointAfter(firstOp);
mlir::Operation *secondOp = nullptr;
if (analysis.op1.what != analysis.None) {
- secondOp = genAtomicOperation(converter, loc, stmtCtx, analysis.op1.what,
- atomAddr, atom, *get(analysis.op1.assign),
- hint, memOrder, preAt, atomicAt, postAt);
+ secondOp = genAtomicOperation(
+ converter, semaCtx, loc, stmtCtx, analysis.op1.what, atomAddr, atom,
+ *get(analysis.op1.assign), hint, memOrder, preAt, atomicAt, postAt);
}
if (construct.IsCapture()) {
diff --git a/flang/lib/Semantics/CMakeLists.txt b/flang/lib/Semantics/CMakeLists.txt
index 18c89587843a9..c0fda3631c01f 100644
--- a/flang/lib/Semantics/CMakeLists.txt
+++ b/flang/lib/Semantics/CMakeLists.txt
@@ -40,7 +40,6 @@ add_flang_library(FortranSemantics
resolve-directives.cpp
resolve-names-utils.cpp
resolve-names.cpp
- rewrite-directives.cpp
rewrite-parse-tree.cpp
runtime-type-info.cpp
scope.cpp
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 58d28dce7094a..472ede617ab21 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1719,6 +1719,22 @@ void OmpStructureChecker::Leave(const parser::OpenMPDepobjConstruct &x) {
void OmpStructureChecker::Enter(const parser::OpenMPRequiresConstruct &x) {
const auto &dir{std::get<parser::Verbatim>(x.t)};
PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_requires);
+
+ if (visitedAtomicSource_.empty()) {
+ return;
+ }
+ const auto &clauseList{std::get<parser::OmpClauseList>(x.t)};
+ for (const parser::OmpClause &clause : clauseList.v) {
+ llvm::omp::Clause id{clause.Id()};
+ if (id == llvm::omp::Clause::OMPC_atomic_default_mem_order) {
+ parser::MessageFormattedText txt(
+ "REQUIRES directive with '%s' clause found lexically after atomic operation without a memory order clause"_err_en_US,
+ parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(id)));
+ parser::Message message(clause.source, txt);
+ message.Attach(visitedAtomicSource_, "Previous atomic construct"_en_US);
+ context_.Say(std::move(message));
+ }
+ }
}
void OmpStructureChecker::Leave(const parser::OpenMPRequiresConstruct &) {
@@ -4028,6 +4044,9 @@ void OmpStructureChecker::CheckAtomicUpdate(
}
void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
+ if (visitedAtomicSource_.empty())
+ visitedAtomicSource_ = x.source;
+
// All of the following groups have the "exclusive" property, i.e. at
// most one clause from each group is allowed.
// The exclusivity-checking code should eventually be unified for all
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 2074ec611dc2a..beb6e0528e814 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -360,6 +360,7 @@ class OmpStructureChecker
};
int directiveNest_[LastType + 1] = {0};
+ parser::CharBlock visitedAtomicSource_;
SymbolSourceMap deferredNonVariables_;
using LoopConstruct = std::variant<const parser::DoConstruct *,
diff --git a/flang/lib/Semantics/rewrite-directives.cpp b/flang/lib/Semantics/rewrite-directives.cpp
deleted file mode 100644
index 91b60ea151dee..0000000000000
--- a/flang/lib/Semantics/rewrite-directives.cpp
+++ /dev/null
@@ -1,172 +0,0 @@
-//===-- lib/Semantics/rewrite-directives.cpp ------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "rewrite-directives.h"
-#include "flang/Parser/parse-tree-visitor.h"
-#include "flang/Parser/parse-tree.h"
-#include "flang/Semantics/semantics.h"
-#include "flang/Semantics/symbol.h"
-#include "llvm/Frontend/OpenMP/OMP.h"
-#include <list>
-
-namespace Fortran::semantics {
-
-using namespace parser::literals;
-
-class DirectiveRewriteMutator {
-public:
- explicit DirectiveRewriteMutator(SemanticsContext &context)
- : context_{context} {}
-
- // Default action for a parse tree node is to visit children.
- template <typename T> bool Pre(T &) { return true; }
- template <typename T> void Post(T &) {}
-
-protected:
- SemanticsContext &context_;
-};
-
-// 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.
-class OmpRewriteMutator : public DirectiveRewriteMutator {
-public:
- explicit OmpRewriteMutator(SemanticsContext &context)
- : DirectiveRewriteMutator(context) {}
-
- template <typename T> bool Pre(T &) { return true; }
- template <typename T> void Post(T &) {}
-
- bool Pre(parser::OpenMPAtomicConstruct &);
- bool Pre(parser::OpenMPRequiresConstruct &);
-
-private:
- bool atomicDirectiveDefaultOrderFound_{false};
-};
-
-bool OmpRewriteMutator::Pre(parser::OpenMPAtomicConstruct &x) {
- // Find top-level parent of the operation.
- Symbol *topLevelParent{[&]() {
- Symbol *symbol{nullptr};
- Scope *scope{&context_.FindScope(
- std::get<parser::OmpDirectiveSpecification>(x.t).source)};
- do {
- if (Symbol * parent{scope->symbol()}) {
- symbol = parent;
- }
- scope = &scope->parent();
- } while (!scope->IsGlobal());
-
- assert(symbol &&
- "Atomic construct must be within a scope associated with a symbol");
- return symbol;
- }()};
-
- // Get the `atomic_default_mem_order` clause from the top-level parent.
- std::optional<common::OmpMemoryOrderType> defaultMemOrder;
- common::visit(
- [&](auto &details) {
- if constexpr (std::is_convertible_v<decltype(&details),
- WithOmpDeclarative *>) {
- if (details.has_ompAtomicDefaultMemOrder()) {
- defaultMemOrder = *details.ompAtomicDefaultMemOrder();
- }
- }
- },
- topLevelParent->details());
-
- if (!defaultMemOrder) {
- return false;
- }
-
- auto findMemOrderClause{[](const parser::OmpClauseList &clauses) {
- return llvm::any_of(
- clauses.v, [](auto &clause) -> const parser::OmpClause * {
- switch (clause.Id()) {
- case llvm::omp::Clause::OMPC_acq_rel:
- case llvm::omp::Clause::OMPC_acquire:
- case llvm::omp::Clause::OMPC_relaxed:
- case llvm::omp::Clause::OMPC_release:
- case llvm::omp::Clause::OMPC_seq_cst:
- return &clause;
- default:
- return nullptr;
- }
- });
- }};
-
- auto &dirSpec{std::get<parser::OmpDirectiveSpecification>(x.t)};
- auto &clauseList{std::get<std::optional<parser::OmpClauseList>>(dirSpec.t)};
- if (clauseList) {
- if (findMemOrderClause(*clauseList)) {
- return false;
- }
- } else {
- clauseList = parser::OmpClauseList(decltype(parser::OmpClauseList::v){});
- }
-
- // Add a memory order clause to the atomic directive.
- atomicDirectiveDefaultOrderFound_ = true;
- llvm::omp::Clause kind{x.GetKind()};
- switch (*defaultMemOrder) {
- case common::OmpMemoryOrderType::Acq_Rel:
- // FIXME: Implement 5.0 rules, pending clarification on later spec
- // versions.
- // [5.0:62:22-26]
- if (kind == llvm::omp::Clause::OMPC_read) {
- clauseList->v.emplace_back(
- parser::OmpClause{parser::OmpClause::Acquire{}});
- } else if (kind == llvm::omp::Clause::OMPC_update && x.IsCapture()) {
- clauseList->v.emplace_back(
- parser::OmpClause{parser::OmpClause::AcqRel{}});
- } else {
- clauseList->v.emplace_back(
- parser::OmpClause{parser::OmpClause::Release{}});
- }
- break;
- case common::OmpMemoryOrderType::Relaxed:
- clauseList->v.emplace_back(parser::OmpClause{parser::OmpClause::Relaxed{}});
- break;
- case common::OmpMemoryOrderType::Seq_Cst:
- clauseList->v.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;
- }
-
- return false;
-}
-
-bool OmpRewriteMutator::Pre(parser::OpenMPRequiresConstruct &x) {
- for (parser::OmpClause &clause : std::get<parser::OmpClauseList>(x.t).v) {
- if (std::holds_alternative<parser::OmpClause::AtomicDefaultMemOrder>(
- clause.u) &&
- atomicDirectiveDefaultOrderFound_) {
- context_.Say(clause.source,
- "REQUIRES directive with '%s' clause found lexically after atomic "
- "operation without a memory order clause"_err_en_US,
- parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
- llvm::omp::OMPC_atomic_default_mem_order)
- .str()));
- }
- }
- return false;
-}
-
-bool RewriteOmpParts(SemanticsContext &context, parser::Program &program) {
- if (!context.IsEnabled(common::LanguageFeature::OpenMP)) {
- return true;
- }
- OmpRewriteMutator ompMutator{context};
- parser::Walk(program, ompMutator);
- return !context.AnyFatalError();
-}
-
-} // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/rewrite-directives.h b/flang/lib/Semantics/rewrite-directives.h
deleted file mode 100644
index 6759621928427..0000000000000
--- a/flang/lib/Semantics/rewrite-directives.h
+++ /dev/null
@@ -1,24 +0,0 @@
-//===-- lib/Semantics/rewrite-directives.h ----------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef FORTRAN_SEMANTICS_REWRITE_DIRECTIVES_H_
-#define FORTRAN_SEMANTICS_REWRITE_DIRECTIVES_H_
-
-namespace Fortran::parser {
-struct Program;
-} // namespace Fortran::parser
-
-namespace Fortran::semantics {
-class SemanticsContext;
-} // namespace Fortran::semantics
-
-namespace Fortran::semantics {
-bool RewriteOmpParts(SemanticsContext &, parser::Program &);
-} // namespace Fortran::semantics
-
-#endif // FORTRAN_SEMANTICS_REWRITE_DIRECTIVES_H_
diff --git a/flang/lib/Semantics/rewrite-parse-tree.cpp b/flang/lib/Semantics/rewrite-parse-tree.cpp
index 577558e7e33b2..4eeb1b9ed3c1e 100644
--- a/flang/lib/Semantics/rewrite-parse-tree.cpp
+++ b/flang/lib/Semantics/rewrite-parse-tree.cpp
@@ -7,7 +7,7 @@
//===----------------------------------------------------------------------===//
#include "rewrite-parse-tree.h"
-#include "rewrite-directives.h"
+
#include "flang/Common/indirection.h"
#include "flang/Parser/parse-tree-visitor.h"
#include "flang/Parser/parse-tree.h"
@@ -229,7 +229,7 @@ void RewriteMutator::Post(parser::WriteStmt &x) {
bool RewriteParseTree(SemanticsContext &context, parser::Program &program) {
RewriteMutator mutator{context};
parser::Walk(program, mutator);
- return !context.AnyFatalError() && RewriteOmpParts(context, program);
+ return !context.AnyFatalError();
}
} // namespace Fortran::semantics
diff --git a/flang/test/Lower/OpenMP/requires-admo-acqrel.f90 b/flang/test/Lower/OpenMP/requires-admo-acqrel.f90
new file mode 100644
index 0000000000000..525a846f410d8
--- /dev/null
+++ b/flang/test/Lower/OpenMP/requires-admo-acqrel.f90
@@ -0,0 +1,19 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s
+
+module m
+!$omp requires atomic_default_mem_order(acq_rel)
+
+contains
+
+subroutine f00(x, v)
+ integer :: x, v
+!CHECK: omp.atomic.read %{{[ %#=0-9]+}} memory_order(acquire)
+ !$omp atomic read
+ v = x
+
+!CHECK: omp.atomic.write %{{[ %#=0-9]+}} memory_order(release)
+ !$omp atomic write
+ x = v
+end
+
+end module
diff --git a/flang/test/Lower/OpenMP/requires-admo-invalid1.f90 b/flang/test/Lower/OpenMP/requires-admo-invalid1.f90
new file mode 100644
index 0000000000000..b21d3bbbc7866
--- /dev/null
+++ b/flang/test/Lower/OpenMP/requires-admo-invalid1.f90
@@ -0,0 +1,16 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s
+
+module m
+!$omp requires atomic_default_mem_order(acquire)
+
+contains
+
+subroutine f00(x, v)
+ integer :: x, v
+!CHECK: omp.atomic.write %{{[ %#=0-9]+}} memory_order(relaxed)
+ !$omp atomic write
+ x = v
+end
+
+end module
+
diff --git a/flang/test/Lower/OpenMP/requires-admo-invalid2.f90 b/flang/test/Lower/OpenMP/requires-admo-invalid2.f90
new file mode 100644
index 0000000000000..33caa25dcc646
--- /dev/null
+++ b/flang/test/Lower/OpenMP/requires-admo-invalid2.f90
@@ -0,0 +1,16 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s
+
+module m
+!$omp requires atomic_default_mem_order(release)
+
+contains
+
+subroutine f00(x, v)
+ integer :: x, v
+!CHECK: omp.atomic.read {{[ %#=0-9]+}} memory_order(relaxed)
+ !$omp atomic read
+ v = x
+end
+
+end module
+
diff --git a/flang/test/Semantics/OpenMP/requires-atomic01.f90 b/flang/test/Semantics/OpenMP/requires-atomic01.f90
deleted file mode 100644
index e8817c3f5ef61..0000000000000
--- a/flang/test/Semantics/OpenMP/requires-atomic01.f90
+++ /dev/null
@@ -1,121 +0,0 @@
-! RUN: %flang_fc1 -fopenmp -fopenmp-version=50 -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
- implicit none
- !$omp requires atomic_default_mem_order(seq_cst)
- integer :: i, j
-
- ! ----------------------------------------------------------------------------
- ! READ
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Read
- ! CHECK: OmpClause -> SeqCst
- !$omp atomic read
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Read
- !$omp atomic relaxed read
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Read
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic read relaxed
- i = j
-
- ! ----------------------------------------------------------------------------
- ! WRITE
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Write
- ! CHECK: OmpClause -> SeqCst
- !$omp atomic write
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Write
- !$omp atomic relaxed write
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Write
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic write relaxed
- i = j
-
- ! ----------------------------------------------------------------------------
- ! UPDATE
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Update
- ! CHECK: OmpClause -> SeqCst
- !$omp atomic update
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Update
- !$omp atomic relaxed update
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Update
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic update relaxed
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> SeqCst
- !$omp atomic
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic relaxed
- i = i + j
-
- ! ----------------------------------------------------------------------------
- ! CAPTURE
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Capture
- ! CHECK: OmpClause -> SeqCst
- !$omp atomic capture
- i = j
- j = j + 1
- !$omp end atomic
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Capture
- !$omp atomic relaxed capture
- i = j
- j = j + 1
- !$omp end atomic
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Capture
- ! CHECK-NOT: OmpClause -> SeqCst
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic capture relaxed
- i = j
- j = j + 1
- !$omp end atomic
-end program requires
diff --git a/flang/test/Semantics/OpenMP/requires-atomic02.f90 b/flang/test/Semantics/OpenMP/requires-atomic02.f90
deleted file mode 100644
index 04a9b7a09aa98..0000000000000
--- a/flang/test/Semantics/OpenMP/requires-atomic02.f90
+++ /dev/null
@@ -1,121 +0,0 @@
-! RUN: %flang_fc1 -fopenmp -fopenmp-version=50 -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. ACQ_REL clause tested here.
-program requires
- implicit none
- !$omp requires atomic_default_mem_order(acq_rel)
- integer :: i, j
-
- ! ----------------------------------------------------------------------------
- ! READ
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Read
- ! CHECK: OmpClause -> Acquire
- !$omp atomic read
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Read
- !$omp atomic relaxed read
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Read
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic read relaxed
- i = j
-
- ! ----------------------------------------------------------------------------
- ! WRITE
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Write
- ! CHECK: OmpClause -> Release
- !$omp atomic write
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Write
- !$omp atomic relaxed write
- i = j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Write
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic write relaxed
- i = j
-
- ! ----------------------------------------------------------------------------
- ! UPDATE
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Update
- ! CHECK: OmpClause -> Release
- !$omp atomic update
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Update
- !$omp atomic relaxed update
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Update
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic update relaxed
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Release
- !$omp atomic
- i = i + j
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic relaxed
- i = i + j
-
- ! ----------------------------------------------------------------------------
- ! CAPTURE
- ! ----------------------------------------------------------------------------
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK: OmpClause -> Capture
- ! CHECK: OmpClause -> AcqRel
- !$omp atomic capture
- i = j
- j = j + 1
- !$omp end atomic
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Relaxed
- ! CHECK: OmpClause -> Capture
- !$omp atomic relaxed capture
- i = j
- j = j + 1
- !$omp end atomic
-
- ! CHECK-LABEL: OpenMPAtomicConstruct
- ! CHECK-NOT: OmpClause -> AcqRel
- ! CHECK: OmpClause -> Capture
- ! CHECK: OmpClause -> Relaxed
- !$omp atomic capture relaxed
- i = j
- j = j + 1
- !$omp end atomic
-end program requires
More information about the flang-commits
mailing list