[flang-commits] [flang] fe2d053 - Added OpenMP 5.0 specification based semantic checks for CRITICAL construct name resolution
via flang-commits
flang-commits at lists.llvm.org
Tue Oct 12 09:49:38 PDT 2021
Author: Nimish Mishra
Date: 2021-10-12T22:18:24+05:30
New Revision: fe2d053c4505b7ccc8a86e266e68d2f97aaca1e1
URL: https://github.com/llvm/llvm-project/commit/fe2d053c4505b7ccc8a86e266e68d2f97aaca1e1
DIFF: https://github.com/llvm/llvm-project/commit/fe2d053c4505b7ccc8a86e266e68d2f97aaca1e1.diff
LOG: Added OpenMP 5.0 specification based semantic checks for CRITICAL construct name resolution
As reported in https://bugs.llvm.org/show_bug.cgi?id=48145, name resolution for omp critical construct was failing. This patch adds functionality to help that name resolution as well as implementation to catch name mismatches.
The following semantic restrictions are therefore handled here:
- If a name is specified on a critical directive, the same name must also be specified on the end critical directive
- If no name appears on the critical directive, no name can appear on the end critical directive
- If a name appears on either the start critical directive or the end critical directive
Reviewed By: kiranchandramohan
Differential Revision: https://reviews.llvm.org/D110502
Added:
flang/test/Semantics/omp-sync-critical01.f90
flang/test/Semantics/omp-sync-critical02.f90
Modified:
flang/include/flang/Parser/parse-tree.h
flang/lib/Parser/openmp-parsers.cpp
flang/lib/Parser/unparse.cpp
flang/lib/Semantics/check-omp-structure.cpp
flang/lib/Semantics/resolve-directives.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 5ce6a110c1e65..6820a874483d0 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3594,7 +3594,7 @@ struct OpenMPDeclarativeConstruct {
struct OmpCriticalDirective {
TUPLE_CLASS_BOILERPLATE(OmpCriticalDirective);
CharBlock source;
- std::tuple<Verbatim, std::optional<Name>, std::optional<OmpClause>> t;
+ std::tuple<Verbatim, std::optional<Name>, OmpClauseList> t;
};
struct OmpEndCriticalDirective {
TUPLE_CLASS_BOILERPLATE(OmpEndCriticalDirective);
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 5585630b20e81..7e7adca6ef1b4 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -469,7 +469,7 @@ TYPE_PARSER(startOmpLine >>
verbatim("END CRITICAL"_tok), maybe(parenthesized(name)))) /
endOmpLine)
TYPE_PARSER(sourced(construct<OmpCriticalDirective>(verbatim("CRITICAL"_tok),
- maybe(parenthesized(name)), maybe(Parser<OmpClause>{}))) /
+ maybe(parenthesized(name)), Parser<OmpClauseList>{})) /
endOmpLine)
TYPE_PARSER(construct<OpenMPCriticalConstruct>(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index eaa4c926068c3..075719926e852 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2287,7 +2287,7 @@ class UnparseVisitor {
BeginOpenMP();
Word("!$OMP CRITICAL");
Walk(" (", std::get<std::optional<Name>>(x.t), ")");
- Walk(std::get<std::optional<OmpClause>>(x.t));
+ Walk(std::get<OmpClauseList>(x.t));
Put("\n");
EndOpenMP();
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 2c1b2913c5c23..c0924996ad477 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1024,9 +1024,39 @@ void OmpStructureChecker::Leave(const parser::OpenMPCancelConstruct &) {
void OmpStructureChecker::Enter(const parser::OpenMPCriticalConstruct &x) {
const auto &dir{std::get<parser::OmpCriticalDirective>(x.t)};
+ const auto &endDir{std::get<parser::OmpEndCriticalDirective>(x.t)};
PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_critical);
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);
+ }
+ 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});
+ }
}
void OmpStructureChecker::Leave(const parser::OpenMPCriticalConstruct &) {
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 6df35af6e78c2..3a3701459efb5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -293,6 +293,8 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
bool Pre(const parser::OpenMPBlockConstruct &);
void Post(const parser::OpenMPBlockConstruct &);
+ bool Pre(const parser::OmpCriticalDirective &x);
+ bool Pre(const parser::OmpEndCriticalDirective &x);
void Post(const parser::OmpBeginBlockDirective &) {
GetContext().withinConstruct = true;
@@ -476,7 +478,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
static constexpr Symbol::Flags ompFlagsRequireNewSymbol{
Symbol::Flag::OmpPrivate, Symbol::Flag::OmpLinear,
Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
- Symbol::Flag::OmpReduction};
+ Symbol::Flag::OmpReduction, Symbol::Flag::OmpCriticalLock};
static constexpr Symbol::Flags ompFlagsRequireMark{
Symbol::Flag::OmpThreadprivate};
@@ -1374,6 +1376,22 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPSectionsConstruct &x) {
return true;
}
+bool OmpAttributeVisitor::Pre(const parser::OmpCriticalDirective &x) {
+ const auto &name{std::get<std::optional<parser::Name>>(x.t)};
+ if (name) {
+ ResolveOmpName(*name, Symbol::Flag::OmpCriticalLock);
+ }
+ return true;
+}
+
+bool OmpAttributeVisitor::Pre(const parser::OmpEndCriticalDirective &x) {
+ const auto &name{std::get<std::optional<parser::Name>>(x.t)};
+ if (name) {
+ ResolveOmpName(*name, Symbol::Flag::OmpCriticalLock);
+ }
+ return true;
+}
+
bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
const auto &criticalDir{std::get<parser::OmpCriticalDirective>(x.t)};
PushContext(criticalDir.source, llvm::omp::Directive::OMPD_critical);
@@ -1497,6 +1515,13 @@ void OmpAttributeVisitor::ResolveOmpName(
AddToContextObjectWithDSA(*resolvedSymbol, ompFlag);
}
}
+ } else if (ompFlagsRequireNewSymbol.test(ompFlag)) {
+ const auto pair{GetContext().scope.try_emplace(
+ name.source, Attrs{}, ObjectEntityDetails{})};
+ CHECK(pair.second);
+ name.symbol = &pair.first->second.get();
+ } else {
+ DIE("OpenMP Name resolution failed");
}
}
diff --git a/flang/test/Semantics/omp-sync-critical01.f90 b/flang/test/Semantics/omp-sync-critical01.f90
new file mode 100644
index 0000000000000..7c80076c6fe2c
--- /dev/null
+++ b/flang/test/Semantics/omp-sync-critical01.f90
@@ -0,0 +1,41 @@
+! RUN: %python %S/test_errors.py %s %flang -fopenmp
+
+! OpenMP Version 5.0
+! 2.17.1 critical construct
+! CRITICAL start and end CRITICAL directive names mismatch
+integer function timer_tick_sec()
+ implicit none
+ integer t
+
+ !$OMP CRITICAL
+ t = t + 1
+ !$OMP END CRITICAL
+
+ !$OMP CRITICAL (foo)
+ t = t + 1
+ !$OMP END CRITICAL (foo)
+
+ !$OMP CRITICAL (foo)
+ t = t + 1
+ !ERROR: CRITICAL directive names do not match
+ !$OMP END CRITICAL (bar)
+
+ !$OMP CRITICAL (bar)
+ t = t + 1
+ !ERROR: CRITICAL directive names do not match
+ !$OMP END CRITICAL (foo)
+
+ !ERROR: CRITICAL directive names do not match
+ !$OMP CRITICAL (bar)
+ t = t + 1
+ !$OMP END CRITICAL
+
+ !$OMP CRITICAL
+ t = t + 1
+ !ERROR: CRITICAL directive names do not match
+ !$OMP END CRITICAL (foo)
+
+ timer_tick_sec = t
+ return
+
+end function timer_tick_sec
diff --git a/flang/test/Semantics/omp-sync-critical02.f90 b/flang/test/Semantics/omp-sync-critical02.f90
new file mode 100644
index 0000000000000..cf78a3c136351
--- /dev/null
+++ b/flang/test/Semantics/omp-sync-critical02.f90
@@ -0,0 +1,53 @@
+! RUN: %python %S/test_errors.py %s %flang -fopenmp
+
+! OpenMP Version 5.0
+! 2.17.1 critical construct
+! If the hint clause is specified, the critical construct must have a name.
+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
+ !$omp critical hint(omp_lock_hint_speculative)
+ j = j + 1
+ !$omp end critical
+
+ !$omp critical (foo) hint(omp_lock_hint_speculative)
+ i = i - 1
+ !$omp end critical (foo)
+
+ !ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
+ !$omp critical hint(omp_lock_hint_nonspeculative)
+ j = j + 1
+ !$omp end critical
+
+ !$omp critical (foo) hint(omp_lock_hint_nonspeculative)
+ i = i - 1
+ !$omp end critical (foo)
+
+ !ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
+ !$omp critical hint(omp_lock_hint_contended)
+ j = j + 1
+ !$omp end critical
+
+ !$omp critical (foo) hint(omp_lock_hint_contended)
+ i = i - 1
+ !$omp end critical (foo)
+
+ !ERROR: Hint clause other than omp_sync_hint_none cannot be specified for an unnamed CRITICAL directive
+ !$omp critical hint(omp_lock_hint_uncontended)
+ j = j + 1
+ !$omp end critical
+
+ !$omp critical (foo) hint(omp_lock_hint_uncontended)
+ i = i - 1
+ !$omp end critical (foo)
+
+ !$omp critical hint(omp_sync_hint_none)
+ j = j + 1
+ !$omp end critical
+
+ !$omp critical (foo) hint(omp_sync_hint_none)
+ i = i - 1
+ !$omp end critical (foo)
+
+end program sample
More information about the flang-commits
mailing list