[flang-commits] [flang] [flang][OpenMP] Allow flush of common block (PR #139528)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Mon May 12 03:18:39 PDT 2025
https://github.com/tblah created https://github.com/llvm/llvm-project/pull/139528
I think this was denied by accident in https://github.com/llvm/llvm-project/commit/68180d8d16f07db8200dfce7bae26a80c43ebc5e.
Flush of a common block is allowed by the standard on my reading. It is
not allowed by classic-flang but is supported by gfortran and ifx.
This doesn't need any lowering changes. The LLVM translation ignores the
flush argument list because the openmp runtime library doesn't support
flushing specific data.
Depends upon https://github.com/llvm/llvm-project/pull/139522. Ignore the first commit in this PR.
>From a64751019f3eab8a7d6d678ad76fdb8c31b86e54 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Mon, 12 May 2025 09:38:01 +0000
Subject: [PATCH 1/2] [flang][OpenMP][Semantics] resolve objects in the flush
arg list
Fixes #136583
Normally the flush argument list would contain a DataRef to some
variable. All DataRefs are handled generically in resolve-names and so
the problem wasn't observed. But when a common block name is specified,
this is not parsed as a DataRef. There was already handling in
resolve-directives for OmpObjectList but not for argument lists. I've
added a visitor for FLUSH which ensures all of the arguments have been
resolved.
The test is there to make sure the compiler doesn't crashed encountering
the unresolved symbol. It shows that we currently deny flushing a common
block. I'm not sure that it is right to restrict common blocks from flush
argument lists, but fixing that can come in a different patch. This one
is fixing an ICE.
---
flang/lib/Semantics/resolve-directives.cpp | 13 +++++++++++++
flang/test/Semantics/OpenMP/flush04.f90 | 11 +++++++++++
2 files changed, 24 insertions(+)
create mode 100644 flang/test/Semantics/OpenMP/flush04.f90
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 8b1caca34a6a7..68f8cf9f17620 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -409,6 +409,19 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
}
void Post(const parser::OpenMPDepobjConstruct &) { PopContext(); }
+ bool Pre(const parser::OpenMPFlushConstruct &x) {
+ PushContext(x.source, llvm::omp::Directive::OMPD_flush);
+ for (auto &arg : x.v.Arguments().v) {
+ if (auto *locator{std::get_if<parser::OmpLocator>(&arg.u)}) {
+ if (auto *object{std::get_if<parser::OmpObject>(&locator->u)}) {
+ ResolveOmpObject(*object, Symbol::Flag::OmpDependObject);
+ }
+ }
+ }
+ return true;
+ }
+ void Post(const parser::OpenMPFlushConstruct &) { PopContext(); }
+
bool Pre(const parser::OpenMPRequiresConstruct &x) {
using Flags = WithOmpDeclarative::RequiresFlags;
using Requires = WithOmpDeclarative::RequiresFlag;
diff --git a/flang/test/Semantics/OpenMP/flush04.f90 b/flang/test/Semantics/OpenMP/flush04.f90
new file mode 100644
index 0000000000000..ffc2273b692dc
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/flush04.f90
@@ -0,0 +1,11 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+
+! Regression test to ensure that the name /c/ in the flush argument list is
+! resolved to the common block symbol.
+
+ common /c/ x
+ real :: x
+!ERROR: FLUSH argument must be a variable list item
+ !$omp flush(/c/)
+end
+
>From 890a8a05addef06de0485f2d74f755f3c53098c1 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Mon, 12 May 2025 10:13:08 +0000
Subject: [PATCH 2/2] [flang][OpenMP] Allow flush of common block
I think this was denied by accident in 68180d8.
Flush of a common block is allowed by the standard on my reading. It is
not allowed by classic-flang but is supported by gfortran and ifx.
This doesn't need any lowering changes. The LLVM translation ignores the
flush argument list because the openmp runtime library doesn't support
flushing specific data.
Depends upon #139522. Ignore the first commit in this PR.
---
flang/lib/Semantics/check-omp-structure.cpp | 7 -------
flang/test/Lower/OpenMP/flush-common.f90 | 13 +++++++++++++
flang/test/Semantics/OpenMP/flush04.f90 | 11 -----------
3 files changed, 13 insertions(+), 18 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/flush-common.f90
delete mode 100644 flang/test/Semantics/OpenMP/flush04.f90
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index f17de42ca2466..591f30db36baa 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2304,13 +2304,6 @@ void OmpStructureChecker::Leave(const parser::OpenMPFlushConstruct &x) {
auto &flushList{std::get<std::optional<parser::OmpArgumentList>>(x.v.t)};
if (flushList) {
- for (const parser::OmpArgument &arg : flushList->v) {
- if (auto *sym{GetArgumentSymbol(arg)}; sym && !IsVariableListItem(*sym)) {
- context_.Say(arg.source,
- "FLUSH argument must be a variable list item"_err_en_US);
- }
- }
-
if (FindClause(llvm::omp::Clause::OMPC_acquire) ||
FindClause(llvm::omp::Clause::OMPC_release) ||
FindClause(llvm::omp::Clause::OMPC_acq_rel)) {
diff --git a/flang/test/Lower/OpenMP/flush-common.f90 b/flang/test/Lower/OpenMP/flush-common.f90
new file mode 100644
index 0000000000000..7656141dcb295
--- /dev/null
+++ b/flang/test/Lower/OpenMP/flush-common.f90
@@ -0,0 +1,13 @@
+! RUN: %flang_fc1 -fopenmp -emit-hlfir -o - %s | FileCheck %s
+
+! Regression test to ensure that the name /c/ in the flush argument list is
+! resolved to the common block symbol and common blocks are allowed in the
+! flush argument list.
+
+! CHECK: %[[GLBL:.*]] = fir.address_of({{.*}}) : !fir.ref<!fir.array<4xi8>>
+ common /c/ x
+ real :: x
+! CHECK: omp.flush(%[[GLBL]] : !fir.ref<!fir.array<4xi8>>)
+ !$omp flush(/c/)
+end
+
diff --git a/flang/test/Semantics/OpenMP/flush04.f90 b/flang/test/Semantics/OpenMP/flush04.f90
deleted file mode 100644
index ffc2273b692dc..0000000000000
--- a/flang/test/Semantics/OpenMP/flush04.f90
+++ /dev/null
@@ -1,11 +0,0 @@
-! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
-
-! Regression test to ensure that the name /c/ in the flush argument list is
-! resolved to the common block symbol.
-
- common /c/ x
- real :: x
-!ERROR: FLUSH argument must be a variable list item
- !$omp flush(/c/)
-end
-
More information about the flang-commits
mailing list