[flang-commits] [flang] 1df8fa7 - [Flang][OpenMP][NFC][2/2] Reorder OmpStructureChecker and simplify it.
Sameeran joshi via flang-commits
flang-commits at lists.llvm.org
Sat Nov 21 09:08:04 PST 2020
Author: sameeran joshi
Date: 2020-11-21T22:37:35+05:30
New Revision: 1df8fa78e652d112c83d21096e5f2750f70f5b66
URL: https://github.com/llvm/llvm-project/commit/1df8fa78e652d112c83d21096e5f2750f70f5b66
DIFF: https://github.com/llvm/llvm-project/commit/1df8fa78e652d112c83d21096e5f2750f70f5b66.diff
LOG: [Flang][OpenMP][NFC][2/2] Reorder OmpStructureChecker and simplify it.
`OmpStructureChecker` has too much boilerplate code in source file.
This patch:
1. Use helpers from `check-directive-structure.h` and reduces the boilerplate.
2. Use TableGen infrastructure as much as possible.
Reviewed By: kiranchandramohan
Differential Revision: https://reviews.llvm.org/D90834
Added:
Modified:
flang/lib/Semantics/check-omp-structure.cpp
flang/lib/Semantics/check-omp-structure.h
flang/test/Semantics/omp-clause-validity01.f90
flang/test/Semantics/omp-combined-constructs.f90
flang/test/Semantics/omp-device-constructs.f90
llvm/include/llvm/Frontend/OpenMP/OMP.td
Removed:
################################################################################
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 93787672b4440..3cf77137ec12e 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -9,6 +9,7 @@
#include "check-omp-structure.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/tools.h"
+#include <algorithm>
namespace Fortran::semantics {
@@ -160,8 +161,8 @@ void OmpStructureChecker::Enter(const parser::OmpEndSectionsDirective &x) {
switch (dir.v) {
// 2.7.2 end-sections -> END SECTIONS [nowait-clause]
case llvm::omp::Directive::OMPD_sections:
- SetContextDirectiveEnum(llvm::omp::Directive::OMPD_end_sections);
- SetContextAllowedOnce(OmpClauseSet{llvm::omp::Clause::OMPC_nowait});
+ PushContextAndClauseSets(
+ dir.source, llvm::omp::Directive::OMPD_end_sections);
break;
default:
// no clauses are allowed
@@ -183,8 +184,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPDeclareTargetConstruct &x) {
PushContext(dir.source, llvm::omp::Directive::OMPD_declare_target);
const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
if (std::holds_alternative<parser::OmpDeclareTargetWithClause>(spec.u)) {
- SetContextAllowed(
- OmpClauseSet{llvm::omp::Clause::OMPC_to, llvm::omp::Clause::OMPC_link});
+ SetClauseSets(llvm::omp::Directive::OMPD_declare_target);
}
}
@@ -248,17 +248,13 @@ void OmpStructureChecker::Enter(const parser::OmpEndBlockDirective &x) {
switch (dir.v) {
// 2.7.3 end-single-clause -> copyprivate-clause |
// nowait-clause
- case llvm::omp::Directive::OMPD_single: {
- SetContextDirectiveEnum(llvm::omp::Directive::OMPD_end_single);
- OmpClauseSet allowed{llvm::omp::Clause::OMPC_copyprivate};
- SetContextAllowed(allowed);
- OmpClauseSet allowedOnce{llvm::omp::Clause::OMPC_nowait};
- SetContextAllowedOnce(allowedOnce);
- } break;
+ case llvm::omp::Directive::OMPD_single:
+ PushContextAndClauseSets(dir.source, llvm::omp::Directive::OMPD_end_single);
+ break;
// 2.7.4 end-workshare -> END WORKSHARE [nowait-clause]
case llvm::omp::Directive::OMPD_workshare:
- SetContextDirectiveEnum(llvm::omp::Directive::OMPD_end_workshare);
- SetContextAllowed(OmpClauseSet{llvm::omp::Clause::OMPC_nowait});
+ PushContextAndClauseSets(
+ dir.source, llvm::omp::Directive::OMPD_end_workshare);
break;
default:
// no clauses are allowed
@@ -300,11 +296,8 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
std::get<parser::OmpClause::Ordered>(clause->u)};
if (orderedClause.v) {
- if (FindClause(llvm::omp::Clause::OMPC_linear)) {
- context_.Say(clause->source,
- "A loop directive may not have both a LINEAR clause and "
- "an ORDERED clause with a parameter"_err_en_US);
- }
+ CheckNotAllowedIfClause(
+ llvm::omp::Clause::OMPC_ordered, {llvm::omp::Clause::OMPC_linear});
if (auto *clause2{FindClause(llvm::omp::Clause::OMPC_collapse)}) {
const auto &collapseClause{
@@ -347,19 +340,13 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
}
}
}
-
// TODO: A list-item cannot appear in more than one aligned clause
} // SIMD
// 2.7.3 Single Construct Restriction
if (GetContext().directive == llvm::omp::Directive::OMPD_end_single) {
- if (auto *clause{FindClause(llvm::omp::Clause::OMPC_copyprivate)}) {
- if (FindClause(llvm::omp::Clause::OMPC_nowait)) {
- context_.Say(clause->source,
- "The COPYPRIVATE clause must not be used with "
- "the NOWAIT clause"_err_en_US);
- }
- }
+ CheckNotAllowedIfClause(
+ llvm::omp::Clause::OMPC_copyprivate, {llvm::omp::Clause::OMPC_nowait});
}
GetContext().requiredClauses.IterateOverMembers(
@@ -410,7 +397,6 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Ordered &x) {
// the parameter of ordered clause is optional
if (const auto &expr{x.v}) {
RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_ordered, *expr);
-
// 2.8.3 Loop SIMD Construct Restriction
if (llvm::omp::doSimdSet.test(GetContext().directive)) {
context_.Say(GetContext().clauseSource,
@@ -436,13 +422,7 @@ void OmpStructureChecker::Enter(const parser::OmpAlignedClause &x) {
if (const auto &expr{
std::get<std::optional<parser::ScalarIntConstantExpr>>(x.t)}) {
- if (const auto v{GetIntValue(*expr)}) {
- if (*v <= 0) {
- context_.Say(GetContext().clauseSource,
- "The ALIGNMENT parameter of the ALIGNED clause must be "
- "a constant positive integer expression"_err_en_US);
- }
- }
+ RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_aligned, *expr);
}
// 2.8.1 TODO: list-item attribute check
}
@@ -501,6 +481,28 @@ void OmpStructureChecker::Enter(const parser::OmpLinearClause &x) {
}
}
}
+
+void OmpStructureChecker::CheckAllowedMapTypes(
+ const parser::OmpMapType::Type &type,
+ const std::list<parser::OmpMapType::Type> &allowedMapTypeList) {
+ const auto found{std::find(
+ std::begin(allowedMapTypeList), std::end(allowedMapTypeList), type)};
+ if (found == std::end(allowedMapTypeList)) {
+ std::string commaSeperatedMapTypes;
+ llvm::interleave(
+ allowedMapTypeList.begin(), allowedMapTypeList.end(),
+ [&](const parser::OmpMapType::Type &mapType) {
+ commaSeperatedMapTypes.append(parser::ToUpperCaseLetters(
+ parser::OmpMapType::EnumToString(mapType)));
+ },
+ [&] { commaSeperatedMapTypes.append(", "); });
+ context_.Say(GetContext().clauseSource,
+ "Only the %s map types are permitted "
+ "for MAP clauses on the %s directive"_err_en_US,
+ commaSeperatedMapTypes, ContextDirectiveAsFortran());
+ }
+}
+
void OmpStructureChecker::Enter(const parser::OmpMapClause &x) {
CheckAllowed(llvm::omp::Clause::OMPC_map);
if (const auto &maptype{std::get<std::optional<parser::OmpMapType>>(x.t)}) {
@@ -513,31 +515,16 @@ void OmpStructureChecker::Enter(const parser::OmpMapClause &x) {
case llvm::omp::Directive::OMPD_target_teams_distribute_simd:
case llvm::omp::Directive::OMPD_target_teams_distribute_parallel_do:
case llvm::omp::Directive::OMPD_target_teams_distribute_parallel_do_simd:
- case llvm::omp::Directive::OMPD_target_data: {
- if (type != Type::To && type != Type::From && type != Type::Tofrom &&
- type != Type::Alloc) {
- context_.Say(GetContext().clauseSource,
- "Only the TO, FROM, TOFROM, or ALLOC map types are permitted "
- "for MAP clauses on the %s directive"_err_en_US,
- ContextDirectiveAsFortran());
- }
- } break;
- case llvm::omp::Directive::OMPD_target_enter_data: {
- if (type != Type::To && type != Type::Alloc) {
- context_.Say(GetContext().clauseSource,
- "Only the TO or ALLOC map types are permitted "
- "for MAP clauses on the %s directive"_err_en_US,
- ContextDirectiveAsFortran());
- }
- } break;
- case llvm::omp::Directive::OMPD_target_exit_data: {
- if (type != Type::Delete && type != Type::Release && type != Type::From) {
- context_.Say(GetContext().clauseSource,
- "Only the FROM, RELEASE, or DELETE map types are permitted "
- "for MAP clauses on the %s directive"_err_en_US,
- ContextDirectiveAsFortran());
- }
- } break;
+ case llvm::omp::Directive::OMPD_target_data:
+ CheckAllowedMapTypes(
+ type, {Type::To, Type::From, Type::Tofrom, Type::Alloc});
+ break;
+ case llvm::omp::Directive::OMPD_target_enter_data:
+ CheckAllowedMapTypes(type, {Type::To, Type::Alloc});
+ break;
+ case llvm::omp::Directive::OMPD_target_exit_data:
+ CheckAllowedMapTypes(type, {Type::From, Type::Release, Type::Delete});
+ break;
default:
break;
}
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index fbb319f30f9f9..cf10608efa725 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -171,7 +171,8 @@ class OmpStructureChecker
// specific clause related
bool ScheduleModifierHasType(const parser::OmpScheduleClause &,
const parser::OmpScheduleModifierType::ModType &);
-
+ void CheckAllowedMapTypes(const parser::OmpMapType::Type &,
+ const std::list<parser::OmpMapType::Type> &);
llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
diff --git a/flang/test/Semantics/omp-clause-validity01.f90 b/flang/test/Semantics/omp-clause-validity01.f90
index aa51dfd9dcbb0..5ff748ec33c6d 100644
--- a/flang/test/Semantics/omp-clause-validity01.f90
+++ b/flang/test/Semantics/omp-clause-validity01.f90
@@ -214,8 +214,9 @@
a = 3.14
enddo
+ !ERROR: Clause LINEAR is not allowed if clause ORDERED appears on the DO directive
+ !ERROR: Clause LINEAR is not allowed if clause ORDERED appears on the DO directive
!ERROR: The parameter of the ORDERED clause must be a constant positive integer expression
- !ERROR: A loop directive may not have both a LINEAR clause and an ORDERED clause with a parameter
!$omp do ordered(1-1) private(b) linear(b) linear(a)
do i = 1, N
a = 3.14
@@ -319,7 +320,7 @@
!ERROR: LASTPRIVATE clause is not allowed on the SINGLE directive
!$omp single private(a) lastprivate(c)
a = 3.14
- !ERROR: The COPYPRIVATE clause must not be used with the NOWAIT clause
+ !ERROR: Clause NOWAIT is not allowed if clause COPYPRIVATE appears on the END SINGLE directive
!ERROR: At most one NOWAIT clause can appear on the END SINGLE directive
!$omp end single copyprivate(a) nowait nowait
c = 2
@@ -365,7 +366,7 @@
a = 3.14
enddo
- !ERROR: The ALIGNMENT parameter of the ALIGNED clause must be a constant positive integer expression
+ !ERROR: The parameter of the ALIGNED clause must be a constant positive integer expression
!$omp simd aligned(b:-2)
do i = 1, N
a = 3.14
@@ -552,7 +553,7 @@
enddo
!ERROR: The parameter of the SIMDLEN clause must be a constant positive integer expression
- !ERROR: The ALIGNMENT parameter of the ALIGNED clause must be a constant positive integer expression
+ !ERROR: The parameter of the ALIGNED clause must be a constant positive integer expression
!$omp taskloop simd simdlen(-1) aligned(a:-2)
do i = 1, N
a = 3.14
diff --git a/flang/test/Semantics/omp-combined-constructs.f90 b/flang/test/Semantics/omp-combined-constructs.f90
index 78821cf0e06cb..afb70b7eda66f 100644
--- a/flang/test/Semantics/omp-combined-constructs.f90
+++ b/flang/test/Semantics/omp-combined-constructs.f90
@@ -205,7 +205,7 @@ program main
enddo
!$omp end target teams
- !ERROR: Only the TO, FROM, TOFROM, or ALLOC map types are permitted for MAP clauses on the TARGET TEAMS directive
+ !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS directive
!$omp target teams map(delete:a)
do i = 1, N
a(i) = 3.14
@@ -305,7 +305,7 @@ program main
enddo
!$omp end target teams distribute
- !ERROR: Only the TO, FROM, TOFROM, or ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE directive
+ !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE directive
!$omp target teams distribute map(delete:a)
do i = 1, N
a(i) = 3.14
@@ -398,7 +398,7 @@ program main
enddo
!$omp end target teams distribute parallel do
- !ERROR: Only the TO, FROM, TOFROM, or ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO directive
+ !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO directive
!$omp target teams distribute parallel do map(delete:a)
do i = 1, N
a(i) = 3.14
@@ -498,7 +498,7 @@ program main
enddo
!$omp end target teams distribute parallel do simd
- !ERROR: Only the TO, FROM, TOFROM, or ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD directive
+ !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD directive
!$omp target teams distribute parallel do simd map(delete:a)
do i = 1, N
a(i) = 3.14
diff --git a/flang/test/Semantics/omp-device-constructs.f90 b/flang/test/Semantics/omp-device-constructs.f90
index 7b3eef22524f0..8386744571d2f 100644
--- a/flang/test/Semantics/omp-device-constructs.f90
+++ b/flang/test/Semantics/omp-device-constructs.f90
@@ -109,7 +109,7 @@ program main
enddo
!$omp end target
- !ERROR: Only the TO, FROM, TOFROM, or ALLOC map types are permitted for MAP clauses on the TARGET directive
+ !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET directive
!$omp target map(delete:a)
do i = 1, N
a = 3.14
@@ -132,7 +132,7 @@ program main
!ERROR: At most one IF clause can appear on the TARGET ENTER DATA directive
!$omp target enter data map(to:a) if(.true.) if(.false.)
- !ERROR: Only the TO or ALLOC map types are permitted for MAP clauses on the TARGET ENTER DATA directive
+ !ERROR: Only the TO, ALLOC map types are permitted for MAP clauses on the TARGET ENTER DATA directive
!$omp target enter data map(from:a)
!$omp target exit data map(delete:a)
@@ -140,7 +140,7 @@ program main
!ERROR: At most one DEVICE clause can appear on the TARGET EXIT DATA directive
!$omp target exit data map(from:a) device(0) device(1)
- !ERROR: Only the FROM, RELEASE, or DELETE map types are permitted for MAP clauses on the TARGET EXIT DATA directive
+ !ERROR: Only the FROM, RELEASE, DELETE map types are permitted for MAP clauses on the TARGET EXIT DATA directive
!$omp target exit data map(to:a)
!$omp target
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index e735176975475..3a93dd73edeb0 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -895,7 +895,12 @@ def OMP_Distribute : Directive<"distribute"> {
VersionedClause<OMPC_DistSchedule>
];
}
-def OMP_DeclareTarget : Directive<"declare target"> {}
+def OMP_DeclareTarget : Directive<"declare target"> {
+ let allowedClauses = [
+ VersionedClause<OMPC_To>,
+ VersionedClause<OMPC_Link>
+ ];
+}
def OMP_EndDeclareTarget : Directive<"end declare target"> {}
def OMP_DistributeParallelFor : Directive<"distribute parallel for"> {
let allowedClauses = [
@@ -1606,9 +1611,24 @@ def OMP_ParallelWorkshare : Directive<"parallel workshare"> {
def OMP_Workshare : Directive<"workshare"> {}
def OMP_EndDo : Directive<"end do"> {}
def OMP_EndDoSimd : Directive<"end do simd"> {}
-def OMP_EndSections : Directive<"end sections"> {}
-def OMP_EndSingle : Directive<"end single"> {}
-def OMP_EndWorkshare : Directive<"end workshare"> {}
+def OMP_EndSections : Directive<"end sections"> {
+ let allowedOnceClauses = [
+ VersionedClause<OMPC_NoWait>
+ ];
+}
+def OMP_EndSingle : Directive<"end single"> {
+ let allowedClauses = [
+ VersionedClause<OMPC_CopyPrivate>
+ ];
+ let allowedOnceClauses = [
+ VersionedClause<OMPC_NoWait>
+ ];
+}
+def OMP_EndWorkshare : Directive<"end workshare"> {
+ let allowedClauses = [
+ VersionedClause<OMPC_NoWait>
+ ];
+}
def OMP_Unknown : Directive<"unknown"> {
let isDefault = true;
}
More information about the flang-commits
mailing list