[flang-commits] [flang] [flang][OpenMP] Handle conflicts between REQUIRES and ATOMIC restrict… (PR #163805)
via flang-commits
flang-commits at lists.llvm.org
Thu Oct 16 08:30:08 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Krzysztof Parzyszek (kparzysz)
<details>
<summary>Changes</summary>
…ions
When the atomic default memory order specified on a REQUIRES directive is disallowed on a given ATOMIC operation, and it's not ACQ_REL, the order reverts to RELAXED. ACQ_REL decays to either ACQUIRE or RELEASE, depending on the operation.
This fixes MLIR verification failure in
Fortran/gfortran/regression/gomp/requires-9.f90
---
Full diff: https://github.com/llvm/llvm-project/pull/163805.diff
7 Files Affected:
- (modified) flang/lib/Lower/OpenMP/Atomic.cpp (+81-16)
- (added) flang/test/Lower/OpenMP/atomic-requires-conflict-v50-1.f90 (+11)
- (added) flang/test/Lower/OpenMP/atomic-requires-conflict-v50-2.f90 (+11)
- (added) flang/test/Lower/OpenMP/atomic-requires-conflict-v50-3.f90 (+11)
- (added) flang/test/Lower/OpenMP/atomic-requires-conflict-v50-4.f90 (+13)
- (added) flang/test/Lower/OpenMP/atomic-requires-conflict-v60-1.f90 (+11)
- (added) flang/test/Lower/OpenMP/atomic-requires-conflict-v60-2.f90 (+11)
``````````diff
diff --git a/flang/lib/Lower/OpenMP/Atomic.cpp b/flang/lib/Lower/OpenMP/Atomic.cpp
index ff82a36951bfa..58df5fdb32aac 100644
--- a/flang/lib/Lower/OpenMP/Atomic.cpp
+++ b/flang/lib/Lower/OpenMP/Atomic.cpp
@@ -20,6 +20,7 @@
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
#include "flang/Parser/parse-tree.h"
+#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/semantics.h"
#include "flang/Semantics/type.h"
#include "flang/Support/Fortran.h"
@@ -183,12 +184,8 @@ getMemoryOrderFromRequires(const semantics::Scope &scope) {
// 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 semantics::Scope &unitScope = semantics::omp::GetProgramUnit(scope);
+ if (auto *symbol = unitScope.symbol()) {
const common::OmpMemoryOrderType *admo = common::visit(
[](auto &&s) {
using WithOmpDeclarative = semantics::WithOmpDeclarative;
@@ -198,7 +195,8 @@ getMemoryOrderFromRequires(const semantics::Scope &scope) {
}
return static_cast<const common::OmpMemoryOrderType *>(nullptr);
},
- sym->details());
+ symbol->details());
+
if (admo)
return getMemoryOrderKind(*admo);
}
@@ -214,19 +212,83 @@ getDefaultAtomicMemOrder(semantics::SemanticsContext &semaCtx) {
return std::nullopt;
}
-static std::optional<mlir::omp::ClauseMemoryOrderKind>
+static std::pair<std::optional<mlir::omp::ClauseMemoryOrderKind>, bool>
getAtomicMemoryOrder(semantics::SemanticsContext &semaCtx,
const omp::List<omp::Clause> &clauses,
const semantics::Scope &scope) {
for (const omp::Clause &clause : clauses) {
if (auto maybeKind = getMemoryOrderKind(clause.id))
- return *maybeKind;
+ return std::make_pair(*maybeKind, /*canOverride=*/false);
}
if (auto maybeKind = getMemoryOrderFromRequires(scope))
- return *maybeKind;
+ return std::make_pair(*maybeKind, /*canOverride=*/true);
- return getDefaultAtomicMemOrder(semaCtx);
+ return std::make_pair(getDefaultAtomicMemOrder(semaCtx),
+ /*canOverride=*/false);
+}
+
+static std::optional<mlir::omp::ClauseMemoryOrderKind>
+makeValidForAction(std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder,
+ int action0, int action1, unsigned version) {
+// When the atomic default memory order specified on a REQUIRES directive is
+// disallowed on a given ATOMIC operation, and it's not ACQ_REL, the order
+// reverts to RELAXED. ACQ_REL decays to either ACQUIRE or RELEASE, depending
+// on the operation.
+
+ if (!memOrder) {
+ return memOrder;
+ }
+
+ using Analysis = parser::OpenMPAtomicConstruct::Analysis;
+ // Figure out the main action (i.e. disregard a potential capture operation)
+ int action = action0;
+ if (action1 != Analysis::None)
+ action = action0 == Analysis::Read ? action1 : action0;
+
+ // Avaliable orderings: acquire, acq_rel, relaxed, release, seq_cst
+
+ if (action == Analysis::Read) {
+ // "acq_rel" decays to "acquire"
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acq_rel)
+ return mlir::omp::ClauseMemoryOrderKind::Acquire;;
+ } else if (action == Analysis::Write) {
+ // "acq_rel" decays to "release"
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acq_rel)
+ return mlir::omp::ClauseMemoryOrderKind::Release;
+ }
+
+ if (version > 50) {
+ if (action == Analysis::Read) {
+ // "release" prohibited
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Release)
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ }
+ if (action == Analysis::Write) {
+ // "acquire" prohibited
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acquire)
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ }
+ } else {
+ if (action == Analysis::Read) {
+ // "release" prohibited
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Release)
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ } else {
+ if (action & Analysis::Write) { // include "update"
+ // "acquire" prohibited
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acquire)
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ if (action == Analysis::Update) {
+ // "acq_rel" prohibited
+ if (*memOrder == mlir::omp::ClauseMemoryOrderKind::Acq_rel)
+ return mlir::omp::ClauseMemoryOrderKind::Relaxed;
+ }
+ }
+ }
+ }
+
+ return memOrder;
}
static mlir::omp::ClauseMemoryOrderKindAttr
@@ -449,16 +511,19 @@ void Fortran::lower::omp::lowerAtomic(
mlir::Value atomAddr =
fir::getBase(converter.genExprAddr(atom, stmtCtx, &loc));
mlir::IntegerAttr hint = getAtomicHint(converter, clauses);
- std::optional<mlir::omp::ClauseMemoryOrderKind> memOrder =
- getAtomicMemoryOrder(semaCtx, clauses,
- semaCtx.FindScope(construct.source));
+ auto [memOrder, canOverride] = getAtomicMemoryOrder(
+ semaCtx, clauses, semaCtx.FindScope(construct.source));
+
+ unsigned version = semaCtx.langOptions().OpenMPVersion;
+ int action0 = analysis.op0.what & analysis.Action;
+ int action1 = analysis.op1.what & analysis.Action;
+ if (canOverride)
+ memOrder = makeValidForAction(memOrder, action0, action1, version);
if (auto *cond = get(analysis.cond)) {
(void)cond;
TODO(loc, "OpenMP ATOMIC COMPARE");
} else {
- int action0 = analysis.op0.what & analysis.Action;
- int action1 = analysis.op1.what & analysis.Action;
mlir::Operation *captureOp = nullptr;
fir::FirOpBuilder::InsertPoint preAt = builder.saveInsertionPoint();
fir::FirOpBuilder::InsertPoint atomicAt, postAt;
diff --git a/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-1.f90 b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-1.f90
new file mode 100644
index 0000000000000..eb8c4b4a343d0
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-1.f90
@@ -0,0 +1,11 @@
+!RUN: bbc %openmp_flags -fopenmp-version=50 -emit-hlfir %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=50 %s -o - | FileCheck %s
+
+!CHECK: omp.atomic.read %{{[0-9]+}}#0 = %{{[0-9]+}}#0 memory_order(acquire)
+
+subroutine f00(x, v)
+ integer :: x, v
+ !$omp requires atomic_default_mem_order(acq_rel)
+ !$omp atomic read
+ v = x
+end
diff --git a/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-2.f90 b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-2.f90
new file mode 100644
index 0000000000000..d309a21a2b938
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-2.f90
@@ -0,0 +1,11 @@
+!RUN: bbc %openmp_flags -fopenmp-version=50 -emit-hlfir %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=50 %s -o - | FileCheck %s
+
+!CHECK: omp.atomic.write %{{[0-9]+}}#0 = %{{[0-9]+}} memory_order(relaxed)
+
+subroutine f02(x, v)
+ integer :: x, v
+ !$omp requires atomic_default_mem_order(acquire)
+ !$omp atomic write
+ x = v
+end
diff --git a/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-3.f90 b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-3.f90
new file mode 100644
index 0000000000000..bc7529c69340a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-3.f90
@@ -0,0 +1,11 @@
+!RUN: bbc %openmp_flags -fopenmp-version=50 -emit-hlfir %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=50 %s -o - | FileCheck %s
+
+!CHECK: omp.atomic.update memory_order(relaxed)
+
+subroutine f05(x, v)
+ integer :: x, v
+ !$omp requires atomic_default_mem_order(acq_rel)
+ !$omp atomic update
+ x = x + 1
+end
diff --git a/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-4.f90 b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-4.f90
new file mode 100644
index 0000000000000..5cffb1ac9d6c4
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-requires-conflict-v50-4.f90
@@ -0,0 +1,13 @@
+!RUN: bbc %openmp_flags -fopenmp-version=50 -emit-hlfir %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=50 %s -o - | FileCheck %s
+
+!CHECK: omp.atomic.capture memory_order(relaxed)
+
+subroutine f06(x, v)
+ integer :: x, v
+ !$omp requires atomic_default_mem_order(acquire)
+ !$omp atomic update capture
+ v = x
+ x = x + 1
+ !$omp end atomic
+end
diff --git a/flang/test/Lower/OpenMP/atomic-requires-conflict-v60-1.f90 b/flang/test/Lower/OpenMP/atomic-requires-conflict-v60-1.f90
new file mode 100644
index 0000000000000..55f21978b715e
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-requires-conflict-v60-1.f90
@@ -0,0 +1,11 @@
+!RUN: bbc %openmp_flags -fopenmp-version=60 -emit-hlfir %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=60 %s -o - | FileCheck %s
+
+!CHECK: omp.atomic.read %{{[0-9]+}}#0 = %{{[0-9]+}}#0 memory_order(relaxed)
+
+subroutine f01(x, v)
+ integer :: x, v
+ !$omp requires atomic_default_mem_order(release)
+ !$omp atomic read
+ v = x
+end
diff --git a/flang/test/Lower/OpenMP/atomic-requires-conflict-v60-2.f90 b/flang/test/Lower/OpenMP/atomic-requires-conflict-v60-2.f90
new file mode 100644
index 0000000000000..ca048798d8d18
--- /dev/null
+++ b/flang/test/Lower/OpenMP/atomic-requires-conflict-v60-2.f90
@@ -0,0 +1,11 @@
+!RUN: bbc %openmp_flags -fopenmp-version=60 -emit-hlfir %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=60 %s -o - | FileCheck %s
+
+!CHECK: omp.atomic.write %{{[0-9]+}}#0 = %{{[0-9]+}} memory_order(relaxed)
+
+subroutine f02(x, v)
+ integer :: x, v
+ !$omp requires atomic_default_mem_order(acquire)
+ !$omp atomic write
+ x = v
+end
``````````
</details>
https://github.com/llvm/llvm-project/pull/163805
More information about the flang-commits
mailing list