Skip to content

Commit b49f846

Browse files
committed
[Flang][OpenMP][Sema] Add directive rewrite pass to support atomic_default_mem_order REQUIRES clause
This patch creates the `OmpRewriteMutator` pass that runs at the end of `RewriteParseTree()`. This pass is intended to make OpenMP-specific mutations to the PFT after name resolution. In the case of the `atomic_default_mem_order` clause of the REQUIRES directive, name resolution results in populating global symbols with information about the REQUIRES clauses that apply to that scope. The new rewrite pass is then able to use this information in order to explicitly set the memory order of ATOMIC constructs for which that is not already specified. Given that this rewrite happens before semantics checks, the check of the order in which ATOMIC constructs without explicit memory order and REQUIRES directives with `atomic_default_mem_order` appear is moved earlier into the rewrite pass. Otherwise, these problems would not be caught by semantics checks, since the PFT would be modified by that stage. This is patch 4/5 of a series splitting D149337 to simplify review. Depends on D157983. Differential Revision: https://reviews.llvm.org/D158096
1 parent 7338eb5 commit b49f846

8 files changed

+423
-15
lines changed

flang/lib/Semantics/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ add_flang_library(FortranSemantics
3636
resolve-directives.cpp
3737
resolve-names-utils.cpp
3838
resolve-names.cpp
39+
rewrite-directives.cpp
3940
rewrite-parse-tree.cpp
4041
runtime-type-info.cpp
4142
scope.cpp

flang/lib/Semantics/check-omp-structure.cpp

+1-13
Original file line numberDiff line numberDiff line change
@@ -1879,9 +1879,6 @@ void OmpStructureChecker::CheckAtomicMemoryOrderClause(
18791879
if (rightHandClauseList) {
18801880
checkForValidMemoryOrderClause(rightHandClauseList);
18811881
}
1882-
if (numMemoryOrderClause == 0) {
1883-
atomicDirectiveDefaultOrderFound_ = true;
1884-
}
18851882
}
18861883

18871884
void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
@@ -3219,16 +3216,7 @@ void OmpStructureChecker::Enter(
32193216
void OmpStructureChecker::CheckAllowedRequiresClause(llvmOmpClause clause) {
32203217
CheckAllowed(clause);
32213218

3222-
if (clause == llvm::omp::Clause::OMPC_atomic_default_mem_order) {
3223-
// Check that it does not appear after an atomic operation without memory
3224-
// order
3225-
if (atomicDirectiveDefaultOrderFound_) {
3226-
context_.Say(GetContext().clauseSource,
3227-
"REQUIRES directive with '%s' clause found lexically after atomic "
3228-
"operation without a memory order clause"_err_en_US,
3229-
parser::ToUpperCaseLetters(getClauseName(clause).str()));
3230-
}
3231-
} else {
3219+
if (clause != llvm::omp::Clause::OMPC_atomic_default_mem_order) {
32323220
// Check that it does not appear after a device construct
32333221
if (deviceConstructFound_) {
32343222
context_.Say(GetContext().clauseSource,

flang/lib/Semantics/check-omp-structure.h

-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,6 @@ class OmpStructureChecker
210210

211211
void CheckAllowedRequiresClause(llvmOmpClause clause);
212212
bool deviceConstructFound_{false};
213-
bool atomicDirectiveDefaultOrderFound_{false};
214213

215214
void EnterDirectiveNest(const int index) { directiveNest_[index]++; }
216215
void ExitDirectiveNest(const int index) { directiveNest_[index]--; }
+177
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
//===-- lib/Semantics/rewrite-directives.cpp ------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "rewrite-directives.h"
10+
#include "flang/Parser/parse-tree-visitor.h"
11+
#include "flang/Parser/parse-tree.h"
12+
#include "flang/Semantics/semantics.h"
13+
#include "flang/Semantics/symbol.h"
14+
#include "llvm/Frontend/OpenMP/OMP.h.inc"
15+
#include <list>
16+
17+
namespace Fortran::semantics {
18+
19+
using namespace parser::literals;
20+
21+
class DirectiveRewriteMutator {
22+
public:
23+
explicit DirectiveRewriteMutator(SemanticsContext &context)
24+
: context_{context} {}
25+
26+
// Default action for a parse tree node is to visit children.
27+
template <typename T> bool Pre(T &) { return true; }
28+
template <typename T> void Post(T &) {}
29+
30+
protected:
31+
SemanticsContext &context_;
32+
};
33+
34+
// Rewrite atomic constructs to add an explicit memory ordering to all that do
35+
// not specify it, honoring in this way the `atomic_default_mem_order` clause of
36+
// the REQUIRES directive.
37+
class OmpRewriteMutator : public DirectiveRewriteMutator {
38+
public:
39+
explicit OmpRewriteMutator(SemanticsContext &context)
40+
: DirectiveRewriteMutator(context) {}
41+
42+
template <typename T> bool Pre(T &) { return true; }
43+
template <typename T> void Post(T &) {}
44+
45+
bool Pre(parser::OpenMPAtomicConstruct &);
46+
bool Pre(parser::OpenMPRequiresConstruct &);
47+
48+
private:
49+
bool atomicDirectiveDefaultOrderFound_{false};
50+
};
51+
52+
bool OmpRewriteMutator::Pre(parser::OpenMPAtomicConstruct &x) {
53+
// Find top-level parent of the operation.
54+
Symbol *topLevelParent{common::visit(
55+
[&](auto &atomic) {
56+
Symbol *symbol{nullptr};
57+
Scope *scope{
58+
&context_.FindScope(std::get<parser::Verbatim>(atomic.t).source)};
59+
do {
60+
if (Symbol * parent{scope->symbol()}) {
61+
symbol = parent;
62+
}
63+
scope = &scope->parent();
64+
} while (!scope->IsGlobal());
65+
66+
assert(symbol &&
67+
"Atomic construct must be within a scope associated with a symbol");
68+
return symbol;
69+
},
70+
x.u)};
71+
72+
// Get the `atomic_default_mem_order` clause from the top-level parent.
73+
std::optional<common::OmpAtomicDefaultMemOrderType> defaultMemOrder;
74+
common::visit(
75+
[&](auto &details) {
76+
if constexpr (std::is_convertible_v<decltype(&details),
77+
WithOmpDeclarative *>) {
78+
if (details.has_ompAtomicDefaultMemOrder()) {
79+
defaultMemOrder = *details.ompAtomicDefaultMemOrder();
80+
}
81+
}
82+
},
83+
topLevelParent->details());
84+
85+
if (!defaultMemOrder) {
86+
return false;
87+
}
88+
89+
auto findMemOrderClause =
90+
[](const std::list<parser::OmpAtomicClause> &clauses) {
91+
return std::find_if(
92+
clauses.begin(), clauses.end(), [](const auto &clause) {
93+
return std::get_if<parser::OmpMemoryOrderClause>(
94+
&clause.u);
95+
}) != clauses.end();
96+
};
97+
98+
// Get the clause list to which the new memory order clause must be added,
99+
// only if there are no other memory order clauses present for this atomic
100+
// directive.
101+
std::list<parser::OmpAtomicClause> *clauseList = common::visit(
102+
common::visitors{[&](parser::OmpAtomic &atomicConstruct) {
103+
// OmpAtomic only has a single list of clauses.
104+
auto &clauses{std::get<parser::OmpAtomicClauseList>(
105+
atomicConstruct.t)};
106+
return !findMemOrderClause(clauses.v) ? &clauses.v
107+
: nullptr;
108+
},
109+
[&](auto &atomicConstruct) {
110+
// All other atomic constructs have two lists of clauses.
111+
auto &clausesLhs{std::get<0>(atomicConstruct.t)};
112+
auto &clausesRhs{std::get<2>(atomicConstruct.t)};
113+
return !findMemOrderClause(clausesLhs.v) &&
114+
!findMemOrderClause(clausesRhs.v)
115+
? &clausesRhs.v
116+
: nullptr;
117+
}},
118+
x.u);
119+
120+
// Add a memory order clause to the atomic directive.
121+
if (clauseList) {
122+
atomicDirectiveDefaultOrderFound_ = true;
123+
switch (*defaultMemOrder) {
124+
case common::OmpAtomicDefaultMemOrderType::AcqRel:
125+
clauseList->emplace_back<parser::OmpMemoryOrderClause>(common::visit(
126+
common::visitors{[](parser::OmpAtomicRead &) -> parser::OmpClause {
127+
return parser::OmpClause::Acquire{};
128+
},
129+
[](parser::OmpAtomicCapture &) -> parser::OmpClause {
130+
return parser::OmpClause::AcqRel{};
131+
},
132+
[](auto &) -> parser::OmpClause {
133+
// parser::{OmpAtomic, OmpAtomicUpdate, OmpAtomicWrite}
134+
return parser::OmpClause::Release{};
135+
}},
136+
x.u));
137+
break;
138+
case common::OmpAtomicDefaultMemOrderType::Relaxed:
139+
clauseList->emplace_back<parser::OmpMemoryOrderClause>(
140+
parser::OmpClause{parser::OmpClause::Relaxed{}});
141+
break;
142+
case common::OmpAtomicDefaultMemOrderType::SeqCst:
143+
clauseList->emplace_back<parser::OmpMemoryOrderClause>(
144+
parser::OmpClause{parser::OmpClause::SeqCst{}});
145+
break;
146+
}
147+
}
148+
149+
return false;
150+
}
151+
152+
bool OmpRewriteMutator::Pre(parser::OpenMPRequiresConstruct &x) {
153+
for (parser::OmpClause &clause : std::get<parser::OmpClauseList>(x.t).v) {
154+
if (std::holds_alternative<parser::OmpClause::AtomicDefaultMemOrder>(
155+
clause.u) &&
156+
atomicDirectiveDefaultOrderFound_) {
157+
context_.Say(clause.source,
158+
"REQUIRES directive with '%s' clause found lexically after atomic "
159+
"operation without a memory order clause"_err_en_US,
160+
parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
161+
llvm::omp::OMPC_atomic_default_mem_order)
162+
.str()));
163+
}
164+
}
165+
return false;
166+
}
167+
168+
bool RewriteOmpParts(SemanticsContext &context, parser::Program &program) {
169+
if (!context.IsEnabled(common::LanguageFeature::OpenMP)) {
170+
return true;
171+
}
172+
OmpRewriteMutator ompMutator{context};
173+
parser::Walk(program, ompMutator);
174+
return !context.AnyFatalError();
175+
}
176+
177+
} // namespace Fortran::semantics
+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//===-- lib/Semantics/rewrite-directives.h ----------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef FORTRAN_SEMANTICS_REWRITE_DIRECTIVES_H_
10+
#define FORTRAN_SEMANTICS_REWRITE_DIRECTIVES_H_
11+
12+
namespace Fortran::parser {
13+
struct Program;
14+
} // namespace Fortran::parser
15+
16+
namespace Fortran::semantics {
17+
class SemanticsContext;
18+
} // namespace Fortran::semantics
19+
20+
namespace Fortran::semantics {
21+
bool RewriteOmpParts(SemanticsContext &, parser::Program &);
22+
} // namespace Fortran::semantics
23+
24+
#endif // FORTRAN_SEMANTICS_REWRITE_DIRECTIVES_H_

flang/lib/Semantics/rewrite-parse-tree.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "rewrite-parse-tree.h"
10+
#include "rewrite-directives.h"
1011
#include "flang/Common/indirection.h"
1112
#include "flang/Parser/parse-tree-visitor.h"
1213
#include "flang/Parser/parse-tree.h"
@@ -175,7 +176,7 @@ void RewriteMutator::Post(parser::WriteStmt &x) {
175176
bool RewriteParseTree(SemanticsContext &context, parser::Program &program) {
176177
RewriteMutator mutator{context};
177178
parser::Walk(program, mutator);
178-
return !context.AnyFatalError();
179+
return !context.AnyFatalError() && RewriteOmpParts(context, program);
179180
}
180181

181182
} // namespace Fortran::semantics
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
! RUN: %flang_fc1 -fopenmp -fdebug-dump-parse-tree %s 2>&1 | FileCheck %s
2+
! Ensure that requires atomic_default_mem_order is used to update atomic
3+
! operations with no explicit memory order set.
4+
program requires
5+
implicit none
6+
!$omp requires atomic_default_mem_order(seq_cst)
7+
integer :: i, j
8+
9+
! ----------------------------------------------------------------------------
10+
! READ
11+
! ----------------------------------------------------------------------------
12+
13+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicRead
14+
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
15+
!$omp atomic read
16+
i = j
17+
18+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicRead
19+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
20+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
21+
!$omp atomic relaxed read
22+
i = j
23+
24+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicRead
25+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
26+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
27+
!$omp atomic read relaxed
28+
i = j
29+
30+
! ----------------------------------------------------------------------------
31+
! WRITE
32+
! ----------------------------------------------------------------------------
33+
34+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicWrite
35+
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
36+
!$omp atomic write
37+
i = j
38+
39+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicWrite
40+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
41+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
42+
!$omp atomic relaxed write
43+
i = j
44+
45+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicWrite
46+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
47+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
48+
!$omp atomic write relaxed
49+
i = j
50+
51+
! ----------------------------------------------------------------------------
52+
! UPDATE
53+
! ----------------------------------------------------------------------------
54+
55+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicUpdate
56+
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
57+
!$omp atomic update
58+
i = i + j
59+
60+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicUpdate
61+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
62+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
63+
!$omp atomic relaxed update
64+
i = i + j
65+
66+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicUpdate
67+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
68+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
69+
!$omp atomic update relaxed
70+
i = i + j
71+
72+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomic
73+
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
74+
!$omp atomic
75+
i = i + j
76+
77+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomic
78+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
79+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
80+
!$omp atomic relaxed
81+
i = i + j
82+
83+
! ----------------------------------------------------------------------------
84+
! CAPTURE
85+
! ----------------------------------------------------------------------------
86+
87+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
88+
! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
89+
!$omp atomic capture
90+
i = j
91+
i = j
92+
!$omp end atomic
93+
94+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
95+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
96+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
97+
!$omp atomic relaxed capture
98+
i = j
99+
i = j
100+
!$omp end atomic
101+
102+
! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
103+
! CHECK-NOT: OmpMemoryOrderClause -> OmpClause -> SeqCst
104+
! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
105+
!$omp atomic capture relaxed
106+
i = j
107+
i = j
108+
!$omp end atomic
109+
end program requires

0 commit comments

Comments
 (0)