Skip to content

Commit af48237

Browse files
kparzyszkparzysz
committed
[flang][OpenMP] Introduce OmpHintClause, simplify OmpAtomicClause
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.
1 parent 45f2716 commit af48237

File tree

11 files changed

+69
-47
lines changed

11 files changed

+69
-47
lines changed

‎flang/include/flang/Lower/DirectivesCommon.h‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,11 @@ static inline void genOmpAtomicHintAndMemoryOrderClauses(
5555
mlir::omp::ClauseMemoryOrderKindAttr &memoryOrder) {
5656
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
5757
for (const Fortran::parser::OmpAtomicClause &clause : clauseList.v) {
58-
if (const auto *ompClause =
59-
std::get_if<Fortran::parser::OmpClause>(&clause.u)) {
60-
if (const auto *hintClause =
61-
std::get_if<Fortran::parser::OmpClause::Hint>(&ompClause->u)) {
62-
const auto *expr = Fortran::semantics::GetExpr(hintClause->v);
63-
uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr);
64-
hint = firOpBuilder.getI64IntegerAttr(hintExprValue);
65-
}
58+
if (const auto *hintClause =
59+
std::get_if<Fortran::parser::OmpHintClause>(&clause.u)) {
60+
const auto *expr = Fortran::semantics::GetExpr(hintClause->v);
61+
uint64_t hintExprValue = *Fortran::evaluate::ToInt64(*expr);
62+
hint = firOpBuilder.getI64IntegerAttr(hintExprValue);
6663
} else if (const auto *ompMemoryOrderClause =
6764
std::get_if<Fortran::parser::OmpMemoryOrderClause>(
6865
&clause.u)) {

‎flang/include/flang/Parser/dump-parse-tree.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ class ParseTreeDumper {
591591
NODE(OmpFromClause, Modifier)
592592
NODE(parser, OmpExpectation)
593593
NODE_ENUM(OmpExpectation, Value)
594+
NODE(parser, OmpHintClause)
594595
NODE(parser, OmpHoldsClause)
595596
NODE(parser, OmpIfClause)
596597
NODE(OmpIfClause, Modifier)

‎flang/include/flang/Parser/parse-tree.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4272,6 +4272,11 @@ struct OmpGrainsizeClause {
42724272
std::tuple<MODIFIERS(), ScalarIntExpr> t;
42734273
};
42744274

4275+
// Ref: [5.0:234-242], [5.1:266-275], [5.2:299], [6.0:472-473]
4276+
struct OmpHintClause {
4277+
WRAPPER_CLASS_BOILERPLATE(OmpHintClause, ScalarIntConstantExpr);
4278+
};
4279+
42754280
// Ref: [5.2: 214]
42764281
//
42774282
// holds-clause ->
@@ -4832,7 +4837,7 @@ struct OmpMemoryOrderClause {
48324837
struct OmpAtomicClause {
48334838
UNION_CLASS_BOILERPLATE(OmpAtomicClause);
48344839
CharBlock source;
4835-
std::variant<OmpMemoryOrderClause, OmpFailClause, OmpClause> u;
4840+
std::variant<OmpMemoryOrderClause, OmpFailClause, OmpHintClause> u;
48364841
};
48374842

48384843
// atomic-clause-list -> [atomic-clause, [atomic-clause], ...]

‎flang/lib/Lower/OpenMP/Clauses.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,8 +863,8 @@ HasDeviceAddr make(const parser::OmpClause::HasDeviceAddr &inp,
863863

864864
Hint make(const parser::OmpClause::Hint &inp,
865865
semantics::SemanticsContext &semaCtx) {
866-
// inp.v -> parser::ConstantExpr
867-
return Hint{/*HintExpr=*/makeExpr(inp.v, semaCtx)};
866+
// inp.v -> parser::OmpHintClause
867+
return Hint{/*HintExpr=*/makeExpr(inp.v.v, semaCtx)};
868868
}
869869

870870
Holds make(const parser::OmpClause::Holds &inp,

‎flang/lib/Parser/openmp-parsers.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,8 @@ TYPE_PARSER(
804804
// OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle)
805805
TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
806806

807+
TYPE_PARSER(construct<OmpHintClause>(scalarIntConstantExpr))
808+
807809
// init clause
808810
TYPE_PARSER(construct<OmpInitClause>(
809811
maybe(nonemptyList(Parser<OmpInitClause::Modifier>{}) / ":"),
@@ -941,8 +943,8 @@ TYPE_PARSER( //
941943
"HAS_DEVICE_ADDR" >>
942944
construct<OmpClause>(construct<OmpClause::HasDeviceAddr>(
943945
parenthesized(Parser<OmpObjectList>{}))) ||
944-
"HINT" >> construct<OmpClause>(
945-
construct<OmpClause::Hint>(parenthesized(constantExpr))) ||
946+
"HINT" >> construct<OmpClause>(construct<OmpClause::Hint>(
947+
parenthesized(Parser<OmpHintClause>{}))) ||
946948
"HOLDS" >> construct<OmpClause>(construct<OmpClause::Holds>(
947949
parenthesized(Parser<OmpHoldsClause>{}))) ||
948950
"IF" >> construct<OmpClause>(construct<OmpClause::If>(
@@ -1217,9 +1219,8 @@ TYPE_PARSER(construct<OmpAtomicDefaultMemOrderClause>(
12171219
TYPE_PARSER(sourced(construct<OmpAtomicClause>(
12181220
construct<OmpAtomicClause>(Parser<OmpMemoryOrderClause>{}) ||
12191221
construct<OmpAtomicClause>("FAIL" >> Parser<OmpFailClause>{}) ||
1220-
construct<OmpAtomicClause>("HINT" >>
1221-
sourced(construct<OmpClause>(
1222-
construct<OmpClause::Hint>(parenthesized(constantExpr))))))))
1222+
construct<OmpAtomicClause>(
1223+
"HINT" >> parenthesized(Parser<OmpHintClause>{})))))
12231224

12241225
// atomic-clause-list -> [atomic-clause, [atomic-clause], ...]
12251226
TYPE_PARSER(sourced(construct<OmpAtomicClauseList>(

‎flang/lib/Parser/unparse.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2890,12 +2890,17 @@ class UnparseVisitor {
28902890
Walk(x.v);
28912891
Put(")");
28922892
}
2893+
void Unparse(const OmpHintClause &x) {
2894+
Word("HINT(");
2895+
Walk(x.v);
2896+
Put(")");
2897+
}
28932898
void Unparse(const OmpMemoryOrderClause &x) { Walk(x.v); }
28942899
void Unparse(const OmpAtomicClause &x) {
28952900
common::visit(common::visitors{
28962901
[&](const OmpMemoryOrderClause &y) { Walk(y); },
28972902
[&](const OmpFailClause &y) { Walk(y); },
2898-
[&](const OmpClause &z) { Walk(z); },
2903+
[&](const OmpHintClause &y) { Walk(y); },
28992904
},
29002905
x.u);
29012906
}

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

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -586,33 +586,40 @@ void OmpStructureChecker::CheckPredefinedAllocatorRestriction(
586586

587587
template <class D>
588588
void OmpStructureChecker::CheckHintClause(
589-
D *leftOmpClauseList, D *rightOmpClauseList) {
589+
D *leftOmpClauseList, D *rightOmpClauseList, std::string_view dirName) {
590+
bool foundHint{false};
591+
590592
auto checkForValidHintClause = [&](const D *clauseList) {
591593
for (const auto &clause : clauseList->v) {
592-
const parser::OmpClause *ompClause = nullptr;
594+
const parser::OmpHintClause *ompHintClause = nullptr;
593595
if constexpr (std::is_same_v<D, const parser::OmpAtomicClauseList>) {
594-
ompClause = std::get_if<parser::OmpClause>(&clause.u);
595-
if (!ompClause)
596-
continue;
596+
ompHintClause = std::get_if<parser::OmpHintClause>(&clause.u);
597597
} else if constexpr (std::is_same_v<D, const parser::OmpClauseList>) {
598-
ompClause = &clause;
598+
if (auto *hint{std::get_if<parser::OmpClause::Hint>(&clause.u)}) {
599+
ompHintClause = &hint->v;
600+
}
599601
}
600-
if (const parser::OmpClause::Hint *hintClause{
601-
std::get_if<parser::OmpClause::Hint>(&ompClause->u)}) {
602-
std::optional<std::int64_t> hintValue = GetIntValue(hintClause->v);
603-
if (hintValue && *hintValue >= 0) {
604-
/*`omp_sync_hint_nonspeculative` and `omp_lock_hint_speculative`*/
605-
if ((*hintValue & 0xC) == 0xC
606-
/*`omp_sync_hint_uncontended` and omp_sync_hint_contended*/
607-
|| (*hintValue & 0x3) == 0x3)
608-
context_.Say(clause.source,
609-
"Hint clause value "
610-
"is not a valid OpenMP synchronization value"_err_en_US);
611-
} else {
602+
if (!ompHintClause)
603+
continue;
604+
if (foundHint) {
605+
context_.Say(clause.source,
606+
"At most one HINT clause can appear on the %s directive"_err_en_US,
607+
parser::ToUpperCaseLetters(dirName));
608+
}
609+
foundHint = true;
610+
std::optional<std::int64_t> hintValue = GetIntValue(ompHintClause->v);
611+
if (hintValue && *hintValue >= 0) {
612+
/*`omp_sync_hint_nonspeculative` and `omp_lock_hint_speculative`*/
613+
if ((*hintValue & 0xC) == 0xC
614+
/*`omp_sync_hint_uncontended` and omp_sync_hint_contended*/
615+
|| (*hintValue & 0x3) == 0x3)
612616
context_.Say(clause.source,
613-
"Hint clause must have non-negative constant "
614-
"integer expression"_err_en_US);
615-
}
617+
"Hint clause value "
618+
"is not a valid OpenMP synchronization value"_err_en_US);
619+
} else {
620+
context_.Say(clause.source,
621+
"Hint clause must have non-negative constant "
622+
"integer expression"_err_en_US);
616623
}
617624
}
618625
};
@@ -2375,7 +2382,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPCriticalConstruct &x) {
23752382
"Hint clause other than omp_sync_hint_none cannot be specified for "
23762383
"an unnamed CRITICAL directive"_err_en_US});
23772384
}
2378-
CheckHintClause<const parser::OmpClauseList>(&ompClause, nullptr);
2385+
CheckHintClause<const parser::OmpClauseList>(&ompClause, nullptr, "CRITICAL");
23792386
}
23802387

23812388
void OmpStructureChecker::Leave(const parser::OpenMPCriticalConstruct &) {
@@ -2950,7 +2957,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
29502957
nullptr);
29512958
CheckHintClause<const parser::OmpAtomicClauseList>(
29522959
&std::get<parser::OmpAtomicClauseList>(atomicConstruct.t),
2953-
nullptr);
2960+
nullptr, "ATOMIC");
29542961
},
29552962
[&](const parser::OmpAtomicUpdate &atomicUpdate) {
29562963
const auto &dir{std::get<parser::Verbatim>(atomicUpdate.t)};
@@ -2963,7 +2970,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
29632970
CheckAtomicMemoryOrderClause(
29642971
&std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t));
29652972
CheckHintClause<const parser::OmpAtomicClauseList>(
2966-
&std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t));
2973+
&std::get<0>(atomicUpdate.t), &std::get<2>(atomicUpdate.t),
2974+
"UPDATE");
29672975
},
29682976
[&](const parser::OmpAtomicRead &atomicRead) {
29692977
const auto &dir{std::get<parser::Verbatim>(atomicRead.t)};
@@ -2972,7 +2980,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
29722980
CheckAtomicMemoryOrderClause(
29732981
&std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t));
29742982
CheckHintClause<const parser::OmpAtomicClauseList>(
2975-
&std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t));
2983+
&std::get<0>(atomicRead.t), &std::get<2>(atomicRead.t), "READ");
29762984
CheckAtomicCaptureStmt(
29772985
std::get<parser::Statement<parser::AssignmentStmt>>(
29782986
atomicRead.t)
@@ -2985,7 +2993,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
29852993
CheckAtomicMemoryOrderClause(
29862994
&std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t));
29872995
CheckHintClause<const parser::OmpAtomicClauseList>(
2988-
&std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t));
2996+
&std::get<0>(atomicWrite.t), &std::get<2>(atomicWrite.t),
2997+
"WRITE");
29892998
CheckAtomicWriteStmt(
29902999
std::get<parser::Statement<parser::AssignmentStmt>>(
29913000
atomicWrite.t)
@@ -2998,7 +3007,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
29983007
CheckAtomicMemoryOrderClause(
29993008
&std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
30003009
CheckHintClause<const parser::OmpAtomicClauseList>(
3001-
&std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
3010+
&std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t),
3011+
"CAPTURE");
30023012
CheckAtomicCaptureConstruct(atomicCapture);
30033013
},
30043014
[&](const parser::OmpAtomicCompare &atomicCompare) {
@@ -3008,7 +3018,8 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
30083018
CheckAtomicMemoryOrderClause(
30093019
&std::get<0>(atomicCompare.t), &std::get<2>(atomicCompare.t));
30103020
CheckHintClause<const parser::OmpAtomicClauseList>(
3011-
&std::get<0>(atomicCompare.t), &std::get<2>(atomicCompare.t));
3021+
&std::get<0>(atomicCompare.t), &std::get<2>(atomicCompare.t),
3022+
"CAPTURE");
30123023
CheckAtomicCompareConstruct(atomicCompare);
30133024
},
30143025
},

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ class OmpStructureChecker
323323
void EnterDirectiveNest(const int index) { directiveNest_[index]++; }
324324
void ExitDirectiveNest(const int index) { directiveNest_[index]--; }
325325
int GetDirectiveNest(const int index) { return directiveNest_[index]; }
326-
template <typename D> void CheckHintClause(D *, D *);
326+
template <typename D> void CheckHintClause(D *, D *, std::string_view);
327327
inline void ErrIfAllocatableVariable(const parser::Variable &);
328328
inline void ErrIfLHSAndRHSSymbolsMatch(
329329
const parser::Variable &, const parser::Expr &);

‎flang/test/Semantics/OpenMP/atomic-hint-clause.f90‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ program sample
7878
y = y * 9
7979

8080
!ERROR: Hint clause must have non-negative constant integer expression
81+
!ERROR: Must have INTEGER type, but is REAL(4)
8182
!$omp atomic hint(1.0) read
8283
y = x
8384

‎flang/test/Semantics/OpenMP/critical-hint-clause.f90‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ program sample
9595
!$omp end critical (name)
9696

9797
!ERROR: Hint clause must have non-negative constant integer expression
98+
!ERROR: Must have INTEGER type, but is REAL(4)
9899
!$omp critical (name) hint(1.0)
99100
y = 2
100101
!$omp end critical (name)

0 commit comments

Comments
 (0)