Skip to content

Commit 7887459

Browse files
kparzyszmahesh-attarde
kparzysz
authored and
mahesh-attarde
committed
[flang][OpenMP] Parse strictly- and loosely-structured blocks (llvm#150298)
Block-associated constructs have, as their body, either a strictly- or a loosely-structured block. In the former case the end-directive is optional. The existing parser required the end-directive to be present in all cases. Note: The definitions of these blocks in the OpenMP spec exclude cases where the block contains more than one construct, and the first one is BLOCK/ENDBLOCK. For example, the following is invalid: ``` !$omp target block ! This cannot be a strictly-structured block, but continue ! a loosely-structured block cannot start with endblock ! BLOCK/ENDBLOCK continue ! !$omp end target ```
1 parent d89d9c4 commit 7887459

File tree

7 files changed

+433
-29
lines changed

7 files changed

+433
-29
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5119,7 +5119,8 @@ struct OmpEndBlockDirective {
51195119

51205120
struct OpenMPBlockConstruct {
51215121
TUPLE_CLASS_BOILERPLATE(OpenMPBlockConstruct);
5122-
std::tuple<OmpBeginBlockDirective, Block, OmpEndBlockDirective> t;
5122+
std::tuple<OmpBeginBlockDirective, Block, std::optional<OmpEndBlockDirective>>
5123+
t;
51235124
};
51245125

51255126
// OpenMP directives enclosing do loop

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,12 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
411411
std::get<parser::OmpBeginBlockDirective>(ompConstruct.t);
412412
beginClauseList =
413413
&std::get<parser::OmpClauseList>(beginDirective.t);
414-
endClauseList = &std::get<parser::OmpClauseList>(
415-
std::get<parser::OmpEndBlockDirective>(ompConstruct.t).t);
414+
if (auto &endDirective =
415+
std::get<std::optional<parser::OmpEndBlockDirective>>(
416+
ompConstruct.t)) {
417+
endClauseList =
418+
&std::get<parser::OmpClauseList>(endDirective->t);
419+
}
416420
},
417421
[&](const parser::OpenMPLoopConstruct &ompConstruct) {
418422
const auto &beginDirective =
@@ -422,9 +426,10 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
422426

423427
if (auto &endDirective =
424428
std::get<std::optional<parser::OmpEndLoopDirective>>(
425-
ompConstruct.t))
429+
ompConstruct.t)) {
426430
endClauseList =
427431
&std::get<parser::OmpClauseList>(endDirective->t);
432+
}
428433
},
429434
[&](const auto &) {}},
430435
ompEval->u);
@@ -3713,16 +3718,19 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
37133718
const parser::OpenMPBlockConstruct &blockConstruct) {
37143719
const auto &beginBlockDirective =
37153720
std::get<parser::OmpBeginBlockDirective>(blockConstruct.t);
3716-
const auto &endBlockDirective =
3717-
std::get<parser::OmpEndBlockDirective>(blockConstruct.t);
37183721
mlir::Location currentLocation =
37193722
converter.genLocation(beginBlockDirective.source);
37203723
const auto origDirective =
37213724
std::get<parser::OmpBlockDirective>(beginBlockDirective.t).v;
37223725
List<Clause> clauses = makeClauses(
37233726
std::get<parser::OmpClauseList>(beginBlockDirective.t), semaCtx);
3724-
clauses.append(makeClauses(
3725-
std::get<parser::OmpClauseList>(endBlockDirective.t), semaCtx));
3727+
3728+
if (const auto &endBlockDirective =
3729+
std::get<std::optional<parser::OmpEndBlockDirective>>(
3730+
blockConstruct.t)) {
3731+
clauses.append(makeClauses(
3732+
std::get<parser::OmpClauseList>(endBlockDirective->t), semaCtx));
3733+
}
37263734

37273735
assert(llvm::omp::blockConstructSet.test(origDirective) &&
37283736
"Expected block construct");

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

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,6 +1208,54 @@ TYPE_PARSER(sourced(
12081208
maybe(Parser<OmpClauseList>{}),
12091209
pure(OmpDirectiveSpecification::Flags::None))))
12101210

1211+
static bool IsFortranBlockConstruct(const ExecutionPartConstruct &epc) {
1212+
// ExecutionPartConstruct -> ExecutableConstruct
1213+
// -> Indirection<BlockConstruct>
1214+
if (auto *ec{std::get_if<ExecutableConstruct>(&epc.u)}) {
1215+
return std::holds_alternative<common::Indirection<BlockConstruct>>(ec->u);
1216+
} else {
1217+
return false;
1218+
}
1219+
}
1220+
1221+
struct StrictlyStructuredBlockParser {
1222+
using resultType = Block;
1223+
1224+
std::optional<resultType> Parse(ParseState &state) const {
1225+
if (auto epc{Parser<ExecutionPartConstruct>{}.Parse(state)}) {
1226+
if (IsFortranBlockConstruct(*epc)) {
1227+
Block block;
1228+
block.emplace_back(std::move(*epc));
1229+
return std::move(block);
1230+
}
1231+
}
1232+
return std::nullopt;
1233+
}
1234+
};
1235+
1236+
struct LooselyStructuredBlockParser {
1237+
using resultType = Block;
1238+
1239+
std::optional<resultType> Parse(ParseState &state) const {
1240+
Block body;
1241+
if (auto epc{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) {
1242+
if (!IsFortranBlockConstruct(*epc)) {
1243+
body.emplace_back(std::move(*epc));
1244+
if (auto &&blk{attempt(block).Parse(state)}) {
1245+
for (auto &&s : *blk) {
1246+
body.emplace_back(std::move(s));
1247+
}
1248+
}
1249+
} else {
1250+
// Fail if the first construct is BLOCK.
1251+
return std::nullopt;
1252+
}
1253+
}
1254+
// Empty body is ok.
1255+
return std::move(body);
1256+
}
1257+
};
1258+
12111259
TYPE_PARSER(sourced(construct<OmpNothingDirective>("NOTHING" >> ok)))
12121260

12131261
TYPE_PARSER(sourced(construct<OpenMPUtilityConstruct>(
@@ -1570,12 +1618,16 @@ TYPE_PARSER(
15701618
Parser<OpenMPInteropConstruct>{})) /
15711619
endOfLine)
15721620

1621+
// Directive names (of non-block constructs) whose prefix is a name of
1622+
// a block-associated construct. We need to exclude them from the block
1623+
// directive parser below to avoid parsing parts of them.
1624+
static constexpr auto StandaloneDirectiveLookahead{//
1625+
"TARGET ENTER DATA"_sptok || "TARGET_ENTER_DATA"_sptok || //
1626+
"TARGET EXIT DATA"_sptok || "TARGET_EXIT"_sptok || //
1627+
"TARGET UPDATE"_sptok || "TARGET_UPDATE"_sptok};
1628+
15731629
// Directives enclosing structured-block
1574-
TYPE_PARSER(
1575-
// In this context "TARGET UPDATE" can be parsed as a TARGET directive
1576-
// followed by an UPDATE clause. This is the only combination at the
1577-
// moment, exclude it explicitly.
1578-
(!("TARGET UPDATE"_sptok || "TARGET_UPDATE"_sptok)) >=
1630+
TYPE_PARSER(!StandaloneDirectiveLookahead >=
15791631
construct<OmpBlockDirective>(first(
15801632
"MASKED" >> pure(llvm::omp::Directive::OMPD_masked),
15811633
"MASTER" >> pure(llvm::omp::Directive::OMPD_master),
@@ -1749,9 +1801,12 @@ TYPE_PARSER(sourced(
17491801
block, maybe(Parser<OmpEndAssumeDirective>{} / endOmpLine))))
17501802

17511803
// Block Construct
1752-
TYPE_PARSER(construct<OpenMPBlockConstruct>(
1753-
Parser<OmpBeginBlockDirective>{} / endOmpLine, block,
1754-
Parser<OmpEndBlockDirective>{} / endOmpLine))
1804+
TYPE_PARSER( //
1805+
construct<OpenMPBlockConstruct>(Parser<OmpBeginBlockDirective>{},
1806+
StrictlyStructuredBlockParser{},
1807+
maybe(Parser<OmpEndBlockDirective>{})) ||
1808+
construct<OpenMPBlockConstruct>(Parser<OmpBeginBlockDirective>{},
1809+
LooselyStructuredBlockParser{}, Parser<OmpEndBlockDirective>{}))
17551810

17561811
// OMP SECTIONS Directive
17571812
TYPE_PARSER(construct<OmpSectionsDirective>(first(

‎flang/lib/Parser/unparse.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2898,11 +2898,13 @@ class UnparseVisitor {
28982898
Put("\n");
28992899
EndOpenMP();
29002900
Walk(std::get<Block>(x.t), "");
2901-
BeginOpenMP();
2902-
Word("!$OMP END ");
2903-
Walk(std::get<OmpEndBlockDirective>(x.t));
2904-
Put("\n");
2905-
EndOpenMP();
2901+
if (auto &&end{std::get<std::optional<OmpEndBlockDirective>>(x.t)}) {
2902+
BeginOpenMP();
2903+
Word("!$OMP END ");
2904+
Walk(*end);
2905+
Put("\n");
2906+
EndOpenMP();
2907+
}
29062908
}
29072909
void Unparse(const OpenMPLoopConstruct &x) {
29082910
BeginOpenMP();

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -781,12 +781,15 @@ void OmpStructureChecker::CheckTargetNest(const parser::OpenMPConstruct &c) {
781781

782782
void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
783783
const auto &beginBlockDir{std::get<parser::OmpBeginBlockDirective>(x.t)};
784-
const auto &endBlockDir{std::get<parser::OmpEndBlockDirective>(x.t)};
784+
const auto &endBlockDir{
785+
std::get<std::optional<parser::OmpEndBlockDirective>>(x.t)};
785786
const auto &beginDir{std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
786-
const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir.t)};
787787
const parser::Block &block{std::get<parser::Block>(x.t)};
788788

789-
CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
789+
if (endBlockDir) {
790+
const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir->t)};
791+
CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
792+
}
790793

791794
PushContextAndClauseSets(beginDir.source, beginDir.v);
792795
if (llvm::omp::allTargetSet.test(GetContext().directive)) {
@@ -836,14 +839,14 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
836839
bool foundNowait{false};
837840
parser::CharBlock NowaitSource;
838841

839-
auto catchCopyPrivateNowaitClauses = [&](const auto &dir, bool endDir) {
842+
auto catchCopyPrivateNowaitClauses = [&](const auto &dir, bool isEnd) {
840843
for (auto &clause : std::get<parser::OmpClauseList>(dir.t).v) {
841844
if (clause.Id() == llvm::omp::Clause::OMPC_copyprivate) {
842845
for (const auto &ompObject : GetOmpObjectList(clause)->v) {
843846
const auto *name{parser::Unwrap<parser::Name>(ompObject)};
844847
if (Symbol * symbol{name->symbol}) {
845848
if (singleCopyprivateSyms.count(symbol)) {
846-
if (endDir) {
849+
if (isEnd) {
847850
context_.Warn(common::UsageWarning::OpenMPUsage, name->source,
848851
"The COPYPRIVATE clause with '%s' is already used on the SINGLE directive"_warn_en_US,
849852
name->ToString());
@@ -857,7 +860,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
857860
"'%s' appears in more than one COPYPRIVATE clause on the END SINGLE directive"_err_en_US,
858861
name->ToString());
859862
} else {
860-
if (endDir) {
863+
if (isEnd) {
861864
endSingleCopyprivateSyms.insert(symbol);
862865
} else {
863866
singleCopyprivateSyms.insert(symbol);
@@ -870,7 +873,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
870873
context_.Say(clause.source,
871874
"At most one NOWAIT clause can appear on the SINGLE directive"_err_en_US);
872875
} else {
873-
foundNowait = !endDir;
876+
foundNowait = !isEnd;
874877
}
875878
if (!NowaitSource.ToString().size()) {
876879
NowaitSource = clause.source;
@@ -879,7 +882,9 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
879882
}
880883
};
881884
catchCopyPrivateNowaitClauses(beginBlockDir, false);
882-
catchCopyPrivateNowaitClauses(endBlockDir, true);
885+
if (endBlockDir) {
886+
catchCopyPrivateNowaitClauses(*endBlockDir, true);
887+
}
883888
unsigned version{context_.langOptions().OpenMPVersion};
884889
if (version <= 52 && NowaitSource.ToString().size() &&
885890
(singleCopyprivateSyms.size() || endSingleCopyprivateSyms.size())) {

0 commit comments

Comments
 (0)