-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[flang][OpenMP] Parsing support for iterator in DEPEND clause #113622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
Warn about use of iterators OpenMP versions that didn't have them (support added in 5.0). Emit a TODO error in lowering.
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-fir-hlfir Author: Krzysztof Parzyszek (kparzysz) ChangesWarn about use of iterators OpenMP versions that didn't have them (support added in 5.0). Emit a TODO error in lowering. Full diff: https://github.com/llvm/llvm-project/pull/113622.diff 7 Files Affected:
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 25e2f42ca29bd5..227fb50fcf7e7e 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3566,7 +3566,9 @@ struct OmpDependClause {
WRAPPER_CLASS(Sink, std::list<OmpDependSinkVec>);
struct InOut {
TUPLE_CLASS_BOILERPLATE(InOut);
- std::tuple<OmpTaskDependenceType, OmpObjectList> t;
+ std::tuple<std::optional<OmpIteratorModifier>, 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 12fe3b572244f0..c71fbded443dee 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -795,35 +795,41 @@ bool ClauseProcessor::processCopyprivate(
bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- return findRepeatableClause<omp::clause::Depend>(
- [&](const omp::clause::Depend &clause, const parser::CharBlock &) {
- using Depend = omp::clause::Depend;
- assert(std::holds_alternative<Depend::DepType>(clause.u) &&
- "Only the form with depenence 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);
- result.dependKinds.append(objects.size(), dependTypeOperand);
-
- for (const omp::Object &object : objects) {
- assert(object.ref() && "Expecting designator");
-
- if (evaluate::ExtractSubstring(*object.ref())) {
- TODO(converter.getCurrentLocation(),
- "substring not supported for task depend");
- } else if (evaluate::IsArrayElement(*object.ref())) {
- TODO(converter.getCurrentLocation(),
- "array sections not supported for task depend");
- }
+ auto process = [&](const omp::clause::Depend &clause,
+ const parser::CharBlock &) {
+ using Depend = omp::clause::Depend;
+ assert(std::holds_alternative<Depend::DepType>(clause.u) &&
+ "Only the form with depenence 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);
+
+ if (std::get<std::optional<omp::clause::Iterator>>(depType.t)) {
+ TODO(converter.getCurrentLocation(),
+ "Support for iterator modifiers is not implemented yet");
+ }
+ mlir::omp::ClauseTaskDependAttr dependTypeOperand =
+ genDependKindAttr(firOpBuilder, kind);
+ result.dependKinds.append(objects.size(), dependTypeOperand);
+
+ for (const omp::Object &object : objects) {
+ assert(object.ref() && "Expecting designator");
+
+ if (evaluate::ExtractSubstring(*object.ref())) {
+ TODO(converter.getCurrentLocation(),
+ "substring not supported for task depend");
+ } else if (evaluate::IsArrayElement(*object.ref())) {
+ TODO(converter.getCurrentLocation(),
+ "array sections not supported for task depend");
+ }
- semantics::Symbol *sym = object.sym();
- const mlir::Value variable = converter.getSymbolAddress(*sym);
- result.dependVars.push_back(variable);
- }
- });
+ semantics::Symbol *sym = object.sym();
+ const mlir::Value variable = converter.getSymbolAddress(*sym);
+ result.dependVars.push_back(variable);
+ }
+ };
+
+ return findRepeatableClause<omp::clause::Depend>(process);
}
bool ClauseProcessor::processHasDeviceAddr(
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 57667fbbf7d4aa..21a246815782c9 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -595,12 +595,16 @@ Depend make(const parser::OmpClause::Depend &inp,
},
// Depend::DepType
[&](const wrapped::InOut &s) -> Variant {
- 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)}};
+ auto &t0 =
+ std::get<std::optional<parser::OmpIteratorModifier>>(s.t);
+ auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
+ auto &t2 = std::get<parser::OmpObjectList>(s.t);
+
+ auto &&maybeIter = maybeApply(
+ [&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
+ return Depend::DepType{{/*TaskDependenceType=*/convert1(t1.v),
+ /*Iterator=*/std::move(maybeIter),
+ /*LocatorList=*/makeObjects(t2, semaCtx)}};
},
},
inp.v.u)};
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 3a19d44559dccd..869419b00bf867 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -376,7 +376,8 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
construct<OmpDependClause>(
construct<OmpDependClause::Source>("SOURCE"_tok)) ||
construct<OmpDependClause>(construct<OmpDependClause::InOut>(
- Parser<OmpTaskDependenceType>{}, ":" >> Parser<OmpObjectList>{})))
+ maybe(Parser<OmpIteratorModifier>{} / ","_tok),
+ Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})))
// 2.15.3.7 LINEAR (linear-list: linear-step)
// linear-list -> list | modifier(list)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index e9a2fcecd9efe6..99f208f1280c2a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3307,6 +3307,15 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
}
}
}
+ if (std::get<std::optional<parser::OmpIteratorModifier>>(inOut->t)) {
+ unsigned version{context_.langOptions().OpenMPVersion};
+ unsigned allowedInVersion = 50;
+ if (version < allowedInVersion) {
+ context_.Say(GetContext().clauseSource,
+ "Iterator modifiers are not supported in %s, %s"_warn_en_US,
+ ThisVersion(version), TryVersion(allowedInVersion));
+ }
+ }
}
}
diff --git a/flang/test/Lower/OpenMP/Todo/depend-clause.f90 b/flang/test/Lower/OpenMP/Todo/depend-clause.f90
new file mode 100644
index 00000000000000..74525888c91d6d
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/depend-clause.f90
@@ -0,0 +1,10 @@
+!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
+!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
+
+!CHECK: Support for iterator modifiers is not implemented yet
+subroutine f00(x)
+ integer :: x(10)
+ !$omp task depend(iterator(i = 1:10), in: x(i))
+ x = 0
+ !$omp end task
+end
diff --git a/flang/test/Semantics/OpenMP/depend05.f90 b/flang/test/Semantics/OpenMP/depend05.f90
new file mode 100644
index 00000000000000..53fd82bd08a9eb
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/depend05.f90
@@ -0,0 +1,9 @@
+!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=45 -Werror
+
+subroutine f00(x)
+ integer :: x(10)
+!WARNING: Iterator modifiers are not supported in OpenMP v4.5, try -fopenmp-version=50
+ !$omp task depend(iterator(i = 1:10), in: x(i))
+ x = 0
+ !$omp end task
+end
|
…kparzysz/spr/d02-flang-omp-hint
…ysz/spr/d03-flang-depend-iterator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG.
@@ -3307,6 +3307,15 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { | |||
} | |||
} | |||
} | |||
if (std::get<std::optional<parser::OmpIteratorModifier>>(inOut->t)) { | |||
unsigned version{context_.langOptions().OpenMPVersion}; | |||
unsigned allowedInVersion = 50; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unsigned allowedInVersion = 50; | |
unsigned allowedInVersion{50}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
const parser::CharBlock &) { | ||
using Depend = omp::clause::Depend; | ||
assert(std::holds_alternative<Depend::DepType>(clause.u) && | ||
"Only the form with depenence type is handled at the moment"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
…13622) Warn about use of iterators OpenMP versions that didn't have them (support added in 5.0). Emit a TODO error in lowering.
Warn about use of iterators OpenMP versions that didn't have them (support added in 5.0). Emit a TODO error in lowering.