[flang-commits] [flang] 09a4bcf - [flang][OpenMP] Update handling of DEPEND clause (#113620)
via flang-commits
flang-commits at lists.llvm.org
Mon Oct 28 14:06:26 PDT 2024
Author: Krzysztof Parzyszek
Date: 2024-10-28T16:06:22-05:00
New Revision: 09a4bcf1a549eea738bda74b2b7dc0f5c8309310
URL: https://github.com/llvm/llvm-project/commit/09a4bcf1a549eea738bda74b2b7dc0f5c8309310
DIFF: https://github.com/llvm/llvm-project/commit/09a4bcf1a549eea738bda74b2b7dc0f5c8309310.diff
LOG: [flang][OpenMP] Update handling of DEPEND clause (#113620)
Parse the locator list in OmpDependClause as an OmpObjectList (instead
of a list of Designators). When a common block appears in the locator
list, show an informative message.
Implement resolving symbols in DependSinkVec in a dedicated visitor
instead of having a visitor for OmpDependClause.
Resolve unresolved names common blocks in OmpObjectList.
Minor changes to the code organization:
- rename OmpDependenceType to OmpTaskDependenceType (to follow 5.2
terminology),
- rename Depend::WithLocators to Depend::DepType,
- add comments with more detailed spec references to parse-tree.h.
---------
Co-authored-by: Kiran Chandramohan <kiran.chandramohan at arm.com>
Added:
flang/test/Semantics/OpenMP/depend04.f90
Modified:
flang/examples/FeatureList/FeatureList.cpp
flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
flang/include/flang/Parser/dump-parse-tree.h
flang/include/flang/Parser/parse-tree.h
flang/lib/Lower/OpenMP/ClauseProcessor.cpp
flang/lib/Lower/OpenMP/Clauses.cpp
flang/lib/Parser/openmp-parsers.cpp
flang/lib/Parser/unparse.cpp
flang/lib/Semantics/check-omp-structure.cpp
flang/lib/Semantics/resolve-directives.cpp
llvm/include/llvm/Frontend/OpenMP/ClauseT.h
Removed:
################################################################################
diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 9fce67e61ed30f..62f8d39a8abaa5 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -473,8 +473,8 @@ struct NodeVisitor {
READ_FEATURE(OmpDependClause::InOut)
READ_FEATURE(OmpDependClause::Sink)
READ_FEATURE(OmpDependClause::Source)
- READ_FEATURE(OmpDependenceType)
- READ_FEATURE(OmpDependenceType::Type)
+ READ_FEATURE(OmpTaskDependenceType)
+ READ_FEATURE(OmpTaskDependenceType::Type)
READ_FEATURE(OmpDependSinkVec)
READ_FEATURE(OmpDependSinkVecLength)
READ_FEATURE(OmpEndAllocators)
diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
index 5d3c5cd72eef04..d28ed0534d6002 100644
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
@@ -222,9 +222,9 @@ void OpenMPCounterVisitor::Post(const OmpLinearModifier::Type &c) {
clauseDetails +=
"modifier=" + std::string{OmpLinearModifier::EnumToString(c)} + ";";
}
-void OpenMPCounterVisitor::Post(const OmpDependenceType::Type &c) {
+void OpenMPCounterVisitor::Post(const OmpTaskDependenceType::Type &c) {
clauseDetails +=
- "type=" + std::string{OmpDependenceType::EnumToString(c)} + ";";
+ "type=" + std::string{OmpTaskDependenceType::EnumToString(c)} + ";";
}
void OpenMPCounterVisitor::Post(const OmpMapClause::Type &c) {
clauseDetails += "type=" + std::string{OmpMapClause::EnumToString(c)} + ";";
diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
index 380534ebbfd70a..68c52db46e2f00 100644
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
@@ -73,7 +73,7 @@ struct OpenMPCounterVisitor {
void Post(const OmpDeviceTypeClause::Type &c);
void Post(const OmpScheduleModifierType::ModType &c);
void Post(const OmpLinearModifier::Type &c);
- void Post(const OmpDependenceType::Type &c);
+ void Post(const OmpTaskDependenceType::Type &c);
void Post(const OmpMapClause::Type &c);
void Post(const OmpScheduleClause::ScheduleType &c);
void Post(const OmpIfClause::DirectiveNameModifier &c);
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index ccdfe980f6f38c..31ad1b7c6ce5b5 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -513,8 +513,8 @@ class ParseTreeDumper {
NODE(OmpDependClause, InOut)
NODE(OmpDependClause, Sink)
NODE(OmpDependClause, Source)
- NODE(parser, OmpDependenceType)
- NODE_ENUM(OmpDependenceType, Type)
+ NODE(parser, OmpTaskDependenceType)
+ NODE_ENUM(OmpTaskDependenceType, Type)
NODE(parser, OmpDependSinkVec)
NODE(parser, OmpDependSinkVecLength)
NODE(parser, OmpEndAllocators)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 2a312e29a3a44d..506a470c5557b7 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3439,6 +3439,18 @@ struct OmpObject {
WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
+// Ref: [4.5:169-170], [5.0:254-256], [5.1:287-289], [5.2:321]
+//
+// task-dependence-type -> // "dependence-type" in 5.1 and before
+// IN | OUT | INOUT | // since 4.5
+// SOURCE | SINK | // since 4.5, until 5.1
+// MUTEXINOUTSET | DEPOBJ | // since 5.0
+// INOUTSET // since 5.2
+struct OmpTaskDependenceType {
+ ENUM_CLASS(Type, In, Out, Inout, Source, Sink)
+ WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Type);
+};
+
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple
// iterator-modifier -> iterator-specifier-list
struct OmpIteratorSpecifier {
@@ -3534,27 +3546,27 @@ struct OmpDependSinkVecLength {
std::tuple<DefinedOperator, ScalarIntConstantExpr> t;
};
-// 2.13.9 depend-vec -> iterator [+/- depend-vec-length],...,iterator[...]
+// 2.13.9 depend-vec -> induction-variable [depend-vec-length], ...
struct OmpDependSinkVec {
TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec);
std::tuple<Name, std::optional<OmpDependSinkVecLength>> t;
};
-// 2.13.9 depend-type -> IN | OUT | INOUT | SOURCE | SINK
-struct OmpDependenceType {
- ENUM_CLASS(Type, In, Out, Inout, Source, Sink)
- WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type);
-};
-
-// 2.13.9 depend-clause -> DEPEND (((IN | OUT | INOUT) : variable-name-list) |
-// SOURCE | SINK : depend-vec)
+// Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289], [5.2:323-324]
+//
+// depend-clause ->
+// DEPEND(SOURCE) | // since 4.5, until 5.1
+// DEPEND(SINK: depend-vec) | // since 4.5, until 5.1
+// DEPEND([depend-modifier,]dependence-type: locator-list) // since 4.5
+//
+// depend-modifier -> iterator-modifier // since 5.0
struct OmpDependClause {
UNION_CLASS_BOILERPLATE(OmpDependClause);
EMPTY_CLASS(Source);
WRAPPER_CLASS(Sink, std::list<OmpDependSinkVec>);
struct InOut {
TUPLE_CLASS_BOILERPLATE(InOut);
- std::tuple<OmpDependenceType, std::list<Designator>> t;
+ std::tuple<OmpTaskDependenceType, OmpObjectList> t;
};
std::variant<Source, Sink, InOut> u;
};
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index fbc031f3a93d7d..8fb0dd4a1ec3a7 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -798,11 +798,11 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const {
return findRepeatableClause<omp::clause::Depend>(
[&](const omp::clause::Depend &clause, const parser::CharBlock &) {
using Depend = omp::clause::Depend;
- assert(std::holds_alternative<Depend::WithLocators>(clause.u) &&
- "Only the modern form is handled at the moment");
- auto &modern = std::get<Depend::WithLocators>(clause.u);
- auto kind = std::get<Depend::TaskDependenceType>(modern.t);
- auto &objects = std::get<omp::ObjectList>(modern.t);
+ assert(std::holds_alternative<Depend::DepType>(clause.u) &&
+ "Only the form with dependence type is handled at the moment");
+ auto &depType = std::get<Depend::DepType>(clause.u);
+ auto kind = std::get<Depend::TaskDependenceType>(depType.t);
+ auto &objects = std::get<omp::ObjectList>(depType.t);
mlir::omp::ClauseTaskDependAttr dependTypeOperand =
genDependKindAttr(firOpBuilder, kind);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 3bd89b54328863..b1fa52751fbd7b 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -555,7 +555,7 @@ Depend make(const parser::OmpClause::Depend &inp,
using Iteration = Doacross::Vector::value_type; // LoopIterationT
CLAUSET_ENUM_CONVERT( //
- convert1, parser::OmpDependenceType::Type, Depend::TaskDependenceType,
+ convert1, parser::OmpTaskDependenceType::Type, Depend::TaskDependenceType,
// clang-format off
MS(In, In)
MS(Out, Out)
@@ -593,17 +593,13 @@ Depend make(const parser::OmpClause::Depend &inp,
return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink,
/*Vector=*/makeList(s.v, convert2)}};
},
- // Depend::WithLocators
+ // Depend::DepType
[&](const wrapped::InOut &s) -> Variant {
- auto &t0 = std::get<parser::OmpDependenceType>(s.t);
- auto &t1 = std::get<std::list<parser::Designator>>(s.t);
- auto convert4 = [&](const parser::Designator &t) {
- return makeObject(t, semaCtx);
- };
- return Depend::WithLocators{
- {/*TaskDependenceType=*/convert1(t0.v),
- /*Iterator=*/std::nullopt,
- /*LocatorList=*/makeList(t1, convert4)}};
+ auto &t0 = std::get<parser::OmpTaskDependenceType>(s.t);
+ auto &t1 = std::get<parser::OmpObjectList>(s.t);
+ return Depend::DepType{{/*TaskDependenceType=*/convert1(t0.v),
+ /*Iterator=*/std::nullopt,
+ /*LocatorList=*/makeObjects(t1, semaCtx)}};
},
},
inp.v.u)};
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index ae0c351fed56d1..3ca4e93a6c9b93 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -365,10 +365,10 @@ TYPE_PARSER(construct<OmpDependSinkVecLength>(
TYPE_PARSER(
construct<OmpDependSinkVec>(name, maybe(Parser<OmpDependSinkVecLength>{})))
-TYPE_PARSER(
- construct<OmpDependenceType>("IN"_id >> pure(OmpDependenceType::Type::In) ||
- "INOUT" >> pure(OmpDependenceType::Type::Inout) ||
- "OUT" >> pure(OmpDependenceType::Type::Out)))
+TYPE_PARSER(construct<OmpTaskDependenceType>(
+ "IN"_id >> pure(OmpTaskDependenceType::Type::In) ||
+ "INOUT" >> pure(OmpTaskDependenceType::Type::Inout) ||
+ "OUT" >> pure(OmpTaskDependenceType::Type::Out)))
TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
construct<OmpDependClause>(construct<OmpDependClause::Sink>(
@@ -376,7 +376,7 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
construct<OmpDependClause>(
construct<OmpDependClause::Source>("SOURCE"_tok)) ||
construct<OmpDependClause>(construct<OmpDependClause::InOut>(
- Parser<OmpDependenceType>{}, ":" >> nonemptyList(designator))))
+ Parser<OmpTaskDependenceType>{}, ":" >> Parser<OmpObjectList>{})))
// 2.15.3.7 LINEAR (linear-list: linear-step)
// linear-list -> list | modifier(list)
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index ba4155469073e6..39fcb61609e33b 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2216,9 +2216,9 @@ class UnparseVisitor {
}
void Unparse(const OmpDependClause::InOut &x) {
Put("(");
- Walk(std::get<OmpDependenceType>(x.t));
+ Walk(std::get<OmpTaskDependenceType>(x.t));
Put(":");
- Walk(std::get<std::list<Designator>>(x.t), ",");
+ Walk(std::get<OmpObjectList>(x.t));
Put(")");
}
bool Pre(const OmpDependClause &x) {
@@ -2829,7 +2829,7 @@ class UnparseVisitor {
OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier
WALK_NESTED_ENUM(OmpLinearModifier, Type) // OMP linear-modifier
- WALK_NESTED_ENUM(OmpDependenceType, Type) // OMP dependence-type
+ WALK_NESTED_ENUM(OmpTaskDependenceType, Type) // OMP task-dependence-type
WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type
WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 455322d610d6c2..599cc61a83bf0a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3288,15 +3288,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
parser::ToUpperCaseLetters(getDirectiveName(GetContext().directive)));
}
if (const auto *inOut{std::get_if<parser::OmpDependClause::InOut>(&x.v.u)}) {
- const auto &designators{std::get<std::list<parser::Designator>>(inOut->t)};
- for (const auto &ele : designators) {
- if (const auto *dataRef{std::get_if<parser::DataRef>(&ele.u)}) {
- CheckDependList(*dataRef);
- if (const auto *arr{
- std::get_if<common::Indirection<parser::ArrayElement>>(
- &dataRef->u)}) {
- CheckArraySection(arr->value(), GetLastName(*dataRef),
- llvm::omp::Clause::OMPC_depend);
+ for (const auto &object : std::get<parser::OmpObjectList>(inOut->t).v) {
+ if (const auto *name{std::get_if<parser::Name>(&object.u)}) {
+ context_.Say(GetContext().clauseSource,
+ "Common block name ('%s') cannot appear in a DEPEND "
+ "clause"_err_en_US,
+ name->ToString());
+ } else if (auto *designator{std::get_if<parser::Designator>(&object.u)}) {
+ if (auto *dataRef{std::get_if<parser::DataRef>(&designator->u)}) {
+ CheckDependList(*dataRef);
+ if (const auto *arr{
+ std::get_if<common::Indirection<parser::ArrayElement>>(
+ &dataRef->u)}) {
+ CheckArraySection(arr->value(), GetLastName(*dataRef),
+ llvm::omp::Clause::OMPC_depend);
+ }
}
}
}
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 979570a7d4103a..014b7987a658bd 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -435,6 +435,20 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
bool Pre(const parser::OpenMPAllocatorsConstruct &);
void Post(const parser::OpenMPAllocatorsConstruct &);
+ void Post(const parser::OmpObjectList &x) {
+ // The objects from OMP clauses should have already been resolved,
+ // except common blocks (the ResolveNamesVisitor does not visit
+ // parser::Name, those are dealt with as members of other structures).
+ // Iterate over elements of x, and resolve any common blocks that
+ // are still unresolved.
+ for (const parser::OmpObject &obj : x.v) {
+ auto *name{std::get_if<parser::Name>(&obj.u)};
+ if (name && !name->symbol) {
+ Resolve(*name, currScope().MakeCommonBlock(name->source));
+ }
+ }
+ }
+
// 2.15.3 Data-Sharing Attribute Clauses
void Post(const parser::OmpDefaultClause &);
bool Pre(const parser::OmpClause::Shared &x) {
@@ -531,16 +545,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
return false;
}
- bool Pre(const parser::OmpDependClause &x) {
- if (const auto *dependSink{
- std::get_if<parser::OmpDependClause::Sink>(&x.u)}) {
- const auto &dependSinkVec{dependSink->v};
- for (const auto &dependSinkElement : dependSinkVec) {
- const auto &name{std::get<parser::Name>(dependSinkElement.t)};
- ResolveName(&name);
- }
- }
- return false;
+ void Post(const parser::OmpDependSinkVec &x) {
+ const auto &name{std::get<parser::Name>(x.t)};
+ ResolveName(&name);
}
bool Pre(const parser::OmpClause::UseDevicePtr &x) {
diff --git a/flang/test/Semantics/OpenMP/depend04.f90 b/flang/test/Semantics/OpenMP/depend04.f90
new file mode 100644
index 00000000000000..8bdddb017d2c9d
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/depend04.f90
@@ -0,0 +1,10 @@
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50
+
+subroutine f00
+ integer :: x
+ common /cc/ x
+!ERROR: Common block name ('cc') cannot appear in a DEPEND clause
+ !$omp task depend(in: /cc/)
+ x = 0
+ !$omp end task
+end
diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
index ac34ddafc5e726..2a890905dc6323 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
@@ -503,7 +503,7 @@ struct DependT {
using LocatorList = ObjectListT<I, E>;
using TaskDependenceType = tomp::type::TaskDependenceType;
- struct WithLocators { // Modern form
+ struct DepType { // The form with task dependence type.
using TupleTrait = std::true_type;
// Empty LocatorList means "omp_all_memory".
std::tuple<TaskDependenceType, OPT(Iterator), LocatorList> t;
@@ -511,7 +511,7 @@ struct DependT {
using Doacross = DoacrossT<T, I, E>;
using UnionTrait = std::true_type;
- std::variant<Doacross, WithLocators> u; // Doacross form is legacy
+ std::variant<Doacross, DepType> u; // Doacross form is legacy
};
// V5.2: [3.5] `destroy` clause
More information about the flang-commits
mailing list