[flang][OpenMP] Parsing support for iterator modifiers in FROM and TO #114593
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Parse PRESENT modifier as well while we're at it (no MAPPER though). Add semantic checks for these clauses in the TARGET UPDATE construct, TODO messages in lowering.
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-parser @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesParse PRESENT modifier as well while we're at it (no MAPPER though). Add semantic checks for these clauses in the TARGET UPDATE construct, TODO messages in lowering. Patch is 46.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114593.diff 22 Files Affected:
diff --git a/flang/include/flang/Evaluate/check-expression.h b/flang/include/flang/Evaluate/check-expression.h
index b711d289ba5246..4a541d177ca447 100644
--- a/flang/include/flang/Evaluate/check-expression.h
+++ b/flang/include/flang/Evaluate/check-expression.h
@@ -115,6 +115,10 @@ extern template std::optional<bool> IsContiguous(
const CoarrayRef &, FoldingContext &);
extern template std::optional<bool> IsContiguous(
const Symbol &, FoldingContext &);
+static inline std::optional<bool> IsContiguous(const SymbolRef &s,
+ FoldingContext &c) {
+ return IsContiguous(s.get(), c);
+}
template <typename A>
bool IsSimplyContiguous(const A &x, FoldingContext &context) {
return IsContiguous(x, context).value_or(false);
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 67f7e1aac40edb..066ff2e7f8b163 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -524,6 +524,8 @@ class ParseTreeDumper {
NODE(parser, OmpEndCriticalDirective)
NODE(parser, OmpEndLoopDirective)
NODE(parser, OmpEndSectionsDirective)
+ NODE(parser, OmpFromClause)
+ NODE_ENUM(OmpFromClause, Expectation)
NODE(parser, OmpIfClause)
NODE_ENUM(OmpIfClause, DirectiveNameModifier)
NODE_ENUM(OmpLastprivateClause, LastprivateModifier)
@@ -581,6 +583,9 @@ class ParseTreeDumper {
NODE(parser, OmpSectionBlocks)
NODE(parser, OmpSectionsDirective)
NODE(parser, OmpSimpleStandaloneDirective)
+ NODE(parser, OmpToClause)
+ // No NODE_ENUM for OmpToClause::Expectation, because it's an alias
+ // for OmpFromClause::Expectation.
NODE(parser, Only)
NODE(parser, OpenACCAtomicConstruct)
NODE(parser, OpenACCBlockConstruct)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 13c3353512208b..4026594b99880c 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3582,6 +3582,26 @@ struct OmpDeviceTypeClause {
WRAPPER_CLASS_BOILERPLATE(OmpDeviceTypeClause, Type);
};
+// Ref: [4.5:107-109], [5.0:176-180], [5.1:205-210], [5.2:167-168]
+//
+// from-clause ->
+// FROM(locator-list) |
+// FROM(mapper-modifier: locator-list) | // since 5.0
+// FROM(motion-modifier[,] ...: locator-list) // since 5.1
+// motion-modifier ->
+// PRESENT | mapper-modifier | iterator-modifier
+struct OmpFromClause {
+ ENUM_CLASS(Expectation, Present);
+ TUPLE_CLASS_BOILERPLATE(OmpFromClause);
+
+ // As in the case of MAP, modifiers are parsed as lists, even if they
+ // are unique. These restrictions will be checked in semantic checks.
+ std::tuple<std::optional<std::list<Expectation>>,
+ std::optional<std::list<OmpIteratorModifier>>, OmpObjectList,
+ bool> // were the modifiers comma-separated?
+ t;
+};
+
// OMP 5.2 12.6.1 grainsize-clause -> grainsize ([prescriptiveness :] value)
struct OmpGrainsizeClause {
TUPLE_CLASS_BOILERPLATE(OmpGrainsizeClause);
@@ -3718,6 +3738,28 @@ struct OmpScheduleClause {
t;
};
+// Ref: [4.5:107-109], [5.0:176-180], [5.1:205-210], [5.2:167-168]
+//
+// to-clause (in DECLARE TARGET) ->
+// TO(extended-list) | // until 5.1
+// to-clause (in TARGET UPDATE) ->
+// TO(locator-list) |
+// TO(mapper-modifier: locator-list) | // since 5.0
+// TO(motion-modifier[,] ...: locator-list) // since 5.1
+// motion-modifier ->
+// PRESENT | mapper-modifier | iterator-modifier
+struct OmpToClause {
+ using Expectation = OmpFromClause::Expectation;
+ TUPLE_CLASS_BOILERPLATE(OmpToClause);
+
+ // As in the case of MAP, modifiers are parsed as lists, even if they
+ // are unique. These restrictions will be checked in semantic checks.
+ std::tuple<std::optional<std::list<Expectation>>,
+ std::optional<std::list<OmpIteratorModifier>>, OmpObjectList,
+ bool> // were the modifiers comma-separated?
+ t;
+};
+
// OMP 5.2 12.6.2 num_tasks-clause -> num_tasks ([prescriptiveness :] value)
struct OmpNumTasksClause {
TUPLE_CLASS_BOILERPLATE(OmpNumTasksClause);
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 8eb1fdb4709178..69950b2f044e41 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1019,8 +1019,15 @@ bool ClauseProcessor::processMotionClauses(lower::StatementContext &stmtCtx,
auto callbackFn = [&](const auto &clause, const parser::CharBlock &source) {
mlir::Location clauseLocation = converter.genLocation(source);
-
+ const auto &[expectation, mapper, iterator, objects] = clause.t;
// TODO Support motion modifiers: present, mapper, iterator.
+ if (expectation) {
+ TODO(clauseLocation, "PRESENT modifier is not supported yet");
+ } else if (mapper) {
+ TODO(clauseLocation, "Mapper modifier is not supported yet");
+ } else if (iterator) {
+ TODO(clauseLocation, "Iterator modifier is not supported yet");
+ }
constexpr llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
std::is_same_v<llvm::remove_cvref_t<decltype(clause)>, omp::clause::To>
? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 45b89de023a4bf..c6cdd1dd0ca6b3 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -728,21 +728,51 @@ Firstprivate make(const parser::OmpClause::Firstprivate &inp,
From make(const parser::OmpClause::From &inp,
semantics::SemanticsContext &semaCtx) {
- // inp.v -> parser::OmpObjectList
- return From{{/*Expectation=*/std::nullopt, /*Mapper=*/std::nullopt,
- /*Iterator=*/std::nullopt,
- /*LocatorList=*/makeObjects(inp.v, semaCtx)}};
+ // inp.v -> parser::OmpFromClause
+ using wrapped = parser::OmpFromClause;
+
+ CLAUSET_ENUM_CONVERT( //
+ convert, parser::OmpFromClause::Expectation, From::Expectation,
+ // clang-format off
+ MS(Present, Present)
+ // clang-format on
+ );
+
+ auto &t0 = std::get<std::optional<std::list<wrapped::Expectation>>>(inp.v.t);
+ auto &t1 =
+ std::get<std::optional<std::list<parser::OmpIteratorModifier>>>(inp.v.t);
+ auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
+
+ assert((!t0 || t0->size() == 1) && "Only one expectation modifier allowed");
+ assert((!t1 || t1->size() == 1) && "Only one iterator modifier allowed");
+
+ auto expectation = [&]() -> std::optional<From::Expectation> {
+ if (t0)
+ return convert(t0->front());
+ return std::nullopt;
+ }();
+
+ auto iterator = [&]() -> std::optional<Iterator> {
+ if (t1)
+ return makeIterator(t1->front(), semaCtx);
+ return std::nullopt;
+ }();
+
+ return From{{/*Expectation=*/std::move(expectation), /*Mapper=*/std::nullopt,
+ /*Iterator=*/std::move(iterator),
+ /*LocatorList=*/makeObjects(t2, semaCtx)}};
}
// Full: empty
Grainsize make(const parser::OmpClause::Grainsize &inp,
- semantics::SemanticsContext &semaCtx) {
+ semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpGrainsizeClause
using wrapped = parser::OmpGrainsizeClause;
CLAUSET_ENUM_CONVERT( //
- convert, parser::OmpGrainsizeClause::Prescriptiveness, Grainsize::Prescriptiveness,
+ convert, parser::OmpGrainsizeClause::Prescriptiveness,
+ Grainsize::Prescriptiveness,
// clang-format off
MS(Strict, Strict)
// clang-format on
@@ -1274,10 +1304,39 @@ ThreadLimit make(const parser::OmpClause::ThreadLimit &inp,
To make(const parser::OmpClause::To &inp,
semantics::SemanticsContext &semaCtx) {
- // inp.v -> parser::OmpObjectList
- return To{{/*Expectation=*/std::nullopt, /*Mapper=*/std::nullopt,
- /*Iterator=*/std::nullopt,
- /*LocatorList=*/makeObjects(inp.v, semaCtx)}};
+ // inp.v -> parser::OmpToClause
+ using wrapped = parser::OmpToClause;
+
+ CLAUSET_ENUM_CONVERT( //
+ convert, parser::OmpToClause::Expectation, To::Expectation,
+ // clang-format off
+ MS(Present, Present)
+ // clang-format on
+ );
+
+ auto &t0 = std::get<std::optional<std::list<wrapped::Expectation>>>(inp.v.t);
+ auto &t1 =
+ std::get<std::optional<std::list<parser::OmpIteratorModifier>>>(inp.v.t);
+ auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
+
+ assert((!t0 || t0->size() == 1) && "Only one expectation modifier allowed");
+ assert((!t1 || t1->size() == 1) && "Only one iterator modifier allowed");
+
+ auto expectation = [&]() -> std::optional<To::Expectation> {
+ if (t0)
+ return convert(t0->front());
+ return std::nullopt;
+ }();
+
+ auto iterator = [&]() -> std::optional<Iterator> {
+ if (t1)
+ return makeIterator(t1->front(), semaCtx);
+ return std::nullopt;
+ }();
+
+ return To{{/*Expectation=*/std::move(expectation), /*Mapper=*/std::nullopt,
+ /*Iterator=*/std::move(iterator),
+ /*LocatorList=*/makeObjects(t2, semaCtx)}};
}
// UnifiedAddress: empty
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 6fde70fc5c3878..b0d9716b4f93a6 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -122,6 +122,40 @@ template <typename Separator> struct MapModifiers {
const Separator sep_;
};
+// This is almost exactly the same thing as MapModifiers. It has the same
+// issue (it expects modifiers in a specific order), and the fix for that
+// will change how modifiers are parsed. Instead of making this code more
+// generic, make it simple, and generalize after the fix is in place.
+template <typename Separator> struct MotionModifiers {
+ constexpr MotionModifiers(Separator sep) : sep_(sep) {}
+ constexpr MotionModifiers(const MotionModifiers &) = default;
+ constexpr MotionModifiers(MotionModifiers &&) = default;
+
+ // Parsing of mappers if not implemented yet.
+ using ExpParser = Parser<OmpFromClause::Expectation>;
+ using IterParser = Parser<OmpIteratorModifier>;
+ using ModParser = ConcatSeparated<Separator, ExpParser, IterParser>;
+
+ using resultType = typename ModParser::resultType;
+
+ std::optional<resultType> Parse(ParseState &state) const {
+ auto mp = ModParser(sep_, ExpParser{}, IterParser{});
+ auto mods = mp.Parse(state);
+ // The ModParser always "succeeds", i.e. even if the input is junk, it
+ // will return a tuple filled with nullopts. If any of the components
+ // is not a nullopt, expect a ":".
+ if (std::apply([](auto &&...opts) { return (... || !!opts); }, *mods)) {
+ if (!attempt(":"_tok).Parse(state)) {
+ return std::nullopt;
+ }
+ }
+ return std::move(mods);
+ }
+
+private:
+ const Separator sep_;
+};
+
// OpenMP Clauses
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple |
@@ -382,6 +416,31 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
maybe(Parser<OmpIteratorModifier>{} / ","_tok),
Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})))
+TYPE_PARSER(construct<OmpFromClause::Expectation>(
+ "PRESENT" >> pure(OmpFromClause::Expectation::Present)))
+
+template <typename MotionClause, bool CommasEverywhere>
+static inline MotionClause makeMotionClause(
+ std::tuple<std::optional<std::list<typename MotionClause::Expectation>>,
+ std::optional<std::list<OmpIteratorModifier>>> &&mods,
+ OmpObjectList &&objs) {
+ auto &&[exp, iter] = std::move(mods);
+ return MotionClause(
+ std::move(exp), std::move(iter), std::move(objs), CommasEverywhere);
+}
+
+TYPE_PARSER(construct<OmpFromClause>(
+ applyFunction<OmpFromClause>(makeMotionClause<OmpFromClause, true>,
+ MotionModifiers(","_tok), Parser<OmpObjectList>{}) ||
+ applyFunction<OmpFromClause>(makeMotionClause<OmpFromClause, false>,
+ MotionModifiers(maybe(","_tok)), Parser<OmpObjectList>{})))
+
+TYPE_PARSER(construct<OmpToClause>(
+ applyFunction<OmpToClause>(makeMotionClause<OmpToClause, true>,
+ MotionModifiers(","_tok), Parser<OmpObjectList>{}) ||
+ applyFunction<OmpToClause>(makeMotionClause<OmpToClause, false>,
+ MotionModifiers(maybe(","_tok)), Parser<OmpObjectList>{})))
+
// 2.15.3.7 LINEAR (linear-list: linear-step)
// linear-list -> list | modifier(list)
// linear-modifier -> REF | VAL | UVAL
@@ -475,11 +534,11 @@ TYPE_PARSER(
parenthesized(scalarIntExpr))) ||
"FINAL" >> construct<OmpClause>(construct<OmpClause::Final>(
parenthesized(scalarLogicalExpr))) ||
- "FULL" >> construct<OmpClause>(construct<OmpClause::Full>()) ||
"FIRSTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Firstprivate>(
parenthesized(Parser<OmpObjectList>{}))) ||
"FROM" >> construct<OmpClause>(construct<OmpClause::From>(
- parenthesized(Parser<OmpObjectList>{}))) ||
+ parenthesized(Parser<OmpFromClause>{}))) ||
+ "FULL" >> construct<OmpClause>(construct<OmpClause::Full>()) ||
"GRAINSIZE" >> construct<OmpClause>(construct<OmpClause::Grainsize>(
parenthesized(Parser<OmpGrainsizeClause>{}))) ||
"HAS_DEVICE_ADDR" >>
@@ -554,7 +613,7 @@ TYPE_PARSER(
"THREAD_LIMIT" >> construct<OmpClause>(construct<OmpClause::ThreadLimit>(
parenthesized(scalarIntExpr))) ||
"TO" >> construct<OmpClause>(construct<OmpClause::To>(
- parenthesized(Parser<OmpObjectList>{}))) ||
+ parenthesized(Parser<OmpToClause>{}))) ||
"USE_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::UseDevicePtr>(
parenthesized(Parser<OmpObjectList>{}))) ||
"USE_DEVICE_ADDR" >>
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 3b0824f80161f4..88b5ac626264b1 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2138,6 +2138,27 @@ class UnparseVisitor {
Put(",");
Walk(std::get<std::optional<ScalarIntConstantExpr>>(x.t));
}
+ void Unparse(const OmpFromClause &x) {
+ auto &expect =
+ std::get<std::optional<std::list<OmpFromClause::Expectation>>>(x.t);
+ auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t);
+ bool needComma{false};
+ if (expect) {
+ Walk(*expect);
+ needComma = true;
+ }
+ if (iter) {
+ if (needComma) {
+ Put(", ");
+ }
+ Walk(*iter);
+ needComma = true;
+ }
+ if (needComma) {
+ Put(": ");
+ }
+ Walk(std::get<OmpObjectList>(x.t));
+ }
void Unparse(const OmpIfClause &x) {
Walk(std::get<std::optional<OmpIfClause::DirectiveNameModifier>>(x.t), ":");
Walk(std::get<ScalarLogicalExpr>(x.t));
@@ -2241,6 +2262,27 @@ class UnparseVisitor {
Walk(":",
std::get<std::optional<OmpDefaultmapClause::VariableCategory>>(x.t));
}
+ void Unparse(const OmpToClause &x) {
+ auto &expect =
+ std::get<std::optional<std::list<OmpToClause::Expectation>>>(x.t);
+ auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t);
+ bool needComma{false};
+ if (expect) {
+ Walk(*expect);
+ needComma = true;
+ }
+ if (iter) {
+ if (needComma) {
+ Put(", ");
+ }
+ Walk(*iter);
+ needComma = true;
+ }
+ if (needComma) {
+ Put(": ");
+ }
+ Walk(std::get<OmpObjectList>(x.t));
+ }
#define GEN_FLANG_CLAUSE_UNPARSE
#include "llvm/Frontend/OpenMP/OMP.inc"
void Unparse(const OmpLoopDirective &x) {
@@ -2858,6 +2900,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
WALK_NESTED_ENUM(
OmpReductionClause, ReductionModifier) // OMP reduction-modifier
+ WALK_NESTED_ENUM(OmpFromClause, Expectation) // OMP motion-expectation
WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
WALK_NESTED_ENUM(OmpOrderClause, Type) // OMP order-type
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index c813100b4b16c8..4f7d0356f5a44d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -8,6 +8,7 @@
#include "check-omp-structure.h"
#include "definable.h"
+#include "flang/Evaluate/check-expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/tools.h"
@@ -268,6 +269,57 @@ bool OmpStructureChecker::IsCloselyNestedRegion(const OmpDirectiveSet &set) {
return false;
}
+namespace {
+struct ContiguousHelper {
+ ContiguousHelper(SemanticsContext &context)
+ : sctx_(context), fctx_(context.foldingContext()) {}
+
+ template <typename Contained>
+ std::optional<bool> Visit(const common::Indirection<Contained> &x) {
+ return Visit(x.value());
+ }
+ template <typename Contained>
+ std::optional<bool> Visit(const common::Reference<Contained> &x) {
+ return Visit(x.get());
+ }
+ template <typename T> std::optional<bool> Visit(const evaluate::Expr<T> &x) {
+ return std::visit([this](auto &&s) { return Visit(s); }, x.u);
+ }
+ template <typename T>
+ std::optional<bool> Visit(const evaluate::Designator<T> &x) {
+ return common::visit(
+ [this](auto &&s) { return evaluate::IsContiguous(s, fctx_); }, x.u);
+ }
+ template <typename T> std::optional<bool> Visit(const T &) {
+ // Everything else.
+ return std::nullopt;
+ }
+
+private:
+ SemanticsContext &sctx_;
+ evaluate::FoldingContext &fctx_;
+};
+} // namespace
+
+std::optional<bool>
+OmpStructureChecker::IsContiguous(const parser::OmpObject &object) {
+ return common::visit(
+ common::visitors{
+ [&](const parser::Name &x) {
+ // Any member of a common block must be contiguous.
+ return std::optional<bool>{true};
+ },
+ [&](const parser::Designator &x) {
+ evaluate::ExpressionAnalyzer ea{context_};
+ if (MaybeExpr maybeExpr{ea.Analyze(x)}) {
+ return ContiguousHelper{context_}.Visit(*maybeExpr);
+ }
+ return std::optional<bool>{};
+ },
+ },
+ object.u);
+}
+
void OmpStructureChecker::CheckMultipleOccurrence(
semantics::UnorderedSymbolSet &listVars,
const std::list<parser::Name> &nameList, const parser::CharBlock &item,
@@ -1466,9 +1518,10 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
common::visitors{
[&](const parser::OmpClause::To &toClause) {
toClauseFound = true;
- CheckSymbolNames(dir.source, toClause.v);
- CheckIsVarPartOfAnotherVar(dir.source, toClause.v);
- CheckThreadprivateOrDeclareTargetVar(toClause.v);
+ auto &objList{std::get<parser::OmpObjectList>(toClause.v.t)};
+ CheckSymbolNames(dir.source, objList);
+ CheckIsVarPartOfAnotherVar(dir.source, objList);
+ CheckThreadprivateOrDeclareTargetVar(objList);
},
[&](const parser::OmpClause::Link &linkClause) {
CheckSymbolNames(dir.source, linkClause.v);
@@ -1647,24 +1700,29 @@ void OmpStructureChecker::CheckOrderedDependClause(
}
void OmpStructureChecker::CheckTargetUpdate() {
- const parser::OmpClause *toClause = FindClause(llvm::omp::Clause::OMPC_to);
- const parser::OmpClause *fromClause =
- FindClause(llvm::omp::Clause::OMPC_from);
- if (!toClause && !fromClause) {
+ const parser::OmpClause *toWrapper{FindClause(llvm::omp::Clause::OMPC_to)};
+ const parser::OmpClause *fromWrapper{
+ FindClause(llvm::omp::Clause::OMPC_from)};
+ if (!toWrapper && !fromWrapper) {
context_.Say(GetContext().directiveSource,
- "At least one motion-clause (TO/FROM) must be specified on TARGET UPDATE construct."_err_en_US);
+ "At least one motion-clause (TO/FROM) must be specified on "
+ "TARGET UPDATE construct."_err_en_US);
}
- if (toClause && fromClause) {
+ if (toWrapper && fromWrapper) {
SymbolSourceMap toSymbols, from...
[truncated]
|
Member
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Krzysztof Parzyszek (kparzysz) ChangesParse PRESENT modifier as well while we're at it (no MAPPER though). Add semantic checks for these clauses in the TARGET UPDATE construct, TODO messages in lowering. Patch is 46.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114593.diff 22 Files Affected:
diff --git a/flang/include/flang/Evaluate/check-expression.h b/flang/include/flang/Evaluate/check-expression.h
index b711d289ba5246..4a541d177ca447 100644
--- a/flang/include/flang/Evaluate/check-expression.h
+++ b/flang/include/flang/Evaluate/check-expression.h
@@ -115,6 +115,10 @@ extern template std::optional<bool> IsContiguous(
const CoarrayRef &, FoldingContext &);
extern template std::optional<bool> IsContiguous(
const Symbol &, FoldingContext &);
+static inline std::optional<bool> IsContiguous(const SymbolRef &s,
+ FoldingContext &c) {
+ return IsContiguous(s.get(), c);
+}
template <typename A>
bool IsSimplyContiguous(const A &x, FoldingContext &context) {
return IsContiguous(x, context).value_or(false);
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 67f7e1aac40edb..066ff2e7f8b163 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -524,6 +524,8 @@ class ParseTreeDumper {
NODE(parser, OmpEndCriticalDirective)
NODE(parser, OmpEndLoopDirective)
NODE(parser, OmpEndSectionsDirective)
+ NODE(parser, OmpFromClause)
+ NODE_ENUM(OmpFromClause, Expectation)
NODE(parser, OmpIfClause)
NODE_ENUM(OmpIfClause, DirectiveNameModifier)
NODE_ENUM(OmpLastprivateClause, LastprivateModifier)
@@ -581,6 +583,9 @@ class ParseTreeDumper {
NODE(parser, OmpSectionBlocks)
NODE(parser, OmpSectionsDirective)
NODE(parser, OmpSimpleStandaloneDirective)
+ NODE(parser, OmpToClause)
+ // No NODE_ENUM for OmpToClause::Expectation, because it's an alias
+ // for OmpFromClause::Expectation.
NODE(parser, Only)
NODE(parser, OpenACCAtomicConstruct)
NODE(parser, OpenACCBlockConstruct)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 13c3353512208b..4026594b99880c 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3582,6 +3582,26 @@ struct OmpDeviceTypeClause {
WRAPPER_CLASS_BOILERPLATE(OmpDeviceTypeClause, Type);
};
+// Ref: [4.5:107-109], [5.0:176-180], [5.1:205-210], [5.2:167-168]
+//
+// from-clause ->
+// FROM(locator-list) |
+// FROM(mapper-modifier: locator-list) | // since 5.0
+// FROM(motion-modifier[,] ...: locator-list) // since 5.1
+// motion-modifier ->
+// PRESENT | mapper-modifier | iterator-modifier
+struct OmpFromClause {
+ ENUM_CLASS(Expectation, Present);
+ TUPLE_CLASS_BOILERPLATE(OmpFromClause);
+
+ // As in the case of MAP, modifiers are parsed as lists, even if they
+ // are unique. These restrictions will be checked in semantic checks.
+ std::tuple<std::optional<std::list<Expectation>>,
+ std::optional<std::list<OmpIteratorModifier>>, OmpObjectList,
+ bool> // were the modifiers comma-separated?
+ t;
+};
+
// OMP 5.2 12.6.1 grainsize-clause -> grainsize ([prescriptiveness :] value)
struct OmpGrainsizeClause {
TUPLE_CLASS_BOILERPLATE(OmpGrainsizeClause);
@@ -3718,6 +3738,28 @@ struct OmpScheduleClause {
t;
};
+// Ref: [4.5:107-109], [5.0:176-180], [5.1:205-210], [5.2:167-168]
+//
+// to-clause (in DECLARE TARGET) ->
+// TO(extended-list) | // until 5.1
+// to-clause (in TARGET UPDATE) ->
+// TO(locator-list) |
+// TO(mapper-modifier: locator-list) | // since 5.0
+// TO(motion-modifier[,] ...: locator-list) // since 5.1
+// motion-modifier ->
+// PRESENT | mapper-modifier | iterator-modifier
+struct OmpToClause {
+ using Expectation = OmpFromClause::Expectation;
+ TUPLE_CLASS_BOILERPLATE(OmpToClause);
+
+ // As in the case of MAP, modifiers are parsed as lists, even if they
+ // are unique. These restrictions will be checked in semantic checks.
+ std::tuple<std::optional<std::list<Expectation>>,
+ std::optional<std::list<OmpIteratorModifier>>, OmpObjectList,
+ bool> // were the modifiers comma-separated?
+ t;
+};
+
// OMP 5.2 12.6.2 num_tasks-clause -> num_tasks ([prescriptiveness :] value)
struct OmpNumTasksClause {
TUPLE_CLASS_BOILERPLATE(OmpNumTasksClause);
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 8eb1fdb4709178..69950b2f044e41 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1019,8 +1019,15 @@ bool ClauseProcessor::processMotionClauses(lower::StatementContext &stmtCtx,
auto callbackFn = [&](const auto &clause, const parser::CharBlock &source) {
mlir::Location clauseLocation = converter.genLocation(source);
-
+ const auto &[expectation, mapper, iterator, objects] = clause.t;
// TODO Support motion modifiers: present, mapper, iterator.
+ if (expectation) {
+ TODO(clauseLocation, "PRESENT modifier is not supported yet");
+ } else if (mapper) {
+ TODO(clauseLocation, "Mapper modifier is not supported yet");
+ } else if (iterator) {
+ TODO(clauseLocation, "Iterator modifier is not supported yet");
+ }
constexpr llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
std::is_same_v<llvm::remove_cvref_t<decltype(clause)>, omp::clause::To>
? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 45b89de023a4bf..c6cdd1dd0ca6b3 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -728,21 +728,51 @@ Firstprivate make(const parser::OmpClause::Firstprivate &inp,
From make(const parser::OmpClause::From &inp,
semantics::SemanticsContext &semaCtx) {
- // inp.v -> parser::OmpObjectList
- return From{{/*Expectation=*/std::nullopt, /*Mapper=*/std::nullopt,
- /*Iterator=*/std::nullopt,
- /*LocatorList=*/makeObjects(inp.v, semaCtx)}};
+ // inp.v -> parser::OmpFromClause
+ using wrapped = parser::OmpFromClause;
+
+ CLAUSET_ENUM_CONVERT( //
+ convert, parser::OmpFromClause::Expectation, From::Expectation,
+ // clang-format off
+ MS(Present, Present)
+ // clang-format on
+ );
+
+ auto &t0 = std::get<std::optional<std::list<wrapped::Expectation>>>(inp.v.t);
+ auto &t1 =
+ std::get<std::optional<std::list<parser::OmpIteratorModifier>>>(inp.v.t);
+ auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
+
+ assert((!t0 || t0->size() == 1) && "Only one expectation modifier allowed");
+ assert((!t1 || t1->size() == 1) && "Only one iterator modifier allowed");
+
+ auto expectation = [&]() -> std::optional<From::Expectation> {
+ if (t0)
+ return convert(t0->front());
+ return std::nullopt;
+ }();
+
+ auto iterator = [&]() -> std::optional<Iterator> {
+ if (t1)
+ return makeIterator(t1->front(), semaCtx);
+ return std::nullopt;
+ }();
+
+ return From{{/*Expectation=*/std::move(expectation), /*Mapper=*/std::nullopt,
+ /*Iterator=*/std::move(iterator),
+ /*LocatorList=*/makeObjects(t2, semaCtx)}};
}
// Full: empty
Grainsize make(const parser::OmpClause::Grainsize &inp,
- semantics::SemanticsContext &semaCtx) {
+ semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpGrainsizeClause
using wrapped = parser::OmpGrainsizeClause;
CLAUSET_ENUM_CONVERT( //
- convert, parser::OmpGrainsizeClause::Prescriptiveness, Grainsize::Prescriptiveness,
+ convert, parser::OmpGrainsizeClause::Prescriptiveness,
+ Grainsize::Prescriptiveness,
// clang-format off
MS(Strict, Strict)
// clang-format on
@@ -1274,10 +1304,39 @@ ThreadLimit make(const parser::OmpClause::ThreadLimit &inp,
To make(const parser::OmpClause::To &inp,
semantics::SemanticsContext &semaCtx) {
- // inp.v -> parser::OmpObjectList
- return To{{/*Expectation=*/std::nullopt, /*Mapper=*/std::nullopt,
- /*Iterator=*/std::nullopt,
- /*LocatorList=*/makeObjects(inp.v, semaCtx)}};
+ // inp.v -> parser::OmpToClause
+ using wrapped = parser::OmpToClause;
+
+ CLAUSET_ENUM_CONVERT( //
+ convert, parser::OmpToClause::Expectation, To::Expectation,
+ // clang-format off
+ MS(Present, Present)
+ // clang-format on
+ );
+
+ auto &t0 = std::get<std::optional<std::list<wrapped::Expectation>>>(inp.v.t);
+ auto &t1 =
+ std::get<std::optional<std::list<parser::OmpIteratorModifier>>>(inp.v.t);
+ auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
+
+ assert((!t0 || t0->size() == 1) && "Only one expectation modifier allowed");
+ assert((!t1 || t1->size() == 1) && "Only one iterator modifier allowed");
+
+ auto expectation = [&]() -> std::optional<To::Expectation> {
+ if (t0)
+ return convert(t0->front());
+ return std::nullopt;
+ }();
+
+ auto iterator = [&]() -> std::optional<Iterator> {
+ if (t1)
+ return makeIterator(t1->front(), semaCtx);
+ return std::nullopt;
+ }();
+
+ return To{{/*Expectation=*/std::move(expectation), /*Mapper=*/std::nullopt,
+ /*Iterator=*/std::move(iterator),
+ /*LocatorList=*/makeObjects(t2, semaCtx)}};
}
// UnifiedAddress: empty
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 6fde70fc5c3878..b0d9716b4f93a6 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -122,6 +122,40 @@ template <typename Separator> struct MapModifiers {
const Separator sep_;
};
+// This is almost exactly the same thing as MapModifiers. It has the same
+// issue (it expects modifiers in a specific order), and the fix for that
+// will change how modifiers are parsed. Instead of making this code more
+// generic, make it simple, and generalize after the fix is in place.
+template <typename Separator> struct MotionModifiers {
+ constexpr MotionModifiers(Separator sep) : sep_(sep) {}
+ constexpr MotionModifiers(const MotionModifiers &) = default;
+ constexpr MotionModifiers(MotionModifiers &&) = default;
+
+ // Parsing of mappers if not implemented yet.
+ using ExpParser = Parser<OmpFromClause::Expectation>;
+ using IterParser = Parser<OmpIteratorModifier>;
+ using ModParser = ConcatSeparated<Separator, ExpParser, IterParser>;
+
+ using resultType = typename ModParser::resultType;
+
+ std::optional<resultType> Parse(ParseState &state) const {
+ auto mp = ModParser(sep_, ExpParser{}, IterParser{});
+ auto mods = mp.Parse(state);
+ // The ModParser always "succeeds", i.e. even if the input is junk, it
+ // will return a tuple filled with nullopts. If any of the components
+ // is not a nullopt, expect a ":".
+ if (std::apply([](auto &&...opts) { return (... || !!opts); }, *mods)) {
+ if (!attempt(":"_tok).Parse(state)) {
+ return std::nullopt;
+ }
+ }
+ return std::move(mods);
+ }
+
+private:
+ const Separator sep_;
+};
+
// OpenMP Clauses
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple |
@@ -382,6 +416,31 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
maybe(Parser<OmpIteratorModifier>{} / ","_tok),
Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})))
+TYPE_PARSER(construct<OmpFromClause::Expectation>(
+ "PRESENT" >> pure(OmpFromClause::Expectation::Present)))
+
+template <typename MotionClause, bool CommasEverywhere>
+static inline MotionClause makeMotionClause(
+ std::tuple<std::optional<std::list<typename MotionClause::Expectation>>,
+ std::optional<std::list<OmpIteratorModifier>>> &&mods,
+ OmpObjectList &&objs) {
+ auto &&[exp, iter] = std::move(mods);
+ return MotionClause(
+ std::move(exp), std::move(iter), std::move(objs), CommasEverywhere);
+}
+
+TYPE_PARSER(construct<OmpFromClause>(
+ applyFunction<OmpFromClause>(makeMotionClause<OmpFromClause, true>,
+ MotionModifiers(","_tok), Parser<OmpObjectList>{}) ||
+ applyFunction<OmpFromClause>(makeMotionClause<OmpFromClause, false>,
+ MotionModifiers(maybe(","_tok)), Parser<OmpObjectList>{})))
+
+TYPE_PARSER(construct<OmpToClause>(
+ applyFunction<OmpToClause>(makeMotionClause<OmpToClause, true>,
+ MotionModifiers(","_tok), Parser<OmpObjectList>{}) ||
+ applyFunction<OmpToClause>(makeMotionClause<OmpToClause, false>,
+ MotionModifiers(maybe(","_tok)), Parser<OmpObjectList>{})))
+
// 2.15.3.7 LINEAR (linear-list: linear-step)
// linear-list -> list | modifier(list)
// linear-modifier -> REF | VAL | UVAL
@@ -475,11 +534,11 @@ TYPE_PARSER(
parenthesized(scalarIntExpr))) ||
"FINAL" >> construct<OmpClause>(construct<OmpClause::Final>(
parenthesized(scalarLogicalExpr))) ||
- "FULL" >> construct<OmpClause>(construct<OmpClause::Full>()) ||
"FIRSTPRIVATE" >> construct<OmpClause>(construct<OmpClause::Firstprivate>(
parenthesized(Parser<OmpObjectList>{}))) ||
"FROM" >> construct<OmpClause>(construct<OmpClause::From>(
- parenthesized(Parser<OmpObjectList>{}))) ||
+ parenthesized(Parser<OmpFromClause>{}))) ||
+ "FULL" >> construct<OmpClause>(construct<OmpClause::Full>()) ||
"GRAINSIZE" >> construct<OmpClause>(construct<OmpClause::Grainsize>(
parenthesized(Parser<OmpGrainsizeClause>{}))) ||
"HAS_DEVICE_ADDR" >>
@@ -554,7 +613,7 @@ TYPE_PARSER(
"THREAD_LIMIT" >> construct<OmpClause>(construct<OmpClause::ThreadLimit>(
parenthesized(scalarIntExpr))) ||
"TO" >> construct<OmpClause>(construct<OmpClause::To>(
- parenthesized(Parser<OmpObjectList>{}))) ||
+ parenthesized(Parser<OmpToClause>{}))) ||
"USE_DEVICE_PTR" >> construct<OmpClause>(construct<OmpClause::UseDevicePtr>(
parenthesized(Parser<OmpObjectList>{}))) ||
"USE_DEVICE_ADDR" >>
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 3b0824f80161f4..88b5ac626264b1 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2138,6 +2138,27 @@ class UnparseVisitor {
Put(",");
Walk(std::get<std::optional<ScalarIntConstantExpr>>(x.t));
}
+ void Unparse(const OmpFromClause &x) {
+ auto &expect =
+ std::get<std::optional<std::list<OmpFromClause::Expectation>>>(x.t);
+ auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t);
+ bool needComma{false};
+ if (expect) {
+ Walk(*expect);
+ needComma = true;
+ }
+ if (iter) {
+ if (needComma) {
+ Put(", ");
+ }
+ Walk(*iter);
+ needComma = true;
+ }
+ if (needComma) {
+ Put(": ");
+ }
+ Walk(std::get<OmpObjectList>(x.t));
+ }
void Unparse(const OmpIfClause &x) {
Walk(std::get<std::optional<OmpIfClause::DirectiveNameModifier>>(x.t), ":");
Walk(std::get<ScalarLogicalExpr>(x.t));
@@ -2241,6 +2262,27 @@ class UnparseVisitor {
Walk(":",
std::get<std::optional<OmpDefaultmapClause::VariableCategory>>(x.t));
}
+ void Unparse(const OmpToClause &x) {
+ auto &expect =
+ std::get<std::optional<std::list<OmpToClause::Expectation>>>(x.t);
+ auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t);
+ bool needComma{false};
+ if (expect) {
+ Walk(*expect);
+ needComma = true;
+ }
+ if (iter) {
+ if (needComma) {
+ Put(", ");
+ }
+ Walk(*iter);
+ needComma = true;
+ }
+ if (needComma) {
+ Put(": ");
+ }
+ Walk(std::get<OmpObjectList>(x.t));
+ }
#define GEN_FLANG_CLAUSE_UNPARSE
#include "llvm/Frontend/OpenMP/OMP.inc"
void Unparse(const OmpLoopDirective &x) {
@@ -2858,6 +2900,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
WALK_NESTED_ENUM(
OmpReductionClause, ReductionModifier) // OMP reduction-modifier
+ WALK_NESTED_ENUM(OmpFromClause, Expectation) // OMP motion-expectation
WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
WALK_NESTED_ENUM(OmpOrderClause, Type) // OMP order-type
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index c813100b4b16c8..4f7d0356f5a44d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -8,6 +8,7 @@
#include "check-omp-structure.h"
#include "definable.h"
+#include "flang/Evaluate/check-expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/tools.h"
@@ -268,6 +269,57 @@ bool OmpStructureChecker::IsCloselyNestedRegion(const OmpDirectiveSet &set) {
return false;
}
+namespace {
+struct ContiguousHelper {
+ ContiguousHelper(SemanticsContext &context)
+ : sctx_(context), fctx_(context.foldingContext()) {}
+
+ template <typename Contained>
+ std::optional<bool> Visit(const common::Indirection<Contained> &x) {
+ return Visit(x.value());
+ }
+ template <typename Contained>
+ std::optional<bool> Visit(const common::Reference<Contained> &x) {
+ return Visit(x.get());
+ }
+ template <typename T> std::optional<bool> Visit(const evaluate::Expr<T> &x) {
+ return std::visit([this](auto &&s) { return Visit(s); }, x.u);
+ }
+ template <typename T>
+ std::optional<bool> Visit(const evaluate::Designator<T> &x) {
+ return common::visit(
+ [this](auto &&s) { return evaluate::IsContiguous(s, fctx_); }, x.u);
+ }
+ template <typename T> std::optional<bool> Visit(const T &) {
+ // Everything else.
+ return std::nullopt;
+ }
+
+private:
+ SemanticsContext &sctx_;
+ evaluate::FoldingContext &fctx_;
+};
+} // namespace
+
+std::optional<bool>
+OmpStructureChecker::IsContiguous(const parser::OmpObject &object) {
+ return common::visit(
+ common::visitors{
+ [&](const parser::Name &x) {
+ // Any member of a common block must be contiguous.
+ return std::optional<bool>{true};
+ },
+ [&](const parser::Designator &x) {
+ evaluate::ExpressionAnalyzer ea{context_};
+ if (MaybeExpr maybeExpr{ea.Analyze(x)}) {
+ return ContiguousHelper{context_}.Visit(*maybeExpr);
+ }
+ return std::optional<bool>{};
+ },
+ },
+ object.u);
+}
+
void OmpStructureChecker::CheckMultipleOccurrence(
semantics::UnorderedSymbolSet &listVars,
const std::list<parser::Name> &nameList, const parser::CharBlock &item,
@@ -1466,9 +1518,10 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
common::visitors{
[&](const parser::OmpClause::To &toClause) {
toClauseFound = true;
- CheckSymbolNames(dir.source, toClause.v);
- CheckIsVarPartOfAnotherVar(dir.source, toClause.v);
- CheckThreadprivateOrDeclareTargetVar(toClause.v);
+ auto &objList{std::get<parser::OmpObjectList>(toClause.v.t)};
+ CheckSymbolNames(dir.source, objList);
+ CheckIsVarPartOfAnotherVar(dir.source, objList);
+ CheckThreadprivateOrDeclareTargetVar(objList);
},
[&](const parser::OmpClause::Link &linkClause) {
CheckSymbolNames(dir.source, linkClause.v);
@@ -1647,24 +1700,29 @@ void OmpStructureChecker::CheckOrderedDependClause(
}
void OmpStructureChecker::CheckTargetUpdate() {
- const parser::OmpClause *toClause = FindClause(llvm::omp::Clause::OMPC_to);
- const parser::OmpClause *fromClause =
- FindClause(llvm::omp::Clause::OMPC_from);
- if (!toClause && !fromClause) {
+ const parser::OmpClause *toWrapper{FindClause(llvm::omp::Clause::OMPC_to)};
+ const parser::OmpClause *fromWrapper{
+ FindClause(llvm::omp::Clause::OMPC_from)};
+ if (!toWrapper && !fromWrapper) {
context_.Say(GetContext().directiveSource,
- "At least one motion-clause (TO/FROM) must be specified on TARGET UPDATE construct."_err_en_US);
+ "At least one motion-clause (TO/FROM) must be specified on "
+ "TARGET UPDATE construct."_err_en_US);
}
- if (toClause && fromClause) {
+ if (toWrapper && fromWrapper) {
SymbolSourceMap toSymbols, from...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
kiranchandramohan
left a comment
kiranchandramohan
left a comment
There was a problem hiding this comment.
A few trivial comments.
Comment on lines
1724
to
1725
| "A list item ('%s') can only appear in a TO or FROM clause, " | ||
| "but not in both."_err_en_US, |
Contributor
There was a problem hiding this comment.
If the formatting allows, it is better to have the error in a single line so that you can search for it.
flang/lib/Parser/unparse.cpp
Outdated
Comment on lines
2142
to
2144
| auto &expect = | ||
| std::get<std::optional<std::list<OmpFromClause::Expectation>>>(x.t); | ||
| auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t); |
Contributor
There was a problem hiding this comment.
Nit: braced initialization.
flang/lib/Parser/openmp-parsers.cpp
Outdated
Comment on lines
142
to
143
| auto mp = ModParser(sep_, ExpParser{}, IterParser{}); | ||
| auto mods = mp.Parse(state); |
Contributor
There was a problem hiding this comment.
Nit: Braced initialization.
flang/lib/Parser/unparse.cpp
Outdated
Comment on lines
2266
to
2268
| auto &expect = | ||
| std::get<std::optional<std::list<OmpToClause::Expectation>>>(x.t); | ||
| auto &iter = std::get<std::optional<std::list<OmpIteratorModifier>>>(x.t); |
Contributor
There was a problem hiding this comment.
Nit: braced initialisation
Comment on lines
273
to
302
| struct ContiguousHelper { | ||
| ContiguousHelper(SemanticsContext &context) | ||
| : sctx_(context), fctx_(context.foldingContext()) {} | ||
|
|
||
| template <typename Contained> | ||
| std::optional<bool> Visit(const common::Indirection<Contained> &x) { | ||
| return Visit(x.value()); | ||
| } | ||
| template <typename Contained> | ||
| std::optional<bool> Visit(const common::Reference<Contained> &x) { | ||
| return Visit(x.get()); | ||
| } | ||
| template <typename T> std::optional<bool> Visit(const evaluate::Expr<T> &x) { | ||
| return std::visit([this](auto &&s) { return Visit(s); }, x.u); | ||
| } | ||
| template <typename T> | ||
| std::optional<bool> Visit(const evaluate::Designator<T> &x) { | ||
| return common::visit( | ||
| [this](auto &&s) { return evaluate::IsContiguous(s, fctx_); }, x.u); | ||
| } | ||
| template <typename T> std::optional<bool> Visit(const T &) { | ||
| // Everything else. | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| private: | ||
| SemanticsContext &sctx_; | ||
| evaluate::FoldingContext &fctx_; | ||
| }; | ||
| } // namespace |
Contributor
There was a problem hiding this comment.
Why do we need this? I see that there is IsContiguous that works in check-expression.
Contributor
Author
There was a problem hiding this comment.
It only works for certain classes in evaluator, specifically the ones in variable.h, i.e. the ones representing memory access. This code extracts that from the top-level Expr class, and passes the result to the IsContiguous from check-expresssion.
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/9745 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/11788 Here is the relevant piece of the build log for the reference |
Contributor
|
I've fixed warnings from this PR with 76c1665. I didn't delete |
PhilippRados
pushed a commit
to PhilippRados/llvm-project
that referenced
this pull request
Nov 6, 2024
…llvm#114593) Parse PRESENT modifier as well while we're at it (no MAPPER though). Add semantic checks for these clauses in the TARGET UPDATE construct, TODO messages in lowering.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Parse PRESENT modifier as well while we're at it (no MAPPER though). Add semantic checks for these clauses in the TARGET UPDATE construct, TODO messages in lowering.