Skip to content

Commit e368b53

Browse files
kparzyszkparzysz
authored
[flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (#152007)
This allows not having the END CRITICAL directive in certain situations. Update semantic checks and symbol resolution.
1 parent cfa00d4 commit e368b53

File tree

16 files changed

+154
-133
lines changed

16 files changed

+154
-133
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4986,9 +4986,9 @@ struct OmpEndCriticalDirective {
49864986
CharBlock source;
49874987
std::tuple<Verbatim, std::optional<Name>> t;
49884988
};
4989-
struct OpenMPCriticalConstruct {
4990-
TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
4991-
std::tuple<OmpCriticalDirective, Block, OmpEndCriticalDirective> t;
4989+
4990+
struct OpenMPCriticalConstruct : public OmpBlockConstruct {
4991+
INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct, OmpBlockConstruct);
49924992
};
49934993

49944994
// 2.11.3 allocate -> ALLOCATE [(variable-name-list)] [clause]

‎flang/lib/Semantics/openmp-utils.h‎ renamed to ‎flang/include/flang/Semantics/openmp-utils.h‎

File renamed without changes.

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "flang/Parser/openmp-utils.h"
3535
#include "flang/Parser/parse-tree.h"
3636
#include "flang/Semantics/openmp-directive-sets.h"
37+
#include "flang/Semantics/openmp-utils.h"
3738
#include "flang/Semantics/tools.h"
3839
#include "flang/Support/Flags.h"
3940
#include "flang/Support/OpenMP-utils.h"
@@ -3820,18 +3821,29 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
38203821
semantics::SemanticsContext &semaCtx,
38213822
lower::pft::Evaluation &eval,
38223823
const parser::OpenMPCriticalConstruct &criticalConstruct) {
3823-
const auto &cd = std::get<parser::OmpCriticalDirective>(criticalConstruct.t);
3824-
List<Clause> clauses =
3825-
makeClauses(std::get<parser::OmpClauseList>(cd.t), semaCtx);
3824+
const parser::OmpDirectiveSpecification &beginSpec =
3825+
criticalConstruct.BeginDir();
3826+
List<Clause> clauses = makeClauses(beginSpec.Clauses(), semaCtx);
38263827

38273828
ConstructQueue queue{buildConstructQueue(
3828-
converter.getFirOpBuilder().getModule(), semaCtx, eval, cd.source,
3829+
converter.getFirOpBuilder().getModule(), semaCtx, eval, beginSpec.source,
38293830
llvm::omp::Directive::OMPD_critical, clauses)};
38303831

3831-
const auto &name = std::get<std::optional<parser::Name>>(cd.t);
3832+
std::optional<parser::Name> critName;
3833+
const parser::OmpArgumentList &args = beginSpec.Arguments();
3834+
if (!args.v.empty()) {
3835+
// All of these things should be guaranteed to exist after semantic checks.
3836+
auto *object = parser::Unwrap<parser::OmpObject>(args.v.front());
3837+
assert(object && "Expecting object as argument");
3838+
auto *designator = semantics::omp::GetDesignatorFromObj(*object);
3839+
assert(designator && "Expecting desginator in argument");
3840+
auto *name = semantics::getDesignatorNameIfDataRef(*designator);
3841+
assert(name && "Expecting dataref in designator");
3842+
critName = *name;
3843+
}
38323844
mlir::Location currentLocation = converter.getCurrentLocation();
38333845
genCriticalOp(converter, symTable, semaCtx, eval, currentLocation, queue,
3834-
queue.begin(), name);
3846+
queue.begin(), critName);
38353847
}
38363848

38373849
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1758,17 +1758,8 @@ TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
17581758
TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
17591759
construct<OmpReductionCombiner>(Parser<FunctionReference>{}))
17601760

1761-
// 2.13.2 OMP CRITICAL
1762-
TYPE_PARSER(startOmpLine >>
1763-
sourced(construct<OmpEndCriticalDirective>(
1764-
verbatim("END CRITICAL"_tok), maybe(parenthesized(name)))) /
1765-
endOmpLine)
1766-
TYPE_PARSER(sourced(construct<OmpCriticalDirective>(verbatim("CRITICAL"_tok),
1767-
maybe(parenthesized(name)), Parser<OmpClauseList>{})) /
1768-
endOmpLine)
1769-
17701761
TYPE_PARSER(construct<OpenMPCriticalConstruct>(
1771-
Parser<OmpCriticalDirective>{}, block, Parser<OmpEndCriticalDirective>{}))
1762+
OmpBlockConstructParser{llvm::omp::Directive::OMPD_critical}))
17721763

17731764
// 2.11.3 Executable Allocate directive
17741765
TYPE_PARSER(

‎flang/lib/Parser/unparse.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2606,9 +2606,7 @@ class UnparseVisitor {
26062606
EndOpenMP();
26072607
}
26082608
void Unparse(const OpenMPCriticalConstruct &x) {
2609-
Walk(std::get<OmpCriticalDirective>(x.t));
2610-
Walk(std::get<Block>(x.t), "");
2611-
Walk(std::get<OmpEndCriticalDirective>(x.t));
2609+
Unparse(static_cast<const OmpBlockConstruct &>(x));
26122610
}
26132611
void Unparse(const OmpDeclareTargetWithList &x) {
26142612
Put("("), Walk(x.v), Put(")");

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "check-omp-structure.h"
14-
#include "openmp-utils.h"
1514

1615
#include "flang/Common/indirection.h"
1716
#include "flang/Evaluate/expression.h"
1817
#include "flang/Evaluate/tools.h"
1918
#include "flang/Parser/char-block.h"
2019
#include "flang/Parser/parse-tree.h"
20+
#include "flang/Semantics/openmp-utils.h"
2121
#include "flang/Semantics/symbol.h"
2222
#include "flang/Semantics/tools.h"
2323
#include "flang/Semantics/type.h"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
#include "check-omp-structure.h"
1414

1515
#include "check-directive-structure.h"
16-
#include "openmp-utils.h"
1716

1817
#include "flang/Common/idioms.h"
1918
#include "flang/Common/visit.h"
@@ -23,6 +22,7 @@
2322
#include "flang/Parser/parse-tree.h"
2423
#include "flang/Parser/tools.h"
2524
#include "flang/Semantics/openmp-modifiers.h"
25+
#include "flang/Semantics/openmp-utils.h"
2626
#include "flang/Semantics/semantics.h"
2727
#include "flang/Semantics/symbol.h"
2828
#include "flang/Semantics/tools.h"

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@
1212

1313
#include "check-omp-structure.h"
1414

15-
#include "openmp-utils.h"
16-
1715
#include "flang/Common/idioms.h"
1816
#include "flang/Common/indirection.h"
1917
#include "flang/Common/visit.h"
2018
#include "flang/Parser/characters.h"
2119
#include "flang/Parser/message.h"
2220
#include "flang/Parser/parse-tree.h"
2321
#include "flang/Semantics/openmp-modifiers.h"
22+
#include "flang/Semantics/openmp-utils.h"
2423
#include "flang/Semantics/tools.h"
2524

2625
#include "llvm/Frontend/OpenMP/OMP.h"

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

Lines changed: 81 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
#include "check-directive-structure.h"
1212
#include "definable.h"
13-
#include "openmp-utils.h"
1413
#include "resolve-names-utils.h"
1514

1615
#include "flang/Common/idioms.h"
@@ -27,6 +26,7 @@
2726
#include "flang/Semantics/expression.h"
2827
#include "flang/Semantics/openmp-directive-sets.h"
2928
#include "flang/Semantics/openmp-modifiers.h"
29+
#include "flang/Semantics/openmp-utils.h"
3030
#include "flang/Semantics/scope.h"
3131
#include "flang/Semantics/semantics.h"
3232
#include "flang/Semantics/symbol.h"
@@ -537,14 +537,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
537537
checker_(x.v.source, Directive::OMPD_assume);
538538
return false;
539539
}
540-
bool Pre(const parser::OmpCriticalDirective &x) {
541-
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_critical);
542-
return false;
543-
}
544-
bool Pre(const parser::OmpEndCriticalDirective &x) {
545-
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_critical);
546-
return false;
547-
}
548540
bool Pre(const parser::OmpMetadirectiveDirective &x) {
549541
checker_(
550542
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_metadirective);
@@ -2034,41 +2026,87 @@ void OmpStructureChecker::Leave(const parser::OpenMPCancelConstruct &) {
20342026
}
20352027

20362028
void OmpStructureChecker::Enter(const parser::OpenMPCriticalConstruct &x) {
2037-
const auto &dir{std::get<parser::OmpCriticalDirective>(x.t)};
2038-
const auto &dirSource{std::get<parser::Verbatim>(dir.t).source};
2039-
const auto &endDir{std::get<parser::OmpEndCriticalDirective>(x.t)};
2040-
PushContextAndClauseSets(dirSource, llvm::omp::Directive::OMPD_critical);
2029+
const parser::OmpBeginDirective &beginSpec{x.BeginDir()};
2030+
const std::optional<parser::OmpEndDirective> &endSpec{x.EndDir()};
2031+
PushContextAndClauseSets(beginSpec.DirName().source, beginSpec.DirName().v);
2032+
20412033
const auto &block{std::get<parser::Block>(x.t)};
2042-
CheckNoBranching(block, llvm::omp::Directive::OMPD_critical, dir.source);
2043-
const auto &dirName{std::get<std::optional<parser::Name>>(dir.t)};
2044-
const auto &endDirName{std::get<std::optional<parser::Name>>(endDir.t)};
2045-
const auto &ompClause{std::get<parser::OmpClauseList>(dir.t)};
2046-
if (dirName && endDirName &&
2047-
dirName->ToString().compare(endDirName->ToString())) {
2048-
context_
2049-
.Say(endDirName->source,
2050-
parser::MessageFormattedText{
2051-
"CRITICAL directive names do not match"_err_en_US})
2052-
.Attach(dirName->source, "should be "_en_US);
2053-
} else if (dirName && !endDirName) {
2054-
context_
2055-
.Say(dirName->source,
2056-
parser::MessageFormattedText{
2057-
"CRITICAL directive names do not match"_err_en_US})
2058-
.Attach(dirName->source, "should be NULL"_en_US);
2059-
} else if (!dirName && endDirName) {
2060-
context_
2061-
.Say(endDirName->source,
2062-
parser::MessageFormattedText{
2063-
"CRITICAL directive names do not match"_err_en_US})
2064-
.Attach(endDirName->source, "should be NULL"_en_US);
2065-
}
2066-
if (!dirName && !ompClause.source.empty() &&
2067-
ompClause.source.NULTerminatedToString() != "hint(omp_sync_hint_none)") {
2068-
context_.Say(dir.source,
2069-
parser::MessageFormattedText{
2070-
"Hint clause other than omp_sync_hint_none cannot be specified for "
2071-
"an unnamed CRITICAL directive"_err_en_US});
2034+
CheckNoBranching(
2035+
block, llvm::omp::Directive::OMPD_critical, beginSpec.DirName().source);
2036+
2037+
auto getNameFromArg{[](const parser::OmpArgument &arg) {
2038+
if (auto *object{parser::Unwrap<parser::OmpObject>(arg.u)}) {
2039+
if (auto *designator{omp::GetDesignatorFromObj(*object)}) {
2040+
return getDesignatorNameIfDataRef(*designator);
2041+
}
2042+
}
2043+
return static_cast<const parser::Name *>(nullptr);
2044+
}};
2045+
2046+
auto checkArgumentList{[&](const parser::OmpArgumentList &args) {
2047+
if (args.v.size() > 1) {
2048+
context_.Say(args.source,
2049+
"Only a single argument is allowed in CRITICAL directive"_err_en_US);
2050+
} else if (!args.v.empty()) {
2051+
if (!getNameFromArg(args.v.front())) {
2052+
context_.Say(args.v.front().source,
2053+
"CRITICAL argument should be a name"_err_en_US);
2054+
}
2055+
}
2056+
}};
2057+
2058+
const parser::Name *beginName{nullptr};
2059+
const parser::Name *endName{nullptr};
2060+
2061+
auto &beginArgs{beginSpec.Arguments()};
2062+
checkArgumentList(beginArgs);
2063+
2064+
if (!beginArgs.v.empty()) {
2065+
beginName = getNameFromArg(beginArgs.v.front());
2066+
}
2067+
2068+
if (endSpec) {
2069+
auto &endArgs{endSpec->Arguments()};
2070+
checkArgumentList(endArgs);
2071+
2072+
if (beginArgs.v.empty() != endArgs.v.empty()) {
2073+
parser::CharBlock source{
2074+
beginArgs.v.empty() ? endArgs.source : beginArgs.source};
2075+
context_.Say(source,
2076+
"Either both CRITICAL and END CRITICAL should have an argument, or none of them should"_err_en_US);
2077+
} else if (!beginArgs.v.empty()) {
2078+
endName = getNameFromArg(endArgs.v.front());
2079+
if (beginName && endName) {
2080+
if (beginName->ToString() != endName->ToString()) {
2081+
context_.Say(endName->source,
2082+
"The names on CRITICAL and END CRITICAL must match"_err_en_US);
2083+
}
2084+
}
2085+
}
2086+
}
2087+
2088+
for (auto &clause : beginSpec.Clauses().v) {
2089+
auto *hint{std::get_if<parser::OmpClause::Hint>(&clause.u)};
2090+
if (!hint) {
2091+
continue;
2092+
}
2093+
const int64_t OmpSyncHintNone = 0; // omp_sync_hint_none
2094+
std::optional<int64_t> hintValue{GetIntValue(hint->v.v)};
2095+
if (hintValue && *hintValue != OmpSyncHintNone) {
2096+
// Emit a diagnostic if the name is missing, and point to the directive
2097+
// with a missing name.
2098+
parser::CharBlock source;
2099+
if (!beginName) {
2100+
source = beginSpec.DirName().source;
2101+
} else if (endSpec && !endName) {
2102+
source = endSpec->DirName().source;
2103+
}
2104+
2105+
if (!source.empty()) {
2106+
context_.Say(source,
2107+
"When HINT other than 'omp_sync_hint_none' is present, CRITICAL directive should have a name"_err_en_US);
2108+
}
2109+
}
20722110
}
20732111
}
20742112

‎flang/lib/Semantics/openmp-utils.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
#include "openmp-utils.h"
13+
#include "flang/Semantics/openmp-utils.h"
1414

1515
#include "flang/Common/indirection.h"
1616
#include "flang/Common/reference.h"

0 commit comments

Comments
 (0)