From e6c345a76cf2ac2653e04fb07d0ddd0f248908b3 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 14 Oct 2024 09:40:52 -0500 Subject: [PATCH 1/8] [flang][OpenMP] Parse iterators, add to MAP clause, TODO for lowering Define `OmpIteratorSpecifier` and `OmpIteratorModifier` parser classes, and add parsing for them. Those are reusable between any clauses that use iterator modifiers. Add support for iterator modifiers to the MAP clause up to lowering, where a TODO message is emitted. --- flang/include/flang/Parser/dump-parse-tree.h | 2 + flang/include/flang/Parser/parse-tree.h | 31 ++- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 105 +++++---- flang/lib/Lower/OpenMP/Clauses.cpp | 72 +++++- flang/lib/Lower/OpenMP/Clauses.h | 11 +- flang/lib/Parser/openmp-parsers.cpp | 214 +++++++++++++----- flang/lib/Parser/type-parsers.h | 1 + flang/lib/Parser/unparse.cpp | 55 +++-- flang/lib/Semantics/check-omp-structure.cpp | 101 ++++++++- flang/lib/Semantics/check-omp-structure.h | 25 ++ flang/lib/Semantics/resolve-directives.cpp | 7 +- flang/lib/Semantics/resolve-names.cpp | 35 +++ flang/test/Parser/OpenMP/map-modifiers.f90 | 198 +++++++++++++++- flang/test/Semantics/OpenMP/map-modifiers.f90 | 78 ++++++- flang/test/Semantics/OpenMP/symbol08.f90 | 18 +- llvm/include/llvm/Frontend/OpenMP/OMP.h | 3 + llvm/lib/Frontend/OpenMP/OMP.cpp | 14 ++ 17 files changed, 800 insertions(+), 170 deletions(-) diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index ccbe5475d051e..040065c0cbc02 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -474,6 +474,8 @@ class ParseTreeDumper { NODE(parser, NullInit) NODE(parser, ObjectDecl) NODE(parser, OldParameterStmt) + NODE(parser, OmpIteratorSpecifier) + NODE(parser, OmpIteratorModifier) NODE(parser, OmpAlignedClause) NODE(parser, OmpAtomic) NODE(parser, OmpAtomicCapture) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 4a3c992c4ec51..35821288699fd 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3424,7 +3424,17 @@ struct AssignedGotoStmt { WRAPPER_CLASS(PauseStmt, std::optional); -// Parse tree nodes for OpenMP 4.5 directives and clauses +// Parse tree nodes for OpenMP 4.5+ directives and clauses + +// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple +// iterator-modifier -> iterator-specifier-list +struct OmpIteratorSpecifier { + TUPLE_CLASS_BOILERPLATE(OmpIteratorSpecifier); + CharBlock source; + std::tuple t; +}; + +WRAPPER_CLASS(OmpIteratorModifier, std::list); // 2.5 proc-bind-clause -> PROC_BIND (MASTER | CLOSE | SPREAD) struct OmpProcBindClause { @@ -3450,16 +3460,25 @@ struct OmpObject { WRAPPER_CLASS(OmpObjectList, std::list); // 2.15.5.1 map -> -// MAP ([ [map-type-modifiers [,] ] map-type : ] variable-name-list) -// map-type-modifiers -> map-type-modifier [,] [...] +// MAP ([[map-type-modifier-list [,]] [iterator-modifier [,]] map-type : ] +// variable-name-list) +// map-type-modifier-list -> map-type-modifier [,] [...] // map-type-modifier -> ALWAYS | CLOSE | PRESENT | OMPX_HOLD // map-type -> TO | FROM | TOFROM | ALLOC | RELEASE | DELETE struct OmpMapClause { - ENUM_CLASS(TypeModifier, Always, Close, Present, OmpxHold); + ENUM_CLASS(TypeModifier, Always, Close, Present, Ompx_Hold); ENUM_CLASS(Type, To, From, Tofrom, Alloc, Release, Delete) TUPLE_CLASS_BOILERPLATE(OmpMapClause); - std::tuple>, std::optional, - OmpObjectList> + + // All modifiers are parsed into optional lists, even if they are unique. + // The checks for satisfying those constraints are deferred to semantics. + // In OpenMP 5.2 the non-comma syntax has been deprecated: keep the + // information about separator presence to emit a diagnostic if needed. + std::tuple>, + std::optional>, // unique + std::optional>, // unique + OmpObjectList, + bool> // were the modifiers comma-separated t; }; diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 88c443b4198ab..fbc031f3a93d7 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -936,57 +936,64 @@ bool ClauseProcessor::processMap( llvm::SmallVector> parentMemberIndices; - bool clauseFound = findRepeatableClause( - [&](const omp::clause::Map &clause, const parser::CharBlock &source) { - using Map = omp::clause::Map; - mlir::Location clauseLocation = converter.genLocation(source); - const auto &mapType = std::get>(clause.t); - llvm::omp::OpenMPOffloadMappingFlags mapTypeBits = - llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE; - // If the map type is specified, then process it else Tofrom is the - // default. - Map::MapType type = mapType.value_or(Map::MapType::Tofrom); - switch (type) { - case Map::MapType::To: - mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO; - break; - case Map::MapType::From: - mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; - break; - case Map::MapType::Tofrom: - mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO | - llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; - break; - case Map::MapType::Alloc: - case Map::MapType::Release: - // alloc and release is the default map_type for the Target Data - // Ops, i.e. if no bits for map_type is supplied then alloc/release - // is implicitly assumed based on the target directive. Default - // value for Target Data and Enter Data is alloc and for Exit Data - // it is release. - break; - case Map::MapType::Delete: - mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE; - } + auto process = [&](const omp::clause::Map &clause, + const parser::CharBlock &source) { + using Map = omp::clause::Map; + mlir::Location clauseLocation = converter.genLocation(source); + const auto &mapType = std::get>(clause.t); + llvm::omp::OpenMPOffloadMappingFlags mapTypeBits = + llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE; + // If the map type is specified, then process it else Tofrom is the + // default. + Map::MapType type = mapType.value_or(Map::MapType::Tofrom); + switch (type) { + case Map::MapType::To: + mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO; + break; + case Map::MapType::From: + mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; + break; + case Map::MapType::Tofrom: + mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO | + llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; + break; + case Map::MapType::Alloc: + case Map::MapType::Release: + // alloc and release is the default map_type for the Target Data + // Ops, i.e. if no bits for map_type is supplied then alloc/release + // is implicitly assumed based on the target directive. Default + // value for Target Data and Enter Data is alloc and for Exit Data + // it is release. + break; + case Map::MapType::Delete: + mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE; + } - auto &modTypeMods = - std::get>(clause.t); - if (modTypeMods) { - if (llvm::is_contained(*modTypeMods, Map::MapTypeModifier::Always)) - mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS; - // Diagnose unimplemented map-type-modifiers. - if (llvm::any_of(*modTypeMods, [](Map::MapTypeModifier m) { - return m != Map::MapTypeModifier::Always; - })) { - TODO(currentLocation, "Map type modifiers (other than 'ALWAYS')" - " are not implemented yet"); - } - } - processMapObjects(stmtCtx, clauseLocation, - std::get(clause.t), mapTypeBits, - parentMemberIndices, result.mapVars, *ptrMapSyms); - }); + auto &modTypeMods = + std::get>(clause.t); + if (modTypeMods) { + if (llvm::is_contained(*modTypeMods, Map::MapTypeModifier::Always)) + mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS; + // Diagnose unimplemented map-type-modifiers. + if (llvm::any_of(*modTypeMods, [](Map::MapTypeModifier m) { + return m != Map::MapTypeModifier::Always; + })) { + TODO(currentLocation, "Map type modifiers (other than 'ALWAYS')" + " are not implemented yet"); + } + } + + if (std::get>(clause.t)) { + TODO(currentLocation, + "Support for iterator modifiers is not implemented yet"); + } + + processMapObjects(stmtCtx, clauseLocation, + std::get(clause.t), mapTypeBits, + parentMemberIndices, result.mapVars, *ptrMapSyms); + }; + bool clauseFound = findRepeatableClause(process); insertChildMapInfoIntoParent(converter, parentMemberIndices, result.mapVars, *ptrMapSyms); diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 64d661256a187..8974b4211b968 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -232,6 +232,46 @@ MAKE_INCOMPLETE_CLASS(Match, Match); // MAKE_INCOMPLETE_CLASS(Otherwise, ); // missing-in-parser MAKE_INCOMPLETE_CLASS(When, When); +List +makeIteratorSpecifiers(const parser::OmpIteratorSpecifier &inp, + semantics::SemanticsContext &semaCtx) { + List specifiers; + + auto &[begin, end, step] = std::get(inp.t).t; + assert(begin && end && "Expecting begin/end values"); + evaluate::ExpressionAnalyzer ea{semaCtx}; + + MaybeExpr rbegin{ea.Analyze(*begin)}, rend{ea.Analyze(*end)}; + MaybeExpr rstep; + if (step) + rstep = ea.Analyze(*step); + + assert(rbegin && rend && "Unable to get range bounds"); + Range range{{*rbegin, *rend, rstep}}; + + auto &tds = std::get(inp.t); + auto &entities = std::get>(tds.t); + for (const parser::EntityDecl &ed : entities) { + auto &name = std::get(ed.t); + assert(name.symbol && "Expecting symbol for iterator variable"); + auto *stype = name.symbol->GetType(); + assert(stype && "Expecting symbol type"); + IteratorSpecifier spec{{evaluate::DynamicType::From(*stype), + makeObject(name, semaCtx), range}}; + specifiers.emplace_back(std::move(spec)); + } + + return specifiers; +} + +Iterator makeIterator(const parser::OmpIteratorModifier &inp, + semantics::SemanticsContext &semaCtx) { + Iterator iterator; + for (auto &&spec : inp.v) + llvm::append_range(iterator, makeIteratorSpecifiers(spec, semaCtx)); + return iterator; +} + DefinedOperator makeDefinedOperator(const parser::DefinedOperator &inp, semantics::SemanticsContext &semaCtx) { CLAUSET_ENUM_CONVERT( // @@ -843,18 +883,32 @@ Map make(const parser::OmpClause::Map &inp, CLAUSET_ENUM_CONVERT( // convert2, parser::OmpMapClause::TypeModifier, Map::MapTypeModifier, // clang-format off - MS(Always, Always) - MS(Close, Close) - MS(OmpxHold, OmpxHold) - MS(Present, Present) + MS(Always, Always) + MS(Close, Close) + MS(Ompx_Hold, OmpxHold) + MS(Present, Present) // clang-format on ); auto &t0 = std::get>>(inp.v.t); - auto &t1 = std::get>(inp.v.t); - auto &t2 = std::get(inp.v.t); + auto &t1 = + std::get>>(inp.v.t); + auto &t2 = std::get>>(inp.v.t); + auto &t3 = std::get(inp.v.t); + + // These should have been diagnosed already. + assert((!t1 || t1->size() == 1) && "Only one iterator modifier is allowed"); + assert((!t2 || t2->size() == 1) && "Only one map type is allowed"); + + auto iterator = [&]() -> std::optional { + if (t1) + return makeIterator(t1->front(), semaCtx); + return std::nullopt; + }(); - std::optional maybeType = maybeApply(convert1, t1); + std::optional maybeType; + if (t2) + maybeType = maybeApply(convert1, std::optional(t2->front())); std::optional maybeTypeMods = maybeApply( [&](const std::list &typeMods) { @@ -867,8 +921,8 @@ Map make(const parser::OmpClause::Map &inp, return Map{{/*MapType=*/maybeType, /*MapTypeModifiers=*/maybeTypeMods, - /*Mapper=*/std::nullopt, /*Iterator=*/std::nullopt, - /*LocatorList=*/makeObjects(t2, semaCtx)}}; + /*Mapper=*/std::nullopt, /*Iterator=*/std::move(iterator), + /*LocatorList=*/makeObjects(t3, semaCtx)}}; } // Match: incomplete diff --git a/flang/lib/Lower/OpenMP/Clauses.h b/flang/lib/Lower/OpenMP/Clauses.h index 62f3df3e3ee95..1e911a2046857 100644 --- a/flang/lib/Lower/OpenMP/Clauses.h +++ b/flang/lib/Lower/OpenMP/Clauses.h @@ -9,6 +9,7 @@ #define FORTRAN_LOWER_OPENMP_CLAUSES_H #include "flang/Evaluate/expression.h" +#include "flang/Evaluate/type.h" #include "flang/Parser/parse-tree.h" #include "flang/Semantics/expression.h" #include "flang/Semantics/semantics.h" @@ -29,12 +30,7 @@ namespace Fortran::lower::omp { using namespace Fortran; using SomeExpr = semantics::SomeExpr; using MaybeExpr = semantics::MaybeExpr; - -// evaluate::SomeType doesn't provide == operation. It's not really used in -// flang's clauses so far, so a trivial implementation is sufficient. -struct TypeTy : public evaluate::SomeType { - bool operator==(const TypeTy &t) const { return true; } -}; +using TypeTy = evaluate::DynamicType; template struct IdTyTemplate { @@ -150,6 +146,9 @@ std::optional getBaseObject(const Object &object, semantics::SemanticsContext &semaCtx); namespace clause { +using Range = tomp::type::RangeT; +using Iterator = tomp::type::IteratorT; +using IteratorSpecifier = tomp::type::IteratorSpecifierT; using DefinedOperator = tomp::type::DefinedOperatorT; using ProcedureDesignator = tomp::type::ProcedureDesignatorT; using ReductionOperator = tomp::type::ReductionIdentifierT; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 52c7529369dfb..26ee1653cc1d4 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -23,71 +23,162 @@ namespace Fortran::parser { constexpr auto startOmpLine = skipStuffBeforeStatement >> "!$OMP "_sptok; constexpr auto endOmpLine = space >> endOfLine; -// Map modifiers come from two categories: map-type-modifier and map-type. -// There can be zero or more map-type-modifiers, and zero or one map-type. +// Helper class to deal with a list of modifiers of various types. +// The list (to be parsed) is assumed to start with all modifiers of the +// first type, followed by a list of modifiers of the second type, etc. +// Each list can be empty, e.g. +// mod_of_kind_2, mod_of_kind_3, mod_of_kind_5, ... +// The result type is a tuple of optional lists of each modifier type. +template +struct ConcatSeparated { + template + using OptListOf = std::optional>; + template using TupleFor = std::tuple>; + + using resultType = std::tuple, OptListOf...>; + + constexpr ConcatSeparated(ConcatSeparated &&) = default; + constexpr ConcatSeparated(const ConcatSeparated &) = default; + constexpr ConcatSeparated(Separator sep, Parser p, Parsers... ps) + : parser_(p), sepAndParsers_(sep, ps...) {} + + std::optional Parse(ParseState &state) const { + // firstParser is a list parser, it returns optional. + auto firstParser = + attempt(nonemptySeparated(parser_, std::get<0>(sepAndParsers_))); + + if constexpr (sizeof...(Parsers) == 0) { + return TupleFor{std::move(firstParser.Parse(state))}; + } else { + using restParserType = ConcatSeparated; + auto restParser = std::make_from_tuple(sepAndParsers_); + + if (auto first{firstParser.Parse(state)}) { + if (attempt(std::get<0>(sepAndParsers_)).Parse(state)) { + return std::tuple_cat(TupleFor(std::move(*first)), + std::move(*restParser.Parse(state))); + } + return std::tuple_cat(TupleFor{std::move(*first)}, + std::tuple...>{}); + } + return std::tuple_cat(TupleFor{}, + std::move(*restParser.Parse(state))); + } + } + +private: + const Parser parser_; + const std::tuple sepAndParsers_; +}; + +// Map modifiers come from four categories: +// - map-type-modifier, +// - mapper (not parsed yet), +// - iterator, +// - map-type. +// There can be zero or more map-type-modifiers, and zero or one modifier +// of every other kind. // Syntax-wise they look like a single list, where the last element could // be a map-type, and all elements in that list are comma-separated[1]. // Only if there was at least one modifier (of any kind) specified, the // list must end with ":". -// [1] Any of the commas are optional, but that syntax has been deprecated, -// and the parsing code is intended to identify that. There are complications -// coming from the fact that the comma separating the two kinds of modifiers -// is only allowed if there is at least one modifier of each kind. -// The MapModifiers parser parses the modifier list as a whole, and returns -// a tuple with the (optional) map-type-modifier list, and the (optional) -// type modifier as its members. -// The list is then parsed, first with a mandatory separator, and if that -// fails, with an optional one. If the latter succeeds, a deprecation -// message is printed. -template struct MapModifiers { - constexpr MapModifiers( - Separator sep, std::optional msg = std::nullopt) - : sep_(sep), msg_(msg) {} +// There are complications coming from the fact that the comma separating the +// two kinds of modifiers is only allowed if there is at least one modifier of +// each kind. The MapModifiers parser utilizes the ConcatSeparated parser, which +// takes care of that. ConcatSeparated returns a tuple with optional lists of +// modifiers for every type. +// [1] Any of the commas are optional, but that syntax has been deprecated +// in OpenMP 5.2, and the parsing code keeps a record of whether the commas +// were present. +template +struct MapModifiers { + constexpr MapModifiers(Separator sep) : sep_(sep) {} constexpr MapModifiers(const MapModifiers &) = default; constexpr MapModifiers(MapModifiers &&) = default; - using resultType = - std::tuple>, - std::optional>; + // Parsing of mappers is not supported yet. + using TypeModParser = Parser; + using IterParser = Parser; + using TypeParser = Parser; + using ModParser = + ConcatSeparated; + + using resultType = typename ModParser::resultType; std::optional Parse(ParseState &state) const { - auto pmod{Parser{}}; - auto ptype{Parser{}}; - auto startLoc{state.GetLocation()}; - - auto &&[mods, type] = [&]() -> resultType { - // The 'maybe' will return optional>, and the outer - // optional will never be nullopt. - if (auto mods{ - *maybe(attempt(nonemptySeparated(pmod, sep_))).Parse(state)}) { - // mods = optional, and the list is nonempty. - return attempt(sep_).Parse(state) - ? resultType(mods, *maybe(attempt(ptype)).Parse(state)) - : resultType(mods, std::nullopt); + auto mp = ModParser(sep_, TypeModParser{}, IterParser{}, TypeParser{}); + 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::nullopt, *maybe(attempt(ptype)).Parse(state)}; - }(); - auto endLoc{state.GetLocation()}; - - // The above always "succeeds", i.e. even if the input is junk, it will - // return a tuple with two nullopts. If any of the components is not a - // nullopt, expect a ":". - if ((mods.has_value() || type.has_value()) && - !attempt(":"_tok).Parse(state)) { - return std::nullopt; - } - if (msg_) { - state.Say(CharBlock{startLoc, endLoc}, *msg_); } - return resultType(mods, type); + return std::move(mods); } private: const Separator sep_; - std::optional msg_; }; // OpenMP Clauses + +// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple | +// identifier = subscript-triple +// [5.0:47:17-18] In an iterator-specifier, if the iterator-type is not +// specified then the type of that iterator is default integer. +// [5.0:49:14] The iterator-type must be an integer type. +static std::list makeEntityList(std::list &&names) { + std::list entities; + + for (auto iter = names.begin(), end = names.end(); iter != end; ++iter) { + EntityDecl entityDecl( + /*ObjectName=*/std::move(*iter), std::optional{}, + std::optional{}, std::optional{}, + std::optional{}); + entities.push_back(std::move(entityDecl)); + } + return entities; +} + +static TypeDeclarationStmt makeIterSpecDecl(DeclarationTypeSpec &&spec, + std::list &&names) { + return TypeDeclarationStmt(std::move(spec), std::list{}, + makeEntityList(std::move(names))); +} + +static TypeDeclarationStmt makeIterSpecDecl(std::list &&names) { + // Assume INTEGER without kind selector. + DeclarationTypeSpec typeSpec( + IntrinsicTypeSpec{IntegerTypeSpec{std::nullopt}}); + + return TypeDeclarationStmt(std::move(typeSpec), std::list{}, + makeEntityList(std::move(names))); +} + +TYPE_PARSER(construct( + // Using Parser or Parser has the problem + // that they will attempt to treat what follows the '=' as initialization. + // There are several issues with that, + // 1. integer :: i = 0:10 will be parsed as "integer :: i = 0", followed + // by triplet ":10". + // 2. integer :: j = i:10 will be flagged as an error because the + // initializer 'i' must be constant (in declarations). In an iterator + // specifier the 'j' is not an initializer and can be a variable. + (applyFunction( + makeIterSpecDecl, Parser{} / maybe("::"_tok), + nonemptyList(Parser{}) / "="_tok) || + applyFunction( + makeIterSpecDecl, nonemptyList(Parser{}) / "="_tok)), + subscriptTriplet)) + +// [5.0] 2.1.6 iterator -> iterator-specifier-list +TYPE_PARSER(construct( + "ITERATOR" >> + parenthesized(nonemptyList(sourced(Parser{}))))) + // 2.15.3.1 DEFAULT (PRIVATE | FIRSTPRIVATE | SHARED | NONE) TYPE_PARSER(construct( "PRIVATE" >> pure(OmpDefaultClause::Type::Private) || @@ -110,7 +201,7 @@ TYPE_PARSER(construct( TYPE_PARSER(construct( "ALWAYS" >> pure(OmpMapClause::TypeModifier::Always) || "CLOSE" >> pure(OmpMapClause::TypeModifier::Close) || - "OMPX_HOLD" >> pure(OmpMapClause::TypeModifier::OmpxHold) || + "OMPX_HOLD" >> pure(OmpMapClause::TypeModifier::Ompx_Hold) || "PRESENT" >> pure(OmpMapClause::TypeModifier::Present))) TYPE_PARSER( @@ -121,20 +212,23 @@ TYPE_PARSER( "TO"_id >> pure(OmpMapClause::Type::To) || "TOFROM" >> pure(OmpMapClause::Type::Tofrom))) -static inline OmpMapClause makeMapClause( - std::tuple>, - std::optional> &&mod, - OmpObjectList &&obj) { - return OmpMapClause{ - std::move(std::get<0>(mod)), std::move(std::get<1>(mod)), std::move(obj)}; +template +static inline OmpMapClause +makeMapClause(std::tuple>, + std::optional>, + std::optional>> &&mods, + OmpObjectList &&objs) { + auto &&[tm, it, ty] = std::move(mods); + return OmpMapClause{std::move(tm), std::move(it), std::move(ty), + std::move(objs), CommasEverywhere}; } -TYPE_PARSER(construct(applyFunction(makeMapClause, - (MapModifiers(","_tok) || - MapModifiers(maybe(","_tok), - "the specification of modifiers without comma separators for the " - "'MAP' clause has been deprecated"_port_en_US)), - Parser{}))) +TYPE_PARSER(construct( + applyFunction(makeMapClause, MapModifiers(","_tok), + Parser{}) || + applyFunction(makeMapClause, + MapModifiers(maybe(","_tok)), + Parser{}))) // [OpenMP 5.0] // 2.19.7.2 defaultmap(implicit-behavior[:variable-category]) diff --git a/flang/lib/Parser/type-parsers.h b/flang/lib/Parser/type-parsers.h index da7d017d59768..adbf6d23cbd99 100644 --- a/flang/lib/Parser/type-parsers.h +++ b/flang/lib/Parser/type-parsers.h @@ -85,6 +85,7 @@ constexpr Parser variable; // R902 constexpr Parser substring; // R908 constexpr Parser dataRef; // R911, R914, R917 constexpr Parser structureComponent; // R913 +constexpr Parser subscriptTriplet; // R921 constexpr Parser allocateStmt; // R927 constexpr Parser statVariable; // R929 constexpr Parser statOrErrmsg; // R942 & R1165 diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index d1011fe58a026..1839a276c2490 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2067,6 +2067,16 @@ class UnparseVisitor { }, x.u); } + void Unparse(const OmpIteratorSpecifier &x) { + Walk(std::get(x.t)); + Put(" = "); + Walk(std::get(x.t)); + } + void Unparse(const OmpIteratorModifier &x) { + Word("ITERATOR("); + Walk(x.v); + Put(")"); + } void Unparse(const OmpLastprivateClause &x) { Walk( std::get>(x.t), @@ -2076,24 +2086,40 @@ class UnparseVisitor { void Unparse(const OmpMapClause &x) { auto &typeMod = std::get>>(x.t); - auto &type = std::get>(x.t); - Walk(typeMod); - if (typeMod.has_value() && type.has_value()) { - Put(", "); - } - Walk(type); - if (typeMod.has_value() || type.has_value()) { + auto &iter = std::get>>(x.t); + auto &type = std::get>>(x.t); + + // For a given list of items, if the item has a value, then walk it. + // Print commas between items that have values. + // Return 'true' if something did get printed, otherwise 'false'. + auto join = [&](bool comma, auto &&cont, auto &&item, + auto &&...items) -> bool { + // comma: if something needs to be walked (i.e. printed), precede it + // with a comma. + if (!item.has_value()) { + if constexpr (sizeof...(items) != 0) { + return cont(comma, cont, items...); + } else { + return false; + } + } else { + if (comma) { + Put(", "); + } + Walk(*item); + if constexpr (sizeof...(items) != 0) { + return cont(true, cont, items...) || true; + } else { + return true; + } + } + }; + + if (join(false, join, typeMod, iter, type)) { Put(": "); } Walk(std::get(x.t)); } - void Unparse(const OmpMapClause::TypeModifier &x) { - if (x == OmpMapClause::TypeModifier::OmpxHold) { - Word("OMPX_HOLD"); - } else { - Word(OmpMapClause::EnumToString(x)); - } - } void Unparse(const OmpScheduleModifier &x) { Walk(std::get(x.t)); Walk(",", std::get>(x.t)); @@ -2801,6 +2827,7 @@ class UnparseVisitor { WALK_NESTED_ENUM(OmpOrderClause, Type) // OMP order-type WALK_NESTED_ENUM(OmpOrderModifier, Kind) // OMP order-modifier WALK_NESTED_ENUM(OmpMapClause, Type) // OMP map-type + WALK_NESTED_ENUM(OmpMapClause, TypeModifier) // OMP map-type-modifier #undef WALK_NESTED_ENUM void Unparse(const ReductionOperator::Operator x) { switch (x) { diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 461a99f59e4ce..68da0669c1296 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 "definable.h" #include "flang/Parser/parse-tree.h" +#include "flang/Semantics/expression.h" #include "flang/Semantics/tools.h" namespace Fortran::semantics { @@ -556,6 +557,67 @@ void OmpStructureChecker::SetLoopInfo(const parser::OpenMPLoopConstruct &x) { } } +void OmpStructureChecker::CheckIteratorRange( + const parser::OmpIteratorSpecifier &x) { + // Check: + // 1. Whether begin/end are present. + // 2. Whether the step value is non-zero. + // 3. If the step has a known sign, whether the lower/upper bounds form + // a proper interval. + const auto &[begin, end, step]{std::get(x.t).t}; + if (!begin || !end) { + context_.Say( + x.source, + "The begin and end expressions in iterator range-specification are " + "mandatory"_err_en_US); + } + // [5.2:67:19] In a range-specification, if the step is not specified its + // value is implicitly defined to be 1. + if (auto stepv{step ? GetIntValue(*step) : std::optional{1}}) { + if (*stepv == 0) { + context_.Say(x.source, + "The step value in the iterator range is 0"_warn_en_US); + } else if (begin && end) { + std::optional beginv{GetIntValue(*begin)}; + std::optional endv{GetIntValue(*end)}; + if (beginv && endv) { + if (*stepv > 0 && *beginv > *endv) { + context_.Say(x.source, + "The begin value is greater than the end value in iterator " + "range-specification with a positive step"_warn_en_US); + } else if (*stepv < 0 && *beginv < *endv) { + context_.Say(x.source, + "The begin value is less than the end value in iterator " + "range-specification with a negative step"_warn_en_US); + } + } + } + } +} + +void OmpStructureChecker::CheckIteratorModifier( + const parser::OmpIteratorModifier &x) { + // Check if all iterator variables have integer type. + for (auto &&iterSpec : x.v) { + bool isInteger{true}; + auto &typeDecl{std::get(iterSpec.t)}; + auto &typeSpec{std::get(typeDecl.t)}; + if (!std::holds_alternative(typeSpec.u)) { + isInteger = false; + } else { + auto &intrinType{std::get(typeSpec.u)}; + if (!std::holds_alternative(intrinType.u)) { + isInteger = false; + } + } + if (!isInteger) { + context_.Say(iterSpec.source, + "The iterator variable must be of integer type"_err_en_US); + } + CheckIteratorRange(iterSpec); + } +} + void OmpStructureChecker::CheckLoopItrVariableIsInt( const parser::OpenMPLoopConstruct &x) { if (const auto &loopConstruct{ @@ -3073,9 +3135,40 @@ void OmpStructureChecker::CheckAllowedMapTypes( void OmpStructureChecker::Enter(const parser::OmpClause::Map &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_map); + using TypeMod = parser::OmpMapClause::TypeModifier; using Type = parser::OmpMapClause::Type; + using IterMod = parser::OmpIteratorModifier; + + unsigned version{context_.langOptions().OpenMPVersion}; + if (auto commas{std::get(x.v.t)}; !commas && version >= 52) { + context_.Say(GetContext().clauseSource, + "The specification of modifiers without comma separators for the " + "'MAP' clause has been deprecated in OpenMP 5.2"_port_en_US); + } + if (auto &mapTypeMod{std::get>>(x.v.t)}) { + if (auto *dup{FindDuplicateEntry(*mapTypeMod)}) { + context_.Say(GetContext().clauseSource, + "Duplicate map-type-modifier entry '%s' will be ignored"_warn_en_US, + parser::ToUpperCaseLetters(parser::OmpMapClause::EnumToString(*dup))); + } + } + // The size of any of the optional lists is never 0, instead of the list + // being empty, it will be a nullopt. + if (auto &iterMod{std::get>>(x.v.t)}) { + if (iterMod->size() != 1) { + context_.Say(GetContext().clauseSource, + "Only one iterator-modifier is allowed"_err_en_US); + } + CheckIteratorModifier(iterMod->front()); + } + if (auto &mapType{std::get>>(x.v.t)}) { + if (mapType->size() != 1) { + context_.Say(GetContext().clauseSource, + "Multiple map types are not allowed"_err_en_US); + return; + } + parser::OmpMapClause::Type type{mapType->front()}; - if (const auto &mapType{std::get>(x.v.t)}) { switch (GetContext().directive) { case llvm::omp::Directive::OMPD_target: case llvm::omp::Directive::OMPD_target_teams: @@ -3085,13 +3178,13 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Map &x) { case llvm::omp::Directive::OMPD_target_teams_distribute_parallel_do_simd: case llvm::omp::Directive::OMPD_target_data: CheckAllowedMapTypes( - *mapType, {Type::To, Type::From, Type::Tofrom, Type::Alloc}); + type, {Type::To, Type::From, Type::Tofrom, Type::Alloc}); break; case llvm::omp::Directive::OMPD_target_enter_data: - CheckAllowedMapTypes(*mapType, {Type::To, Type::Alloc}); + CheckAllowedMapTypes(type, {Type::To, Type::Alloc}); break; case llvm::omp::Directive::OMPD_target_exit_data: - CheckAllowedMapTypes(*mapType, {Type::From, Type::Release, Type::Delete}); + 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 70a7779ae1fa2..237569bc40c48 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -153,6 +153,7 @@ class OmpStructureChecker const parser::OmpScheduleModifierType::ModType &); void CheckAllowedMapTypes(const parser::OmpMapClause::Type &, const std::list &); + template const T *FindDuplicateEntry(const std::list &); llvm::StringRef getClauseName(llvm::omp::Clause clause) override; llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override; @@ -181,6 +182,8 @@ class OmpStructureChecker bool CheckTargetBlockOnlyTeams(const parser::Block &); void CheckWorkshareBlockStmts(const parser::Block &, parser::CharBlock); + void CheckIteratorRange(const parser::OmpIteratorSpecifier &x); + void CheckIteratorModifier(const parser::OmpIteratorModifier &x); void CheckLoopItrVariableIsInt(const parser::OpenMPLoopConstruct &x); void CheckDoWhile(const parser::OpenMPLoopConstruct &x); void CheckAssociatedLoopConstraints(const parser::OpenMPLoopConstruct &x); @@ -245,5 +248,27 @@ class OmpStructureChecker SymbolSourceMap deferredNonVariables_; }; + +template +const T *OmpStructureChecker::FindDuplicateEntry(const std::list &list) { + // Add elements of the list to a set. If the insertion fails, return + // the address of the failing element. + + // The objects of type T may not be copyable, so add their addresses + // to the set. The set will need to compare the actual objects, so + // the custom comparator is provided. + struct less { + bool operator()(const T *a, const T *b) const { return *a < *b; } + }; + std::set uniq; + + for (const T &item : list) { + if (!uniq.insert(&item).second) { + return &item; + } + } + return nullptr; +} + } // namespace Fortran::semantics #endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_ diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 186b58bcc52c3..490d802cddf42 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -591,9 +591,12 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { void Post(const parser::OmpMapClause &x) { Symbol::Flag ompFlag = Symbol::Flag::OmpMapToFrom; + // There is only one `type' allowed, but it's parsed as a list. Multiple + // types are diagnosed in the semantic checks for OpenMP. if (const auto &mapType{ - std::get>(x.t)}) { - switch (*mapType) { + std::get>>( + x.t)}) { + switch (mapType->front()) { case parser::OmpMapClause::Type::To: ompFlag = Symbol::Flag::OmpMapTo; break; diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 030dbc5ea0f02..f9713fcfc58ed 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1432,6 +1432,7 @@ class OmpVisitor : public virtual DeclarationVisitor { void AddOmpSourceRange(const parser::CharBlock &); static bool NeedsScope(const parser::OpenMPBlockConstruct &); + static bool NeedsScope(const parser::OmpClause &); bool Pre(const parser::OpenMPRequiresConstruct &x) { AddOmpSourceRange(x.source); @@ -1547,6 +1548,17 @@ class OmpVisitor : public virtual DeclarationVisitor { void Post(const parser::OpenMPAtomicConstruct &) { messageHandler().set_currStmtSource(std::nullopt); } + bool Pre(const parser::OmpClause &x) { + if (NeedsScope(x)) { + PushScope(Scope::Kind::OtherConstruct, nullptr); + } + return true; + } + void Post(const parser::OmpClause &x) { + if (NeedsScope(x)) { + PopScope(); + } + } }; bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) { @@ -1562,6 +1574,12 @@ bool OmpVisitor::NeedsScope(const parser::OpenMPBlockConstruct &x) { } } +bool OmpVisitor::NeedsScope(const parser::OmpClause &x) { + // Iterators contain declarations, whose scope extends until the end + // the clause. + return llvm::omp::canHaveIterator(x.Id()); +} + void OmpVisitor::AddOmpSourceRange(const parser::CharBlock &source) { messageHandler().set_currStmtSource(source); currScope().AddSourceRange(source); @@ -7698,6 +7716,23 @@ class ExecutionPartSkimmerBase { return true; } + // Iterator-modifiers contain variable declarations, and do introduce + // a new scope. These variables can only have integer types, and their + // scope only extends until the end of the clause. A potential alternative + // to the code below may be to ignore OpenMP clauses, but it's not clear + // if OMP-specific checks can be avoided altogether. + bool Pre(const parser::OmpClause &x) { + if (OmpVisitor::NeedsScope(x)) { + PushScope(); + } + return true; + } + void Post(const parser::OmpClause &x) { + if (OmpVisitor::NeedsScope(x)) { + PopScope(); + } + } + protected: bool IsHidden(SourceName name) { for (const auto &scope : nestedScopes_) { diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90 index 4c6dcde5a20c5..0c95f21c5e6a5 100644 --- a/flang/test/Parser/OpenMP/map-modifiers.f90 +++ b/flang/test/Parser/OpenMP/map-modifiers.f90 @@ -18,12 +18,13 @@ subroutine f00(x) !PARSE-TREE: OmpBeginBlockDirective !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause -!PARSE-TREE: | | TypeModifier = OmpxHold +!PARSE-TREE: | | TypeModifier = Ompx_Hold !PARSE-TREE: | | TypeModifier = Always !PARSE-TREE: | | TypeModifier = Present !PARSE-TREE: | | TypeModifier = Close !PARSE-TREE: | | Type = To !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'true' subroutine f01(x) integer :: x @@ -42,11 +43,12 @@ subroutine f01(x) !PARSE-TREE: OmpBeginBlockDirective !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause -!PARSE-TREE: | | TypeModifier = OmpxHold +!PARSE-TREE: | | TypeModifier = Ompx_Hold !PARSE-TREE: | | TypeModifier = Always !PARSE-TREE: | | TypeModifier = Present !PARSE-TREE: | | TypeModifier = Close !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'true' subroutine f02(x) integer :: x @@ -67,6 +69,7 @@ subroutine f02(x) !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause !PARSE-TREE: | | Type = From !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'true' subroutine f03(x) integer :: x @@ -86,15 +89,16 @@ subroutine f03(x) !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'true' -subroutine f10(x) +subroutine f04(x) integer :: x !$omp target map(ompx_hold always, present, close, to: x) x = x + 1 !$omp end target end -!UNPARSE: SUBROUTINE f10 (x) +!UNPARSE: SUBROUTINE f04 (x) !UNPARSE: INTEGER x !UNPARSE: !$OMP TARGET MAP(OMPX_HOLD, ALWAYS, PRESENT, CLOSE, TO: x) !UNPARSE: x=x+1_4 @@ -104,21 +108,22 @@ subroutine f10(x) !PARSE-TREE: OmpBeginBlockDirective !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause -!PARSE-TREE: | | TypeModifier = OmpxHold +!PARSE-TREE: | | TypeModifier = Ompx_Hold !PARSE-TREE: | | TypeModifier = Always !PARSE-TREE: | | TypeModifier = Present !PARSE-TREE: | | TypeModifier = Close !PARSE-TREE: | | Type = To !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'false' -subroutine f11(x) +subroutine f05(x) integer :: x !$omp target map(ompx_hold, always, present, close: x) x = x + 1 !$omp end target end -!UNPARSE: SUBROUTINE f11 (x) +!UNPARSE: SUBROUTINE f05 (x) !UNPARSE: INTEGER x !UNPARSE: !$OMP TARGET MAP(OMPX_HOLD, ALWAYS, PRESENT, CLOSE: x) !UNPARSE: x=x+1_4 @@ -128,9 +133,186 @@ subroutine f11(x) !PARSE-TREE: OmpBeginBlockDirective !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target !PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause -!PARSE-TREE: | | TypeModifier = OmpxHold +!PARSE-TREE: | | TypeModifier = Ompx_Hold !PARSE-TREE: | | TypeModifier = Always !PARSE-TREE: | | TypeModifier = Present !PARSE-TREE: | | TypeModifier = Close !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'true' + +subroutine f10(x) + integer :: x(10) + !$omp target map(present, iterator(integer :: i = 1:10), to: x(i)) + x = x + 1 + !$omp end target +end + +!UNPARSE: SUBROUTINE f10 (x) +!UNPARSE: INTEGER x(10_4) +!UNPARSE: !$OMP TARGET MAP(PRESENT, ITERATOR(INTEGER i = 1_4:10_4), TO: x(i)) +!UNPARSE: x=x+1_4 +!UNPARSE: !$OMP END TARGET +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginBlockDirective +!PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | TypeModifier = Present +!PARSE-TREE: | | OmpIteratorModifier -> OmpIteratorSpecifier +!PARSE-TREE: | | | TypeDeclarationStmt +!PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'i' +!PARSE-TREE: | | | SubscriptTriplet +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '1_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '1' +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '10_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '10' +!PARSE-TREE: | | Type = To +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> ArrayElement +!PARSE-TREE: | | | DataRef -> Name = 'x' +!PARSE-TREE: | | | SectionSubscript -> Integer -> Expr = 'i' +!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | bool = 'true' + +subroutine f11(x) + integer :: x(10) + !$omp target map(present, iterator(i = 1:10), to: x(i)) + x = x + 1 + !$omp end target +end + +!UNPARSE: SUBROUTINE f11 (x) +!UNPARSE: INTEGER x(10_4) +!UNPARSE: !$OMP TARGET MAP(PRESENT, ITERATOR(INTEGER i = 1_4:10_4), TO: x(i)) +!UNPARSE: x=x+1_4 +!UNPARSE: !$OMP END TARGET +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginBlockDirective +!PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | TypeModifier = Present +!PARSE-TREE: | | OmpIteratorModifier -> OmpIteratorSpecifier +!PARSE-TREE: | | | TypeDeclarationStmt +!PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'i' +!PARSE-TREE: | | | SubscriptTriplet +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '1_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '1' +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '10_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '10' +!PARSE-TREE: | | Type = To +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> ArrayElement +!PARSE-TREE: | | | DataRef -> Name = 'x' +!PARSE-TREE: | | | SectionSubscript -> Integer -> Expr = 'i' +!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | bool = 'true' + +subroutine f12(x) + integer :: x(10) + !$omp target map(present, iterator(i = 1:10, integer :: j = 1:10), to: x((i + j) / 2)) + x = x + 1 + !$omp end target +end + +!UNPARSE: SUBROUTINE f12 (x) +!UNPARSE: INTEGER x(10_4) +!UNPARSE: !$OMP TARGET MAP(PRESENT, ITERATOR(INTEGER i = 1_4:10_4, INTEGER j = 1_4:10_4), TO: x((i+j)/2_4)) +!UNPARSE: x=x+1_4 +!UNPARSE: !$OMP END TARGET +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginBlockDirective +!PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | TypeModifier = Present +!PARSE-TREE: | | OmpIteratorModifier -> OmpIteratorSpecifier +!PARSE-TREE: | | | TypeDeclarationStmt +!PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'i' +!PARSE-TREE: | | | SubscriptTriplet +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '1_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '1' +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '10_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '10' +!PARSE-TREE: | | OmpIteratorSpecifier +!PARSE-TREE: | | | TypeDeclarationStmt +!PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'j' +!PARSE-TREE: | | | SubscriptTriplet +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '1_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '1' +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '10_4' +!PARSE-TREE: | | | | | LiteralConstant -> IntLiteralConstant = '10' +!PARSE-TREE: | | Type = To +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> ArrayElement +!PARSE-TREE: | | | DataRef -> Name = 'x' +!PARSE-TREE: | | | SectionSubscript -> Integer -> Expr = '(i+j)/2_4' +!PARSE-TREE: | | | | Divide +!PARSE-TREE: | | | | | Expr = '(i+j)' +!PARSE-TREE: | | | | | | Parentheses -> Expr = 'i+j' +!PARSE-TREE: | | | | | | | Add +!PARSE-TREE: | | | | | | | | Expr = 'i' +!PARSE-TREE: | | | | | | | | | Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | | | | | | | Expr = 'j' +!PARSE-TREE: | | | | | | | | | Designator -> DataRef -> Name = 'j' +!PARSE-TREE: | | | | | Expr = '2_4' +!PARSE-TREE: | | | | | | LiteralConstant -> IntLiteralConstant = '2' +!PARSE-TREE: | | bool = 'true' + +subroutine f90(x, y) + integer :: x(10) + integer :: y + integer, parameter :: p = 23 + !$omp target map(present, iterator(i, j = y:p, k = i:j), to: x(k)) + x = x + 1 + !$omp end target +end + +!UNPARSE: SUBROUTINE f90 (x, y) +!UNPARSE: INTEGER x(10_4) +!UNPARSE: INTEGER y +!UNPARSE: INTEGER, PARAMETER :: p = 23_4 +!UNPARSE: !$OMP TARGET MAP(PRESENT, ITERATOR(INTEGER i, j = y:23_4, INTEGER k = i:j), TO: x(k)) +!UNPARSE: x=x+1_4 +!UNPARSE: !$OMP END TARGET +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginBlockDirective +!PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | TypeModifier = Present +!PARSE-TREE: | | OmpIteratorModifier -> OmpIteratorSpecifier +!PARSE-TREE: | | | TypeDeclarationStmt +!PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'i' +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'j' +!PARSE-TREE: | | | SubscriptTriplet +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = 'y' +!PARSE-TREE: | | | | | Designator -> DataRef -> Name = 'y' +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = '23_4' +!PARSE-TREE: | | | | | Designator -> DataRef -> Name = 'p' +!PARSE-TREE: | | OmpIteratorSpecifier +!PARSE-TREE: | | | TypeDeclarationStmt +!PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec -> +!PARSE-TREE: | | | | EntityDecl +!PARSE-TREE: | | | | | Name = 'k' +!PARSE-TREE: | | | SubscriptTriplet +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = 'i' +!PARSE-TREE: | | | | | Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | | | Scalar -> Integer -> Expr = 'j' +!PARSE-TREE: | | | | | Designator -> DataRef -> Name = 'j' +!PARSE-TREE: | | Type = To +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> ArrayElement +!PARSE-TREE: | | | DataRef -> Name = 'x' +!PARSE-TREE: | | | SectionSubscript -> Integer -> Expr = 'k' +!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'k' +!PARSE-TREE: | | bool = 'true' + diff --git a/flang/test/Semantics/OpenMP/map-modifiers.f90 b/flang/test/Semantics/OpenMP/map-modifiers.f90 index 355df6e083aa5..f863185d111e0 100644 --- a/flang/test/Semantics/OpenMP/map-modifiers.f90 +++ b/flang/test/Semantics/OpenMP/map-modifiers.f90 @@ -1,8 +1,8 @@ -!RUN: %python %S/../test_errors.py %s %flang -fopenmp -Werror +!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=52 -Werror subroutine f10(x) integer :: x -!PORTABILITY: the specification of modifiers without comma separators for the 'MAP' clause has been deprecated +!PORTABILITY: The specification of modifiers without comma separators for the 'MAP' clause has been deprecated in OpenMP 5.2 !$omp target map(always, present close, to: x) x = x + 1 !$omp end target @@ -10,9 +10,81 @@ subroutine f10(x) subroutine f11(x) integer :: x -!PORTABILITY: the specification of modifiers without comma separators for the 'MAP' clause has been deprecated +!PORTABILITY: The specification of modifiers without comma separators for the 'MAP' clause has been deprecated in OpenMP 5.2 !$omp target map(always, present, close to: x) x = x + 1 !$omp end target end +subroutine f12(x) + integer :: x +!WARNING: Duplicate map-type-modifier entry 'PRESENT' will be ignored + !$omp target map(always, present, close, present, to: x) + x = x + 1 + !$omp end target +end + +subroutine f13(x) + integer :: x(10) +!ERROR: The iterator variable must be of integer type +!ERROR: Must have INTEGER type, but is REAL(4) + !$omp target map(present, iterator(real :: i = 1:10), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f14(x) + integer :: x(10) +!ERROR: The begin and end expressions in iterator range-specification are mandatory + !$omp target map(present, iterator(integer :: i = :10:1), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f15(x) + integer :: x(10) +!ERROR: The begin and end expressions in iterator range-specification are mandatory + !$omp target map(present, iterator(integer :: i = 1:), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f16(x) + integer :: x(10) +!ERROR: The begin and end expressions in iterator range-specification are mandatory + !$omp target map(present, iterator(integer :: i = 1::-1), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f17(x) + integer :: x(10) +!WARNING: The step value in the iterator range is 0 + !$omp target map(present, iterator(integer :: i = 1:2:0), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f18(x) + integer :: x(10) +!WARNING: The begin value is less than the end value in iterator range-specification with a negative step + !$omp target map(present, iterator(integer :: i = 1:10:-2), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f19(x) + integer :: x(10) +!WARNING: The begin value is greater than the end value in iterator range-specification with a positive step + !$omp target map(present, iterator(integer :: i = 12:1:2), to: x(i)) + x = x + 1 + !$omp end target +end + +subroutine f1a(x) + integer :: x(10) +!ERROR: Only one iterator-modifier is allowed + !$omp target map(present, iterator(i = 1:2), iterator(j = 1:2), to: x(i + j)) + x = x + 1 + !$omp end target +end diff --git a/flang/test/Semantics/OpenMP/symbol08.f90 b/flang/test/Semantics/OpenMP/symbol08.f90 index 69ccd17391b54..81b512bce1754 100644 --- a/flang/test/Semantics/OpenMP/symbol08.f90 +++ b/flang/test/Semantics/OpenMP/symbol08.f90 @@ -132,21 +132,21 @@ subroutine dotprod (b, c, n, block_size, num_teams, block_threads) !$omp target map(to:b,c) map(tofrom:sum) !$omp teams num_teams(num_teams) thread_limit(block_threads) reduction(+:sum) !$omp distribute - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/i0 (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/i0 (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) !REF: /dotprod/n !REF: /dotprod/block_size do i0=1,n,block_size !$omp parallel do reduction(+:sum) - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/i0 HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/i0 HostAssoc INTEGER(4) !DEF: /dotprod/min ELEMENTAL, INTRINSIC, PURE (Function) ProcEntity - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/block_size HostAssoc INTEGER(4) - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/n HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/block_size HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/n HostAssoc INTEGER(4) do i=i0,min(i0+block_size, n) - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/sum (OmpReduction) HostAssoc REAL(4) - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/b HostAssoc REAL(4) - !REF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/i - !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/c HostAssoc REAL(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/sum (OmpReduction) HostAssoc REAL(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/b HostAssoc REAL(4) + !REF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/i + !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/c HostAssoc REAL(4) sum = sum+b(i)*c(i) end do end do diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.h b/llvm/include/llvm/Frontend/OpenMP/OMP.h index 54ae672755ba8..c13a623a32dda 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.h @@ -32,6 +32,9 @@ bool isLeafConstruct(Directive D); bool isCompositeConstruct(Directive D); bool isCombinedConstruct(Directive D); +/// Can clause C have an iterator-modifier. +bool canHaveIterator(Clause C); + /// Create a nicer version of a function name for humans to look at. std::string prettifyFunctionName(StringRef FunctionName); diff --git a/llvm/lib/Frontend/OpenMP/OMP.cpp b/llvm/lib/Frontend/OpenMP/OMP.cpp index fdb09678d7a4c..c1d584ca71db4 100644 --- a/llvm/lib/Frontend/OpenMP/OMP.cpp +++ b/llvm/lib/Frontend/OpenMP/OMP.cpp @@ -193,6 +193,20 @@ bool isCombinedConstruct(Directive D) { return !getLeafConstructs(D).empty() && !isCompositeConstruct(D); } +bool canHaveIterator(Clause C) { + // [5.2:67:5] + switch (C) { + case OMPC_affinity: + case OMPC_depend: + case OMPC_from: + case OMPC_map: + case OMPC_to: + return true; + default: + return false; + } +} + std::string prettifyFunctionName(StringRef FunctionName) { // Internalized functions have the right name, but simply a suffix. if (FunctionName.ends_with(".internalized")) From 4cf2b44cd11b15eaeb2bfbd2e7f8465694adfe87 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 21 Oct 2024 09:28:07 -0500 Subject: [PATCH 2/8] format --- flang/include/flang/Parser/parse-tree.h | 8 ++-- flang/lib/Parser/openmp-parsers.cpp | 51 ++++++++++----------- flang/lib/Semantics/check-omp-structure.cpp | 17 ++++--- 3 files changed, 36 insertions(+), 40 deletions(-) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 35821288699fd..f20d6b0e85e0b 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3475,10 +3475,10 @@ struct OmpMapClause { // In OpenMP 5.2 the non-comma syntax has been deprecated: keep the // information about separator presence to emit a diagnostic if needed. std::tuple>, - std::optional>, // unique - std::optional>, // unique - OmpObjectList, - bool> // were the modifiers comma-separated + std::optional>, // unique + std::optional>, // unique + OmpObjectList, + bool> // were the modifiers comma-separated t; }; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 26ee1653cc1d4..4a1daed04f3e9 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -56,13 +56,13 @@ struct ConcatSeparated { if (auto first{firstParser.Parse(state)}) { if (attempt(std::get<0>(sepAndParsers_)).Parse(state)) { return std::tuple_cat(TupleFor(std::move(*first)), - std::move(*restParser.Parse(state))); + std::move(*restParser.Parse(state))); } return std::tuple_cat(TupleFor{std::move(*first)}, - std::tuple...>{}); + std::tuple...>{}); } - return std::tuple_cat(TupleFor{}, - std::move(*restParser.Parse(state))); + return std::tuple_cat( + TupleFor{}, std::move(*restParser.Parse(state))); } } @@ -90,8 +90,7 @@ struct ConcatSeparated { // [1] Any of the commas are optional, but that syntax has been deprecated // in OpenMP 5.2, and the parsing code keeps a record of whether the commas // were present. -template -struct MapModifiers { +template struct MapModifiers { constexpr MapModifiers(Separator sep) : sep_(sep) {} constexpr MapModifiers(const MapModifiers &) = default; constexpr MapModifiers(MapModifiers &&) = default; @@ -143,10 +142,10 @@ static std::list makeEntityList(std::list &&names) { return entities; } -static TypeDeclarationStmt makeIterSpecDecl(DeclarationTypeSpec &&spec, - std::list &&names) { - return TypeDeclarationStmt(std::move(spec), std::list{}, - makeEntityList(std::move(names))); +static TypeDeclarationStmt makeIterSpecDecl( + DeclarationTypeSpec &&spec, std::list &&names) { + return TypeDeclarationStmt( + std::move(spec), std::list{}, makeEntityList(std::move(names))); } static TypeDeclarationStmt makeIterSpecDecl(std::list &&names) { @@ -155,7 +154,7 @@ static TypeDeclarationStmt makeIterSpecDecl(std::list &&names) { IntrinsicTypeSpec{IntegerTypeSpec{std::nullopt}}); return TypeDeclarationStmt(std::move(typeSpec), std::list{}, - makeEntityList(std::move(names))); + makeEntityList(std::move(names))); } TYPE_PARSER(construct( @@ -167,16 +166,15 @@ TYPE_PARSER(construct( // 2. integer :: j = i:10 will be flagged as an error because the // initializer 'i' must be constant (in declarations). In an iterator // specifier the 'j' is not an initializer and can be a variable. - (applyFunction( - makeIterSpecDecl, Parser{} / maybe("::"_tok), + (applyFunction(makeIterSpecDecl, + Parser{} / maybe("::"_tok), nonemptyList(Parser{}) / "="_tok) || - applyFunction( - makeIterSpecDecl, nonemptyList(Parser{}) / "="_tok)), + applyFunction( + makeIterSpecDecl, nonemptyList(Parser{}) / "="_tok)), subscriptTriplet)) // [5.0] 2.1.6 iterator -> iterator-specifier-list -TYPE_PARSER(construct( - "ITERATOR" >> +TYPE_PARSER(construct("ITERATOR" >> parenthesized(nonemptyList(sourced(Parser{}))))) // 2.15.3.1 DEFAULT (PRIVATE | FIRSTPRIVATE | SHARED | NONE) @@ -213,22 +211,21 @@ TYPE_PARSER( "TOFROM" >> pure(OmpMapClause::Type::Tofrom))) template -static inline OmpMapClause -makeMapClause(std::tuple>, - std::optional>, - std::optional>> &&mods, - OmpObjectList &&objs) { +static inline OmpMapClause makeMapClause( + std::tuple>, + std::optional>, + std::optional>> &&mods, + OmpObjectList &&objs) { auto &&[tm, it, ty] = std::move(mods); return OmpMapClause{std::move(tm), std::move(it), std::move(ty), - std::move(objs), CommasEverywhere}; + std::move(objs), CommasEverywhere}; } TYPE_PARSER(construct( - applyFunction(makeMapClause, MapModifiers(","_tok), - Parser{}) || + applyFunction( + makeMapClause, MapModifiers(","_tok), Parser{}) || applyFunction(makeMapClause, - MapModifiers(maybe(","_tok)), - Parser{}))) + MapModifiers(maybe(","_tok)), Parser{}))) // [OpenMP 5.0] // 2.19.7.2 defaultmap(implicit-behavior[:variable-category]) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 68da0669c1296..c3b711f04fb5e 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -566,8 +566,7 @@ void OmpStructureChecker::CheckIteratorRange( // a proper interval. const auto &[begin, end, step]{std::get(x.t).t}; if (!begin || !end) { - context_.Say( - x.source, + context_.Say(x.source, "The begin and end expressions in iterator range-specification are " "mandatory"_err_en_US); } @@ -575,20 +574,20 @@ void OmpStructureChecker::CheckIteratorRange( // value is implicitly defined to be 1. if (auto stepv{step ? GetIntValue(*step) : std::optional{1}}) { if (*stepv == 0) { - context_.Say(x.source, - "The step value in the iterator range is 0"_warn_en_US); + context_.Say( + x.source, "The step value in the iterator range is 0"_warn_en_US); } else if (begin && end) { std::optional beginv{GetIntValue(*begin)}; std::optional endv{GetIntValue(*end)}; if (beginv && endv) { if (*stepv > 0 && *beginv > *endv) { context_.Say(x.source, - "The begin value is greater than the end value in iterator " - "range-specification with a positive step"_warn_en_US); + "The begin value is greater than the end value in iterator " + "range-specification with a positive step"_warn_en_US); } else if (*stepv < 0 && *beginv < *endv) { context_.Say(x.source, - "The begin value is less than the end value in iterator " - "range-specification with a negative step"_warn_en_US); + "The begin value is less than the end value in iterator " + "range-specification with a negative step"_warn_en_US); } } } @@ -612,7 +611,7 @@ void OmpStructureChecker::CheckIteratorModifier( } if (!isInteger) { context_.Say(iterSpec.source, - "The iterator variable must be of integer type"_err_en_US); + "The iterator variable must be of integer type"_err_en_US); } CheckIteratorRange(iterSpec); } From 44c98846220fce6b2fad72e7247ac5861d8262d5 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 21 Oct 2024 13:44:44 -0500 Subject: [PATCH 3/8] Replace `join` lambda with explicit calls to Walk clang flags some issues with inline templates being undefined. The problem may be legitimate, but in order to save time, the template-lambda was removed altogether. --- flang/lib/Parser/unparse.cpp | 42 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 1839a276c2490..5870aba0132c8 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2092,30 +2092,26 @@ class UnparseVisitor { // For a given list of items, if the item has a value, then walk it. // Print commas between items that have values. // Return 'true' if something did get printed, otherwise 'false'. - auto join = [&](bool comma, auto &&cont, auto &&item, - auto &&...items) -> bool { - // comma: if something needs to be walked (i.e. printed), precede it - // with a comma. - if (!item.has_value()) { - if constexpr (sizeof...(items) != 0) { - return cont(comma, cont, items...); - } else { - return false; - } - } else { - if (comma) { - Put(", "); - } - Walk(*item); - if constexpr (sizeof...(items) != 0) { - return cont(true, cont, items...) || true; - } else { - return true; - } + bool needComma{false}; + if (typeMod) { + Walk(*typeMod); + needComma = true; + } + if (iter) { + if (needComma) { + Put(", "); } - }; - - if (join(false, join, typeMod, iter, type)) { + Walk(*iter); + needComma = true; + } + if (type) { + if (needComma) { + Put(", "); + } + Walk(*type); + needComma = true; + } + if (needComma) { Put(": "); } Walk(std::get(x.t)); From 13d9cbe9bcbe2c519d884070bf2de5a2b7c44f14 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 22 Oct 2024 12:27:50 -0500 Subject: [PATCH 4/8] update comment --- flang/include/flang/Parser/parse-tree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index f20d6b0e85e0b..16281a58adc30 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3424,7 +3424,7 @@ struct AssignedGotoStmt { WRAPPER_CLASS(PauseStmt, std::optional); -// Parse tree nodes for OpenMP 4.5+ directives and clauses +// Parse tree nodes for OpenMP 5.2 directives and clauses // [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple // iterator-modifier -> iterator-specifier-list From c25df9a605f656cea671cc7126651a7be4412a0d Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 22 Oct 2024 12:57:11 -0500 Subject: [PATCH 5/8] introduce Scope::Kind::OtherClause --- flang/include/flang/Semantics/scope.h | 2 +- flang/lib/Semantics/resolve-names.cpp | 5 +++-- flang/test/Semantics/OpenMP/symbol08.f90 | 18 +++++++++--------- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/flang/include/flang/Semantics/scope.h b/flang/include/flang/Semantics/scope.h index e73a507e9b3f5..b3b033a5a3ae3 100644 --- a/flang/include/flang/Semantics/scope.h +++ b/flang/include/flang/Semantics/scope.h @@ -61,7 +61,7 @@ class Scope { public: ENUM_CLASS(Kind, Global, IntrinsicModules, Module, MainProgram, Subprogram, BlockData, DerivedType, BlockConstruct, Forall, OtherConstruct, - OpenACCConstruct, ImpliedDos) + OpenACCConstruct, ImpliedDos, OtherClause) using ImportKind = common::ImportKind; // Create the Global scope -- the root of the scope tree diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index f9713fcfc58ed..efc775659f8d2 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -1550,7 +1550,7 @@ class OmpVisitor : public virtual DeclarationVisitor { } bool Pre(const parser::OmpClause &x) { if (NeedsScope(x)) { - PushScope(Scope::Kind::OtherConstruct, nullptr); + PushScope(Scope::Kind::OtherClause, nullptr); } return true; } @@ -2440,7 +2440,8 @@ void ScopeHandler::PushScope(Scope &scope) { currScope_ = &scope; auto kind{currScope_->kind()}; if (kind != Scope::Kind::BlockConstruct && - kind != Scope::Kind::OtherConstruct) { + kind != Scope::Kind::OtherConstruct && + kind != Scope::Kind::OtherClause) { BeginScope(scope); } // The name of a module or submodule cannot be "used" in its scope, diff --git a/flang/test/Semantics/OpenMP/symbol08.f90 b/flang/test/Semantics/OpenMP/symbol08.f90 index 81b512bce1754..69ccd17391b54 100644 --- a/flang/test/Semantics/OpenMP/symbol08.f90 +++ b/flang/test/Semantics/OpenMP/symbol08.f90 @@ -132,21 +132,21 @@ subroutine dotprod (b, c, n, block_size, num_teams, block_threads) !$omp target map(to:b,c) map(tofrom:sum) !$omp teams num_teams(num_teams) thread_limit(block_threads) reduction(+:sum) !$omp distribute - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/i0 (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/i0 (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) !REF: /dotprod/n !REF: /dotprod/block_size do i0=1,n,block_size !$omp parallel do reduction(+:sum) - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/i0 HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/i0 HostAssoc INTEGER(4) !DEF: /dotprod/min ELEMENTAL, INTRINSIC, PURE (Function) ProcEntity - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/block_size HostAssoc INTEGER(4) - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/n HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/block_size HostAssoc INTEGER(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/n HostAssoc INTEGER(4) do i=i0,min(i0+block_size, n) - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/sum (OmpReduction) HostAssoc REAL(4) - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/b HostAssoc REAL(4) - !REF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/i - !DEF: /dotprod/OtherConstruct1/OtherConstruct3/OtherConstruct1/OtherConstruct1/c HostAssoc REAL(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/sum (OmpReduction) HostAssoc REAL(4) + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/b HostAssoc REAL(4) + !REF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/i + !DEF: /dotprod/OtherConstruct1/OtherConstruct1/OtherConstruct1/OtherConstruct1/c HostAssoc REAL(4) sum = sum+b(i)*c(i) end do end do From ddc9c8668066ca5e9d2b5ed5f85be29e5551e5a6 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 22 Oct 2024 12:57:39 -0500 Subject: [PATCH 6/8] add ? --- flang/include/flang/Parser/parse-tree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 16281a58adc30..e923e2199bff9 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3478,7 +3478,7 @@ struct OmpMapClause { std::optional>, // unique std::optional>, // unique OmpObjectList, - bool> // were the modifiers comma-separated + bool> // were the modifiers comma-separated? t; }; From 88af970976e3b3261daecbccd0865eb86560c563 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 22 Oct 2024 14:26:13 -0500 Subject: [PATCH 7/8] move function to header --- llvm/include/llvm/Frontend/OpenMP/OMP.h | 14 +++++++++++++- llvm/lib/Frontend/OpenMP/OMP.cpp | 14 -------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.h b/llvm/include/llvm/Frontend/OpenMP/OMP.h index c13a623a32dda..0d79c071ecd30 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.h @@ -33,7 +33,19 @@ bool isCompositeConstruct(Directive D); bool isCombinedConstruct(Directive D); /// Can clause C have an iterator-modifier. -bool canHaveIterator(Clause C); +static constexpr inline bool canHaveIterator(Clause C) { + // [5.2:67:5] + switch (C) { + case OMPC_affinity: + case OMPC_depend: + case OMPC_from: + case OMPC_map: + case OMPC_to: + return true; + default: + return false; + } +} /// Create a nicer version of a function name for humans to look at. std::string prettifyFunctionName(StringRef FunctionName); diff --git a/llvm/lib/Frontend/OpenMP/OMP.cpp b/llvm/lib/Frontend/OpenMP/OMP.cpp index c1d584ca71db4..fdb09678d7a4c 100644 --- a/llvm/lib/Frontend/OpenMP/OMP.cpp +++ b/llvm/lib/Frontend/OpenMP/OMP.cpp @@ -193,20 +193,6 @@ bool isCombinedConstruct(Directive D) { return !getLeafConstructs(D).empty() && !isCompositeConstruct(D); } -bool canHaveIterator(Clause C) { - // [5.2:67:5] - switch (C) { - case OMPC_affinity: - case OMPC_depend: - case OMPC_from: - case OMPC_map: - case OMPC_to: - return true; - default: - return false; - } -} - std::string prettifyFunctionName(StringRef FunctionName) { // Internalized functions have the right name, but simply a suffix. if (FunctionName.ends_with(".internalized")) From 42c19a541d0a8c0250b8b42c25850bb1f1b838cf Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 22 Oct 2024 14:32:18 -0500 Subject: [PATCH 8/8] format --- flang/lib/Semantics/resolve-names.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index efc775659f8d2..add4e4befd3a2 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -2440,8 +2440,7 @@ void ScopeHandler::PushScope(Scope &scope) { currScope_ = &scope; auto kind{currScope_->kind()}; if (kind != Scope::Kind::BlockConstruct && - kind != Scope::Kind::OtherConstruct && - kind != Scope::Kind::OtherClause) { + kind != Scope::Kind::OtherConstruct && kind != Scope::Kind::OtherClause) { BeginScope(scope); } // The name of a module or submodule cannot be "used" in its scope,