shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (#152007)

Browse Source 
This allows not having the END CRITICAL directive in certain situations.
Update semantic checks and symbol resolution.
This commit is contained in:
Krzysztof Parzyszek 2025-08-07 08:10:25 -05:00 committed by GitHub
parent cfa00d4daf
commit e368b5343d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 153 additions and 132 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
flang/include/flang/Parser/parse-tree.h
View File

@ -4986,9 +4986,9 @@ struct OmpEndCriticalDirective {
CharBlock source;
std::tuple<Verbatim, std::optional<Name>> t;
};
struct OpenMPCriticalConstruct {
TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
std::tuple<OmpCriticalDirective, Block, OmpEndCriticalDirective> t;
struct OpenMPCriticalConstruct : public OmpBlockConstruct {
INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct, OmpBlockConstruct);
};
// 2.11.3 allocate -> ALLOCATE [(variable-name-list)] [clause]

0
flang/lib/Semantics/openmp-utils.h → flang/include/flang/Semantics/openmp-utils.h
View File

24
flang/lib/Lower/OpenMP/OpenMP.cpp
View File

@ -34,6 +34,7 @@
#include "flang/Parser/openmp-utils.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-directive-sets.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/tools.h"
#include "flang/Support/Flags.h"
#include "flang/Support/OpenMP-utils.h"
@ -3820,18 +3821,29 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPCriticalConstruct &criticalConstruct) {
const auto &cd = std::get<parser::OmpCriticalDirective>(criticalConstruct.t);
List<Clause> clauses =
makeClauses(std::get<parser::OmpClauseList>(cd.t), semaCtx);
const parser::OmpDirectiveSpecification &beginSpec =
criticalConstruct.BeginDir();
List<Clause> clauses = makeClauses(beginSpec.Clauses(), semaCtx);
ConstructQueue queue{buildConstructQueue(
converter.getFirOpBuilder().getModule(), semaCtx, eval, cd.source,
converter.getFirOpBuilder().getModule(), semaCtx, eval, beginSpec.source,
llvm::omp::Directive::OMPD_critical, clauses)};
const auto &name = std::get<std::optional<parser::Name>>(cd.t);
std::optional<parser::Name> critName;
const parser::OmpArgumentList &args = beginSpec.Arguments();
if (!args.v.empty()) {
// All of these things should be guaranteed to exist after semantic checks.
auto *object = parser::Unwrap<parser::OmpObject>(args.v.front());
assert(object && "Expecting object as argument");
auto *designator = semantics::omp::GetDesignatorFromObj(*object);
assert(designator && "Expecting desginator in argument");
auto *name = semantics::getDesignatorNameIfDataRef(*designator);
assert(name && "Expecting dataref in designator");
critName = *name;
}
mlir::Location currentLocation = converter.getCurrentLocation();
genCriticalOp(converter, symTable, semaCtx, eval, currentLocation, queue,
queue.begin(), name);
queue.begin(), critName);
}
static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,

11
flang/lib/Parser/openmp-parsers.cpp
View File

@ -1758,17 +1758,8 @@ TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
construct<OmpReductionCombiner>(Parser<FunctionReference>{}))
// 2.13.2 OMP CRITICAL
TYPE_PARSER(startOmpLine >>
sourced(construct<OmpEndCriticalDirective>(
verbatim("END CRITICAL"_tok), maybe(parenthesized(name)))) /
endOmpLine)
TYPE_PARSER(sourced(construct<OmpCriticalDirective>(verbatim("CRITICAL"_tok),
maybe(parenthesized(name)), Parser<OmpClauseList>{})) /
endOmpLine)
TYPE_PARSER(construct<OpenMPCriticalConstruct>(
Parser<OmpCriticalDirective>{}, block, Parser<OmpEndCriticalDirective>{}))
OmpBlockConstructParser{llvm::omp::Directive::OMPD_critical}))
// 2.11.3 Executable Allocate directive
TYPE_PARSER(

4
flang/lib/Parser/unparse.cpp
View File

@ -2606,9 +2606,7 @@ public:
EndOpenMP();
}
void Unparse(const OpenMPCriticalConstruct &x) {
Walk(std::get<OmpCriticalDirective>(x.t));
Walk(std::get<Block>(x.t), "");
Walk(std::get<OmpEndCriticalDirective>(x.t));
Unparse(static_cast<const OmpBlockConstruct &>(x));
}
void Unparse(const OmpDeclareTargetWithList &x) {
Put("("), Walk(x.v), Put(")");

2
flang/lib/Semantics/check-omp-atomic.cpp
View File

@ -11,13 +11,13 @@
//===----------------------------------------------------------------------===//
#include "check-omp-structure.h"
#include "openmp-utils.h"
#include "flang/Common/indirection.h"
#include "flang/Evaluate/expression.h"
#include "flang/Evaluate/tools.h"
#include "flang/Parser/char-block.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
#include "flang/Semantics/type.h"

2
flang/lib/Semantics/check-omp-loop.cpp
View File

@ -13,7 +13,6 @@
#include "check-omp-structure.h"
#include "check-directive-structure.h"
#include "openmp-utils.h"
#include "flang/Common/idioms.h"
#include "flang/Common/visit.h"
@ -23,6 +22,7 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Parser/tools.h"
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/semantics.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"

3
flang/lib/Semantics/check-omp-metadirective.cpp
View File

@ -12,8 +12,6 @@
#include "check-omp-structure.h"
#include "openmp-utils.h"
#include "flang/Common/idioms.h"
#include "flang/Common/indirection.h"
#include "flang/Common/visit.h"
@ -21,6 +19,7 @@
#include "flang/Parser/message.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/tools.h"
#include "llvm/Frontend/OpenMP/OMP.h"

122
flang/lib/Semantics/check-omp-structure.cpp
View File

@ -10,7 +10,6 @@
#include "check-directive-structure.h"
#include "definable.h"
#include "openmp-utils.h"
#include "resolve-names-utils.h"
#include "flang/Common/idioms.h"
@ -27,6 +26,7 @@
#include "flang/Semantics/expression.h"
#include "flang/Semantics/openmp-directive-sets.h"
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/scope.h"
#include "flang/Semantics/semantics.h"
#include "flang/Semantics/symbol.h"
@ -537,14 +537,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
checker_(x.v.source, Directive::OMPD_assume);
return false;
}
bool Pre(const parser::OmpCriticalDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_critical);
return false;
}
bool Pre(const parser::OmpEndCriticalDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_critical);
return false;
}
bool Pre(const parser::OmpMetadirectiveDirective &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_metadirective);
@ -2034,41 +2026,87 @@ void OmpStructureChecker::Leave(const parser::OpenMPCancelConstruct &) {
}
void OmpStructureChecker::Enter(const parser::OpenMPCriticalConstruct &x) {
const auto &dir{std::get<parser::OmpCriticalDirective>(x.t)};
const auto &dirSource{std::get<parser::Verbatim>(dir.t).source};
const auto &endDir{std::get<parser::OmpEndCriticalDirective>(x.t)};
PushContextAndClauseSets(dirSource, llvm::omp::Directive::OMPD_critical);
const parser::OmpBeginDirective &beginSpec{x.BeginDir()};
const std::optional<parser::OmpEndDirective> &endSpec{x.EndDir()};
PushContextAndClauseSets(beginSpec.DirName().source, beginSpec.DirName().v);
const auto &block{std::get<parser::Block>(x.t)};
CheckNoBranching(block, llvm::omp::Directive::OMPD_critical, dir.source);
const auto &dirName{std::get<std::optional<parser::Name>>(dir.t)};
const auto &endDirName{std::get<std::optional<parser::Name>>(endDir.t)};
const auto &ompClause{std::get<parser::OmpClauseList>(dir.t)};
if (dirName && endDirName &&
dirName->ToString().compare(endDirName->ToString())) {
context_
.Say(endDirName->source,
parser::MessageFormattedText{
"CRITICAL directive names do not match"_err_en_US})
.Attach(dirName->source, "should be "_en_US);
} else if (dirName && !endDirName) {
context_
.Say(dirName->source,
parser::MessageFormattedText{
"CRITICAL directive names do not match"_err_en_US})
.Attach(dirName->source, "should be NULL"_en_US);
} else if (!dirName && endDirName) {
context_
.Say(endDirName->source,
parser::MessageFormattedText{
"CRITICAL directive names do not match"_err_en_US})
.Attach(endDirName->source, "should be NULL"_en_US);
CheckNoBranching(
block, llvm::omp::Directive::OMPD_critical, beginSpec.DirName().source);
auto getNameFromArg{[](const parser::OmpArgument &arg) {
if (auto *object{parser::Unwrap<parser::OmpObject>(arg.u)}) {
if (auto *designator{omp::GetDesignatorFromObj(*object)}) {
return getDesignatorNameIfDataRef(*designator);
}
}
return static_cast<const parser::Name *>(nullptr);
}};
auto checkArgumentList{[&](const parser::OmpArgumentList &args) {
if (args.v.size() > 1) {
context_.Say(args.source,
"Only a single argument is allowed in CRITICAL directive"_err_en_US);
} else if (!args.v.empty()) {
if (!getNameFromArg(args.v.front())) {
context_.Say(args.v.front().source,
"CRITICAL argument should be a name"_err_en_US);
}
}
}};
const parser::Name *beginName{nullptr};
const parser::Name *endName{nullptr};
auto &beginArgs{beginSpec.Arguments()};
checkArgumentList(beginArgs);
if (!beginArgs.v.empty()) {
beginName = getNameFromArg(beginArgs.v.front());
}
if (!dirName && !ompClause.source.empty() &&
ompClause.source.NULTerminatedToString() != "hint(omp_sync_hint_none)") {
context_.Say(dir.source,
parser::MessageFormattedText{
"Hint clause other than omp_sync_hint_none cannot be specified for "
"an unnamed CRITICAL directive"_err_en_US});
if (endSpec) {
auto &endArgs{endSpec->Arguments()};
checkArgumentList(endArgs);
if (beginArgs.v.empty() != endArgs.v.empty()) {
parser::CharBlock source{
beginArgs.v.empty() ? endArgs.source : beginArgs.source};
context_.Say(source,
"Either both CRITICAL and END CRITICAL should have an argument, or none of them should"_err_en_US);
} else if (!beginArgs.v.empty()) {
endName = getNameFromArg(endArgs.v.front());
if (beginName && endName) {
if (beginName->ToString() != endName->ToString()) {
context_.Say(endName->source,
"The names on CRITICAL and END CRITICAL must match"_err_en_US);
}
}
}
}
for (auto &clause : beginSpec.Clauses().v) {
auto *hint{std::get_if<parser::OmpClause::Hint>(&clause.u)};
if (!hint) {
continue;
}
const int64_t OmpSyncHintNone = 0; // omp_sync_hint_none
std::optional<int64_t> hintValue{GetIntValue(hint->v.v)};
if (hintValue && *hintValue != OmpSyncHintNone) {
// Emit a diagnostic if the name is missing, and point to the directive
// with a missing name.
parser::CharBlock source;
if (!beginName) {
source = beginSpec.DirName().source;
} else if (endSpec && !endName) {
source = endSpec->DirName().source;
}
if (!source.empty()) {
context_.Say(source,
"When HINT other than 'omp_sync_hint_none' is present, CRITICAL directive should have a name"_err_en_US);
}
}
}
}

2
flang/lib/Semantics/openmp-utils.cpp
View File

@ -10,7 +10,7 @@
//
//===----------------------------------------------------------------------===//
#include "openmp-utils.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Common/indirection.h"
#include "flang/Common/reference.h"

6
flang/lib/Semantics/resolve-directives.cpp
View File

@ -10,7 +10,6 @@
#include "check-acc-structure.h"
#include "check-omp-structure.h"
#include "openmp-utils.h"
#include "resolve-names-utils.h"
#include "flang/Common/idioms.h"
#include "flang/Evaluate/fold.h"
@ -22,6 +21,7 @@
#include "flang/Semantics/expression.h"
#include "flang/Semantics/openmp-dsa.h"
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/symbol.h"
#include "flang/Semantics/tools.h"
#include "flang/Support/Flags.h"
@ -2139,8 +2139,8 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPSectionConstruct &x) {
}
bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
const auto &beginCriticalDir{std::get<parser::OmpCriticalDirective>(x.t)};
PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
const parser::OmpBeginDirective &beginSpec{x.BeginDir()};
PushContext(beginSpec.DirName().source, beginSpec.DirName().v);
GetContext().withinConstruct = true;
return true;
}

69
flang/lib/Semantics/resolve-names.cpp
View File

@ -30,6 +30,7 @@
#include "flang/Semantics/attr.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/openmp-utils.h"
#include "flang/Semantics/program-tree.h"
#include "flang/Semantics/scope.h"
#include "flang/Semantics/semantics.h"
@ -1486,6 +1487,16 @@ public:
void Post(const parser::OpenMPBlockConstruct &);
bool Pre(const parser::OmpBeginDirective &x) {
AddOmpSourceRange(x.source);
// Manually resolve names in CRITICAL directives. This is because these
// names do not denote Fortran objects, and the CRITICAL directive causes
// them to be "auto-declared", i.e. inserted into the global scope.
// More specifically, they are not expected to have explicit declarations,
// and if they do the behavior is unspeficied.
if (x.DirName().v == llvm::omp::Directive::OMPD_critical) {
for (const parser::OmpArgument &arg : x.Arguments().v) {
ResolveCriticalName(arg);
}
}
return true;
}
void Post(const parser::OmpBeginDirective &) {
@ -1493,6 +1504,12 @@ public:
}
bool Pre(const parser::OmpEndDirective &x) {
AddOmpSourceRange(x.source);
// Manually resolve names in CRITICAL directives.
if (x.DirName().v == llvm::omp::Directive::OMPD_critical) {
for (const parser::OmpArgument &arg : x.Arguments().v) {
ResolveCriticalName(arg);
}
}
return true;
}
void Post(const parser::OmpEndDirective &) {
@ -1591,32 +1608,6 @@ public:
void Post(const parser::OmpEndSectionsDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
bool Pre(const parser::OmpCriticalDirective &x) {
AddOmpSourceRange(x.source);
// Manually resolve names in CRITICAL directives. This is because these
// names do not denote Fortran objects, and the CRITICAL directive causes
// them to be "auto-declared", i.e. inserted into the global scope.
// More specifically, they are not expected to have explicit declarations,
// and if they do the behavior is unspeficied.
if (auto &maybeName{std::get<std::optional<parser::Name>>(x.t)}) {
ResolveCriticalName(*maybeName);
}
return true;
}
void Post(const parser::OmpCriticalDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
bool Pre(const parser::OmpEndCriticalDirective &x) {
AddOmpSourceRange(x.source);
// Manually resolve names in CRITICAL directives.
if (auto &maybeName{std::get<std::optional<parser::Name>>(x.t)}) {
ResolveCriticalName(*maybeName);
}
return true;
}
void Post(const parser::OmpEndCriticalDirective &) {
messageHandler().set_currStmtSource(std::nullopt);
}
bool Pre(const parser::OpenMPThreadprivate &) {
SkipImplicitTyping(true);
return true;
@ -1732,7 +1723,7 @@ private:
const std::optional<parser::OmpClauseList> &clauses,
const T &wholeConstruct);
void ResolveCriticalName(const parser::Name &name);
void ResolveCriticalName(const parser::OmpArgument &arg);
int metaLevel_{0};
const parser::OmpMetadirectiveDirective *metaDirective_{nullptr};
@ -1961,7 +1952,7 @@ void OmpVisitor::ProcessReductionSpecifier(
}
}
void OmpVisitor::ResolveCriticalName(const parser::Name &name) {
void OmpVisitor::ResolveCriticalName(const parser::OmpArgument &arg) {
auto &globalScope{[&]() -> Scope & {
for (Scope *s{&currScope()};; s = &s->parent()) {
if (s->IsTopLevel()) {
@ -1971,15 +1962,21 @@ void OmpVisitor::ResolveCriticalName(const parser::Name &name) {
llvm_unreachable("Cannot find global scope");
}()};
if (auto *symbol{FindInScope(globalScope, name)}) {
if (!symbol->test(Symbol::Flag::OmpCriticalLock)) {
SayWithDecl(name, *symbol,
"CRITICAL construct name '%s' conflicts with a previous declaration"_warn_en_US,
name.ToString());
if (auto *object{parser::Unwrap<parser::OmpObject>(arg.u)}) {
if (auto *desg{omp::GetDesignatorFromObj(*object)}) {
if (auto *name{getDesignatorNameIfDataRef(*desg)}) {
if (auto *symbol{FindInScope(globalScope, *name)}) {
if (!symbol->test(Symbol::Flag::OmpCriticalLock)) {
SayWithDecl(*name, *symbol,
"CRITICAL construct name '%s' conflicts with a previous declaration"_warn_en_US,
name->ToString());
}
} else {
name->symbol = &MakeSymbol(globalScope, name->source, Attrs{});
name->symbol->set(Symbol::Flag::OmpCriticalLock);
}
}
}
} else {
name.symbol = &MakeSymbol(globalScope, name.source, Attrs{});
name.symbol->set(Symbol::Flag::OmpCriticalLock);
}
}

14
flang/lib/Semantics/unparse-with-symbols.cpp
View File

@ -70,20 +70,6 @@ public:
currStmt_ = std::nullopt;
}
bool Pre(const parser::OmpCriticalDirective &x) {
currStmt_ = x.source;
return true;
}
void Post(const parser::OmpCriticalDirective &) { currStmt_ = std::nullopt; }
bool Pre(const parser::OmpEndCriticalDirective &x) {
currStmt_ = x.source;
return true;
}
void Post(const parser::OmpEndCriticalDirective &) {
currStmt_ = std::nullopt;
}
// Directive arguments can be objects with symbols.
bool Pre(const parser::OmpBeginDirective &x) {
currStmt_ = x.source;

4
flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90
View File

@ -13,9 +13,9 @@ end
!UNPARSE: implicit none
!UNPARSE: !DEF: /f/x ObjectEntity INTEGER(4)
!UNPARSE: integer x
!UNPARSE: !$omp critical (c)
!UNPARSE: !$omp critical(c)
!UNPARSE: !REF: /f/x
!UNPARSE: x = 0
!UNPARSE: !$omp end critical (c)
!UNPARSE: !$omp end critical(c)
!UNPARSE: end subroutine

8
flang/test/Semantics/OpenMP/sync-critical01.f90
View File

@ -17,22 +17,22 @@ integer function timer_tick_sec()
!$OMP CRITICAL (foo)
t = t + 1
!ERROR: CRITICAL directive names do not match
!ERROR: The names on CRITICAL and END CRITICAL must match
!$OMP END CRITICAL (bar)
!$OMP CRITICAL (bar)
t = t + 1
!ERROR: CRITICAL directive names do not match
!ERROR: The names on CRITICAL and END CRITICAL must match
!$OMP END CRITICAL (foo)
!ERROR: CRITICAL directive names do not match
!ERROR: Either both CRITICAL and END CRITICAL should have an argument, or none of them should
!$OMP CRITICAL (bar)
t = t + 1
!$OMP END CRITICAL
!$OMP CRITICAL
t = t + 1
!ERROR: CRITICAL directive names do not match
!ERROR: Either both CRITICAL and END CRITICAL should have an argument, or none of them should
!$OMP END CRITICAL (foo)
timer_tick_sec = t

8
flang/test/Semantics/OpenMP/sync-critical02.f90
View File

@ -8,7 +8,7 @@
program sample
use omp_lib
integer i, j
!ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
!ERROR: When HINT other than 'omp_sync_hint_none' is present, CRITICAL directive should have a name
!$omp critical hint(omp_lock_hint_speculative)
j = j + 1
!$omp end critical
@ -17,7 +17,7 @@ program sample
i = i - 1
!$omp end critical (foo)
!ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
!ERROR: When HINT other than 'omp_sync_hint_none' is present, CRITICAL directive should have a name
!$omp critical hint(omp_lock_hint_nonspeculative)
j = j + 1
!$omp end critical
@ -26,7 +26,7 @@ program sample
i = i - 1
!$omp end critical (foo)
!ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
!ERROR: When HINT other than 'omp_sync_hint_none' is present, CRITICAL directive should have a name
!$omp critical hint(omp_lock_hint_contended)
j = j + 1
!$omp end critical
@ -35,7 +35,7 @@ program sample
i = i - 1
!$omp end critical (foo)
!ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
!ERROR: When HINT other than 'omp_sync_hint_none' is present, CRITICAL directive should have a name
!$omp critical hint(omp_lock_hint_uncontended)
j = j + 1
!$omp end critical