https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/149137
>From acc1a7d3da0d2bd5a60096a9f5dc27338342b952 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <krzysztof.parzys...@amd.com> Date: Wed, 16 Jul 2025 08:04:17 -0500 Subject: [PATCH 1/3] [flang][OpenMP] Sema checks, lowering with new format of MAP modifiers OpenMP 6.0 has changed the modifiers on the MAP clause. Previous patch has introduced parsing support for them. This patch introduces processing of the new forms in semantic checks and in lowering. This only applies to existing modifiers, which were updated in the 6.0 spec. Any of the newly introduced modifiers (SELF and REF) are ignored. --- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 16 +-- flang/lib/Lower/OpenMP/Clauses.cpp | 113 ++++++++++++------ flang/lib/Semantics/canonicalize-omp.cpp | 5 + flang/lib/Semantics/check-omp-structure.cpp | 107 +++++++++++------ flang/lib/Semantics/check-omp-structure.h | 4 +- flang/lib/Semantics/openmp-utils.cpp | 25 ++++ flang/lib/Semantics/openmp-utils.h | 3 + flang/lib/Semantics/resolve-directives.cpp | 50 ++++++-- .../Semantics/OpenMP/combined-constructs.f90 | 8 +- .../Semantics/OpenMP/device-constructs.f90 | 6 +- llvm/include/llvm/Frontend/OpenMP/ClauseT.h | 9 +- .../Frontend/OpenMP/ConstructDecompositionT.h | 5 +- llvm/include/llvm/Frontend/OpenMP/OMP.h | 2 + llvm/lib/Frontend/OpenMP/OMP.cpp | 14 +++ 14 files changed, 260 insertions(+), 107 deletions(-) diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 74087d42a8e6e..ec71014c36093 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1315,7 +1315,8 @@ bool ClauseProcessor::processMap( const parser::CharBlock &source) { using Map = omp::clause::Map; mlir::Location clauseLocation = converter.genLocation(source); - const auto &[mapType, typeMods, mappers, iterator, objects] = clause.t; + const auto &[mapType, typeMods, refMod, mappers, iterator, objects] = + clause.t; llvm::omp::OpenMPOffloadMappingFlags mapTypeBits = llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE; std::string mapperIdName = "__implicit_mapper"; @@ -1342,16 +1343,13 @@ bool ClauseProcessor::processMap( mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO | llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; break; - case Map::MapType::Alloc: - case Map::MapType::Release: + case Map::MapType::Storage: // 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. + // (aka storage in 6.0+) 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; } if (typeMods) { @@ -1362,6 +1360,8 @@ bool ClauseProcessor::processMap( mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRESENT; if (llvm::is_contained(*typeMods, Map::MapTypeModifier::Close)) mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_CLOSE; + if (llvm::is_contained(*typeMods, Map::MapTypeModifier::Delete)) + mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE; if (llvm::is_contained(*typeMods, Map::MapTypeModifier::OmpxHold)) mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_OMPX_HOLD; } diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 22a07219d3a50..18e49719b6013 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -1001,64 +1001,105 @@ Map make(const parser::OmpClause::Map &inp, semantics::SemanticsContext &semaCtx) { // inp.v -> parser::OmpMapClause CLAUSET_ENUM_CONVERT( // - convert1, parser::OmpMapType::Value, Map::MapType, + convert1, parser::OmpMapTypeModifier::Value, Map::MapTypeModifier, // clang-format off - MS(Alloc, Alloc) - MS(Delete, Delete) - MS(From, From) - MS(Release, Release) - MS(To, To) - MS(Tofrom, Tofrom) + MS(Always, Always) + MS(Close, Close) + MS(Ompx_Hold, OmpxHold) + MS(Present, Present) // clang-format on ); CLAUSET_ENUM_CONVERT( // - convert2, parser::OmpMapTypeModifier::Value, Map::MapTypeModifier, + convert2, parser::OmpRefModifier::Value, Map::RefModifier, // clang-format off - MS(Always, Always) - MS(Close, Close) - MS(Ompx_Hold, OmpxHold) - MS(Present, Present) + MS(Ref_Ptee, RefPtee) + MS(Ref_Ptr, RefPtr) + MS(Ref_Ptr_Ptee, RefPtrPtee) // clang-format on ); + // Treat always, close, present, self, delete modifiers as map-type- + // modifiers. auto &mods = semantics::OmpGetModifiers(inp.v); - auto *t1 = semantics::OmpGetUniqueModifier<parser::OmpMapper>(mods); - auto *t2 = semantics::OmpGetUniqueModifier<parser::OmpIterator>(mods); - auto *t3 = semantics::OmpGetUniqueModifier<parser::OmpMapType>(mods); - auto &t4 = std::get<parser::OmpObjectList>(inp.v.t); - auto mappers = [&]() -> std::optional<List<Mapper>> { - if (t1) - return List<Mapper>{Mapper{makeObject(t1->v, semaCtx)}}; - return std::nullopt; - }(); - - auto iterator = [&]() -> std::optional<Iterator> { - if (t2) - return makeIterator(*t2, semaCtx); - return std::nullopt; - }(); + auto *t1 = semantics::OmpGetUniqueModifier<parser::OmpMapType>(mods); + auto &t2 = std::get<parser::OmpObjectList>(inp.v.t); auto type = [&]() -> std::optional<Map::MapType> { - if (t3) - return convert1(t3->v); - return std::nullopt; + std::optional<Map::MapType> type; + if (t1) { + switch (t1->v) { + case parser::OmpMapType::Value::Alloc: + case parser::OmpMapType::Value::Delete: + case parser::OmpMapType::Value::Release: + case parser::OmpMapType::Value::Storage: + type = Map::MapType::Storage; + break; + case parser::OmpMapType::Value::From: + type = Map::MapType::From; + break; + case parser::OmpMapType::Value::To: + type = Map::MapType::To; + break; + case parser::OmpMapType::Value::Tofrom: + type = Map::MapType::Tofrom; + break; + default: + break; + } + } + return type; }(); - Map::MapTypeModifiers typeMods; + llvm::DenseSet<Map::MapTypeModifier> modSet; + if (t1 && t1->v == parser::OmpMapType::Value::Delete) + modSet.insert(Map::MapTypeModifier::Delete); + for (auto *typeMod : semantics::OmpGetRepeatableModifier<parser::OmpMapTypeModifier>(mods)) { - typeMods.push_back(convert2(typeMod->v)); + modSet.insert(convert1(typeMod->v)); } + if (semantics::OmpGetUniqueModifier<parser::OmpAlwaysModifier>(mods)) + modSet.insert(Map::MapTypeModifier::Always); + if (semantics::OmpGetUniqueModifier<parser::OmpCloseModifier>(mods)) + modSet.insert(Map::MapTypeModifier::Close); + if (semantics::OmpGetUniqueModifier<parser::OmpDeleteModifier>(mods)) + modSet.insert(Map::MapTypeModifier::Delete); + if (semantics::OmpGetUniqueModifier<parser::OmpPresentModifier>(mods)) + modSet.insert(Map::MapTypeModifier::Present); + if (semantics::OmpGetUniqueModifier<parser::OmpSelfModifier>(mods)) + modSet.insert(Map::MapTypeModifier::Self); + if (semantics::OmpGetUniqueModifier<parser::OmpxHoldModifier>(mods)) + modSet.insert(Map::MapTypeModifier::OmpxHold); + std::optional<Map::MapTypeModifiers> maybeTypeMods{}; - if (!typeMods.empty()) - maybeTypeMods = std::move(typeMods); + if (!modSet.empty()) + maybeTypeMods = Map::MapTypeModifiers(modSet.begin(), modSet.end()); + + auto refMod = [&]() -> std::optional<Map::RefModifier> { + if (auto *t = semantics::OmpGetUniqueModifier<parser::OmpRefModifier>(mods)) + return convert2(t->v); + return std::nullopt; + }(); + + auto mappers = [&]() -> std::optional<List<Mapper>> { + if (auto *t = semantics::OmpGetUniqueModifier<parser::OmpMapper>(mods)) + return List<Mapper>{Mapper{makeObject(t->v, semaCtx)}}; + return std::nullopt; + }(); + + auto iterator = [&]() -> std::optional<Iterator> { + if (auto *t = semantics::OmpGetUniqueModifier<parser::OmpIterator>(mods)) + return makeIterator(*t, semaCtx); + return std::nullopt; + }(); return Map{{/*MapType=*/std::move(type), /*MapTypeModifiers=*/std::move(maybeTypeMods), - /*Mapper=*/std::move(mappers), /*Iterator=*/std::move(iterator), - /*LocatorList=*/makeObjects(t4, semaCtx)}}; + /*RefModifier=*/std::move(refMod), /*Mapper=*/std::move(mappers), + /*Iterator=*/std::move(iterator), + /*LocatorList=*/makeObjects(t2, semaCtx)}}; } Match make(const parser::OmpClause::Match &inp, diff --git a/flang/lib/Semantics/canonicalize-omp.cpp b/flang/lib/Semantics/canonicalize-omp.cpp index 46aaab19ded0a..77e2fd6ca5097 100644 --- a/flang/lib/Semantics/canonicalize-omp.cpp +++ b/flang/lib/Semantics/canonicalize-omp.cpp @@ -402,6 +402,11 @@ class CanonicalizationOfOmp { // if the specified OpenMP version is less than 6.0, rewrite the affected // modifiers back into the pre-6.0 forms. void CanonicalizeMapModifiers(parser::OmpMapClause &map) { + unsigned version{context_.langOptions().OpenMPVersion}; + if (version >= 60) { + return; + } + // Omp{Always, Close, Present, xHold}Modifier -> OmpMapTypeModifier // OmpDeleteModifier -> OmpMapType using Modifier = parser::OmpMapClause::Modifier; diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 2425265e196c6..94dee3d7afe21 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -37,6 +37,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Frontend/OpenMP/OMP.h" @@ -3399,23 +3400,22 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Detach &x) { } } -void OmpStructureChecker::CheckAllowedMapTypes( - const parser::OmpMapType::Value &type, - const std::list<parser::OmpMapType::Value> &allowedMapTypeList) { - if (!llvm::is_contained(allowedMapTypeList, type)) { - std::string commaSeparatedMapTypes; - llvm::interleave( - allowedMapTypeList.begin(), allowedMapTypeList.end(), - [&](const parser::OmpMapType::Value &mapType) { - commaSeparatedMapTypes.append(parser::ToUpperCaseLetters( - parser::OmpMapType::EnumToString(mapType))); - }, - [&] { commaSeparatedMapTypes.append(", "); }); - context_.Say(GetContext().clauseSource, - "Only the %s map types are permitted " - "for MAP clauses on the %s directive"_err_en_US, - commaSeparatedMapTypes, ContextDirectiveAsFortran()); +void OmpStructureChecker::CheckAllowedMapTypes(parser::OmpMapType::Value type, + llvm::ArrayRef<parser::OmpMapType::Value> allowed) { + if (llvm::is_contained(allowed, type)) { + return; } + + llvm::SmallVector<std::string> names; + llvm::transform( + allowed, std::back_inserter(names), [](parser::OmpMapType::Value val) { + return parser::ToUpperCaseLetters( + parser::OmpMapType::EnumToString(val)); + }); + llvm::sort(names); + context_.Say(GetContext().clauseSource, + "Only the %s map types are permitted for MAP clauses on the %s directive"_err_en_US, + llvm::join(names, ", "), ContextDirectiveAsFortran()); } void OmpStructureChecker::Enter(const parser::OmpClause::Map &x) { @@ -3436,27 +3436,62 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Map &x) { CheckIteratorModifier(*iter); } if (auto *type{OmpGetUniqueModifier<parser::OmpMapType>(modifiers)}) { + using Directive = llvm::omp::Directive; using Value = parser::OmpMapType::Value; - switch (GetContext().directive) { - case llvm::omp::Directive::OMPD_target: - case llvm::omp::Directive::OMPD_target_teams: - case llvm::omp::Directive::OMPD_target_teams_distribute: - case llvm::omp::Directive::OMPD_target_teams_distribute_simd: - case llvm::omp::Directive::OMPD_target_teams_distribute_parallel_do: - case llvm::omp::Directive::OMPD_target_teams_distribute_parallel_do_simd: - case llvm::omp::Directive::OMPD_target_data: - CheckAllowedMapTypes( - type->v, {Value::To, Value::From, Value::Tofrom, Value::Alloc}); - break; - case llvm::omp::Directive::OMPD_target_enter_data: - CheckAllowedMapTypes(type->v, {Value::To, Value::Alloc}); - break; - case llvm::omp::Directive::OMPD_target_exit_data: - CheckAllowedMapTypes( - type->v, {Value::From, Value::Release, Value::Delete}); - break; - default: - break; + + static auto isValidForVersion{ + [](parser::OmpMapType::Value t, unsigned version) { + switch (t) { + case parser::OmpMapType::Value::Alloc: + case parser::OmpMapType::Value::Delete: + case parser::OmpMapType::Value::Release: + return version < 60; + case parser::OmpMapType::Value::Storage: + return version >= 60; + default: + return true; + } + }}; + + llvm::SmallVector<parser::OmpMapType::Value> mapEnteringTypes{[&]() { + llvm::SmallVector<parser::OmpMapType::Value> result; + for (size_t i{0}; i != parser::OmpMapType::Value_enumSize; ++i) { + auto t{static_cast<parser::OmpMapType::Value>(i)}; + if (isValidForVersion(t, version) && IsMapEnteringType(t)) { + result.push_back(t); + } + } + return result; + }()}; + llvm::SmallVector<parser::OmpMapType::Value> mapExitingTypes{[&]() { + llvm::SmallVector<parser::OmpMapType::Value> result; + for (size_t i{0}; i != parser::OmpMapType::Value_enumSize; ++i) { + auto t{static_cast<parser::OmpMapType::Value>(i)}; + if (isValidForVersion(t, version) && IsMapExitingType(t)) { + result.push_back(t); + } + } + return result; + }()}; + + llvm::omp::Directive dir{GetContext().directive}; + llvm::ArrayRef<llvm::omp::Directive> leafs{ + llvm::omp::getLeafConstructsOrSelf(dir)}; + + if (llvm::is_contained(leafs, Directive::OMPD_target) || + llvm::is_contained(leafs, Directive::OMPD_target_data)) { + if (version >= 60) { + // Map types listed in the decay table. [6.0:276] + CheckAllowedMapTypes( + type->v, {Value::Storage, Value::From, Value::To, Value::Tofrom}); + } else { + CheckAllowedMapTypes( + type->v, {Value::Alloc, Value::From, Value::To, Value::Tofrom}); + } + } else if (llvm::is_contained(leafs, Directive::OMPD_target_enter_data)) { + CheckAllowedMapTypes(type->v, mapEnteringTypes); + } else if (llvm::is_contained(leafs, Directive::OMPD_target_exit_data)) { + CheckAllowedMapTypes(type->v, mapExitingTypes); } } diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index 6a877a5d0a7c0..f4a291dc255c8 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -179,8 +179,8 @@ class OmpStructureChecker void HasInvalidDistributeNesting(const parser::OpenMPLoopConstruct &x); void HasInvalidLoopBinding(const parser::OpenMPLoopConstruct &x); // specific clause related - void CheckAllowedMapTypes(const parser::OmpMapType::Value &, - const std::list<parser::OmpMapType::Value> &); + void CheckAllowedMapTypes( + parser::OmpMapType::Value, llvm::ArrayRef<parser::OmpMapType::Value>); const std::list<parser::OmpTraitProperty> &GetTraitPropertyList( const parser::OmpTraitSelector &); diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp index f43d2cc75620e..da14507aa9fe6 100644 --- a/flang/lib/Semantics/openmp-utils.cpp +++ b/flang/lib/Semantics/openmp-utils.cpp @@ -143,6 +143,31 @@ bool IsVarOrFunctionRef(const MaybeExpr &expr) { } } +bool IsMapEnteringType(parser::OmpMapType::Value type) { + switch (type) { + case parser::OmpMapType::Value::Alloc: + case parser::OmpMapType::Value::Storage: + case parser::OmpMapType::Value::To: + case parser::OmpMapType::Value::Tofrom: + return true; + default: + return false; + } +} + +bool IsMapExitingType(parser::OmpMapType::Value type) { + switch (type) { + case parser::OmpMapType::Value::Delete: + case parser::OmpMapType::Value::From: + case parser::OmpMapType::Value::Release: + case parser::OmpMapType::Value::Storage: + case parser::OmpMapType::Value::Tofrom: + return true; + default: + return false; + } +} + std::optional<SomeExpr> GetEvaluateExpr(const parser::Expr &parserExpr) { const parser::TypedExpr &typedExpr{parserExpr.typedExpr}; // ForwardOwningPointer typedExpr diff --git a/flang/lib/Semantics/openmp-utils.h b/flang/lib/Semantics/openmp-utils.h index a96c008fb26e7..001fbeb45ceec 100644 --- a/flang/lib/Semantics/openmp-utils.h +++ b/flang/lib/Semantics/openmp-utils.h @@ -59,6 +59,9 @@ bool IsExtendedListItem(const Symbol &sym); bool IsVariableListItem(const Symbol &sym); bool IsVarOrFunctionRef(const MaybeExpr &expr); +bool IsMapEnteringType(parser::OmpMapType::Value type); +bool IsMapExitingType(parser::OmpMapType::Value type); + std::optional<SomeExpr> GetEvaluateExpr(const parser::Expr &parserExpr); std::optional<evaluate::DynamicType> GetDynamicType( const parser::Expr &parserExpr); diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 0b860314b4a1f..cbd14bc259d38 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -712,7 +712,15 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { void Post(const parser::EorLabel &eorLabel) { CheckSourceLabel(eorLabel.v); } void Post(const parser::OmpMapClause &x) { - Symbol::Flag ompFlag = Symbol::Flag::OmpMapToFrom; + unsigned version{context_.langOptions().OpenMPVersion}; + llvm::omp::Directive id{GetContext().directive}; + std::optional<Symbol::Flag> ompFlag; + // Expand "storage" into either "alloc" or "release", depending on the + // type of the construct. + Symbol::Flag ompFlagForStorage = llvm::omp::isMapEnteringConstruct(id) + ? Symbol::Flag::OmpMapAlloc + : Symbol::Flag::OmpMapRelease; + auto &mods{OmpGetModifiers(x)}; if (auto *mapType{OmpGetUniqueModifier<parser::OmpMapType>(mods)}) { switch (mapType->v) { @@ -734,10 +742,29 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { case parser::OmpMapType::Value::Delete: ompFlag = Symbol::Flag::OmpMapDelete; break; - default: + case parser::OmpMapType::Value::Storage: + ompFlag = ompFlagForStorage; break; } } + if (!ompFlag) { + if (version >= 60) { + // [6.0:275:12-15] + // When a map-type is not specified for a clause on which it may be + // specified, the map-type defaults to storage if the delete-modifier + // is present on the clause or if the list item for which the map-type + // is not specified is an assumed-size array. + if (OmpGetUniqueModifier<parser::OmpDeleteModifier>(mods)) { + ompFlag = ompFlagForStorage; + } + // Otherwise, if delete-modifier is absent, leave ompFlag unset. + } else { + // [5.2:151:10] + // If a map-type is not specified, the map-type defaults to tofrom. + ompFlag = Symbol::Flag::OmpMapToFrom; + } + } + const auto &ompObjList{std::get<parser::OmpObjectList>(x.t)}; for (const auto &ompObj : ompObjList.v) { common::visit( @@ -746,15 +773,14 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { if (const auto *name{ semantics::getDesignatorNameIfDataRef(designator)}) { if (name->symbol) { - name->symbol->set(ompFlag); - AddToContextObjectWithDSA(*name->symbol, ompFlag); - } - if (name->symbol && - semantics::IsAssumedSizeArray(*name->symbol)) { - context_.Say(designator.source, - "Assumed-size whole arrays may not appear on the %s " - "clause"_err_en_US, - "MAP"); + name->symbol->set(ompFlag.value_or(ompFlagForStorage)); + AddToContextObjectWithDSA(*name->symbol, *ompFlag); + if (semantics::IsAssumedSizeArray(*name->symbol)) { + context_.Say(designator.source, + "Assumed-size whole arrays may not appear on the %s " + "clause"_err_en_US, + "MAP"); + } } } }, @@ -762,7 +788,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { }, ompObj.u); - ResolveOmpObject(ompObj, ompFlag); + ResolveOmpObject(ompObj, ompFlag.value_or(ompFlagForStorage)); } } diff --git a/flang/test/Semantics/OpenMP/combined-constructs.f90 b/flang/test/Semantics/OpenMP/combined-constructs.f90 index 4f2a4a4f501b9..2298d33ef33eb 100644 --- a/flang/test/Semantics/OpenMP/combined-constructs.f90 +++ b/flang/test/Semantics/OpenMP/combined-constructs.f90 @@ -207,7 +207,7 @@ program main enddo !$omp end target teams - !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS directive + !ERROR: Only the ALLOC, FROM, TO, TOFROM map types are permitted for MAP clauses on the TARGET TEAMS directive !$omp target teams map(delete:a) do i = 1, N a(i) = 3.14 @@ -307,7 +307,7 @@ program main enddo !$omp end target teams distribute - !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE directive + !ERROR: Only the ALLOC, FROM, TO, TOFROM map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE directive !$omp target teams distribute map(delete:a) do i = 1, N a(i) = 3.14 @@ -400,7 +400,7 @@ program main enddo !$omp end target teams distribute parallel do - !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO directive + !ERROR: Only the ALLOC, FROM, TO, TOFROM map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO directive !$omp target teams distribute parallel do map(delete:a) do i = 1, N a(i) = 3.14 @@ -500,7 +500,7 @@ program main enddo !$omp end target teams distribute parallel do simd - !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD directive + !ERROR: Only the ALLOC, FROM, TO, TOFROM map types are permitted for MAP clauses on the TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD directive !$omp target teams distribute parallel do simd map(delete:a) do i = 1, N a(i) = 3.14 diff --git a/flang/test/Semantics/OpenMP/device-constructs.f90 b/flang/test/Semantics/OpenMP/device-constructs.f90 index 6f545b9021966..431e0f88e3237 100644 --- a/flang/test/Semantics/OpenMP/device-constructs.f90 +++ b/flang/test/Semantics/OpenMP/device-constructs.f90 @@ -123,7 +123,7 @@ program main enddo !$omp end target - !ERROR: Only the TO, FROM, TOFROM, ALLOC map types are permitted for MAP clauses on the TARGET directive + !ERROR: Only the ALLOC, FROM, TO, TOFROM map types are permitted for MAP clauses on the TARGET directive !$omp target map(delete:a) do i = 1, N a = 3.14 @@ -160,7 +160,7 @@ program main !ERROR: At most one IF clause can appear on the TARGET ENTER DATA directive !$omp target enter data map(to:a) if(.true.) if(.false.) - !ERROR: Only the TO, ALLOC map types are permitted for MAP clauses on the TARGET ENTER DATA directive + !ERROR: Only the ALLOC, TO, TOFROM map types are permitted for MAP clauses on the TARGET ENTER DATA directive !$omp target enter data map(from:a) !$omp target exit data map(delete:a) @@ -168,7 +168,7 @@ program main !ERROR: At most one DEVICE clause can appear on the TARGET EXIT DATA directive !$omp target exit data map(from:a) device(0) device(1) - !ERROR: Only the FROM, RELEASE, DELETE map types are permitted for MAP clauses on the TARGET EXIT DATA directive + !ERROR: Only the DELETE, FROM, RELEASE, TOFROM map types are permitted for MAP clauses on the TARGET EXIT DATA directive !$omp target exit data map(to:a) !$omp target update if(.true.) device(1) to(a) from(b) depend(inout:c) nowait diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h index de888ff86fe91..7919f7a8b0c34 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h @@ -779,16 +779,17 @@ struct LinkT { template <typename T, typename I, typename E> // struct MapT { using LocatorList = ObjectListT<I, E>; - ENUM(MapType, To, From, Tofrom, Alloc, Release, Delete); - ENUM(MapTypeModifier, Always, Close, Present, OmpxHold); + ENUM(MapType, To, From, Tofrom, Storage); + ENUM(MapTypeModifier, Always, Close, Delete, Present, Self, OmpxHold); + ENUM(RefModifier, RefPtee, RefPtr, RefPtrPtee); // See note at the definition of the MapperT type. using Mappers = ListT<type::MapperT<I, E>>; // Not a spec name using Iterator = type::IteratorT<T, I, E>; using MapTypeModifiers = ListT<MapTypeModifier>; // Not a spec name using TupleTrait = std::true_type; - std::tuple<OPT(MapType), OPT(MapTypeModifiers), OPT(Mappers), OPT(Iterator), - LocatorList> + std::tuple<OPT(MapType), OPT(MapTypeModifiers), OPT(RefModifier), + OPT(Mappers), OPT(Iterator), LocatorList> t; }; diff --git a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h index 611bfe3f8aced..047baa3a79f5d 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ConstructDecompositionT.h @@ -708,6 +708,7 @@ bool ConstructDecompositionT<C, H>::applyClause( tomp::clause::MapT<TypeTy, IdTy, ExprTy>{ {/*MapType=*/MapType::Tofrom, /*MapTypeModifier=*/std::nullopt, + /*RefModifier=*/std::nullopt, /*Mapper=*/std::nullopt, /*Iterator=*/std::nullopt, /*LocatorList=*/std::move(tofrom)}}); dirTarget->clauses.push_back(map); @@ -969,8 +970,8 @@ bool ConstructDecompositionT<C, H>::applyClause( llvm::omp::Clause::OMPC_map, tomp::clause::MapT<TypeTy, IdTy, ExprTy>{ {/*MapType=*/MapType::Tofrom, /*MapTypeModifier=*/std::nullopt, - /*Mapper=*/std::nullopt, /*Iterator=*/std::nullopt, - /*LocatorList=*/std::move(tofrom)}}); + /*RefModifier=*/std::nullopt, /*Mapper=*/std::nullopt, + /*Iterator=*/std::nullopt, /*LocatorList=*/std::move(tofrom)}}); dirTarget->clauses.push_back(map); applied = true; diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.h b/llvm/include/llvm/Frontend/OpenMP/OMP.h index d44c33301bde7..a8b4503147034 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.h @@ -64,6 +64,8 @@ static constexpr inline bool isPrivatizingClause(Clause C) { } } +LLVM_ABI bool isMapEnteringConstruct(Directive D); + static constexpr unsigned FallbackVersion = 52; LLVM_ABI ArrayRef<unsigned> getOpenMPVersions(); diff --git a/llvm/lib/Frontend/OpenMP/OMP.cpp b/llvm/lib/Frontend/OpenMP/OMP.cpp index 555e2a61e411e..c23fb400666d6 100644 --- a/llvm/lib/Frontend/OpenMP/OMP.cpp +++ b/llvm/lib/Frontend/OpenMP/OMP.cpp @@ -189,6 +189,20 @@ bool isCombinedConstruct(Directive D) { return !getLeafConstructs(D).empty() && !isCompositeConstruct(D); } +bool isMapEnteringConstruct(Directive D) { + for (Directive L : getLeafConstructsOrSelf(D)) { + switch (L) { + case OMPD_target: + case OMPD_target_data: + case OMPD_target_enter_data: + return true; + default: + break; + } + } + return false; +} + ArrayRef<unsigned> getOpenMPVersions() { static unsigned Versions[]{31, 40, 45, 50, 51, 52, 60}; return Versions; >From 0733ec13ae69e2ec3d462987c279ef083890e95c Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <krzysztof.parzys...@amd.com> Date: Wed, 16 Jul 2025 11:41:55 -0500 Subject: [PATCH 2/3] Simplify converting MapType, NFC --- flang/lib/Lower/OpenMP/Clauses.cpp | 47 +++++++++++++----------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 18e49719b6013..ece4a8eaac8cd 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -1001,7 +1001,20 @@ Map make(const parser::OmpClause::Map &inp, semantics::SemanticsContext &semaCtx) { // inp.v -> parser::OmpMapClause CLAUSET_ENUM_CONVERT( // - convert1, parser::OmpMapTypeModifier::Value, Map::MapTypeModifier, + convert1, parser::OmpMapType::Value, Map::MapType, + // clang-format off + MS(Alloc, Storage) + MS(Delete, Storage) + MS(Release, Storage) + MS(Storage, Storage) + MS(From, From) + MS(To, To) + MS(Tofrom, Tofrom) + // clang-format on + ); + + CLAUSET_ENUM_CONVERT( // + convert2, parser::OmpMapTypeModifier::Value, Map::MapTypeModifier, // clang-format off MS(Always, Always) MS(Close, Close) @@ -1011,7 +1024,7 @@ Map make(const parser::OmpClause::Map &inp, ); CLAUSET_ENUM_CONVERT( // - convert2, parser::OmpRefModifier::Value, Map::RefModifier, + convert3, parser::OmpRefModifier::Value, Map::RefModifier, // clang-format off MS(Ref_Ptee, RefPtee) MS(Ref_Ptr, RefPtr) @@ -1027,29 +1040,9 @@ Map make(const parser::OmpClause::Map &inp, auto &t2 = std::get<parser::OmpObjectList>(inp.v.t); auto type = [&]() -> std::optional<Map::MapType> { - std::optional<Map::MapType> type; - if (t1) { - switch (t1->v) { - case parser::OmpMapType::Value::Alloc: - case parser::OmpMapType::Value::Delete: - case parser::OmpMapType::Value::Release: - case parser::OmpMapType::Value::Storage: - type = Map::MapType::Storage; - break; - case parser::OmpMapType::Value::From: - type = Map::MapType::From; - break; - case parser::OmpMapType::Value::To: - type = Map::MapType::To; - break; - case parser::OmpMapType::Value::Tofrom: - type = Map::MapType::Tofrom; - break; - default: - break; - } - } - return type; + if (t1) + return convert1(t1->v); + return std::nullopt; }(); llvm::DenseSet<Map::MapTypeModifier> modSet; @@ -1058,7 +1051,7 @@ Map make(const parser::OmpClause::Map &inp, for (auto *typeMod : semantics::OmpGetRepeatableModifier<parser::OmpMapTypeModifier>(mods)) { - modSet.insert(convert1(typeMod->v)); + modSet.insert(convert2(typeMod->v)); } if (semantics::OmpGetUniqueModifier<parser::OmpAlwaysModifier>(mods)) modSet.insert(Map::MapTypeModifier::Always); @@ -1079,7 +1072,7 @@ Map make(const parser::OmpClause::Map &inp, auto refMod = [&]() -> std::optional<Map::RefModifier> { if (auto *t = semantics::OmpGetUniqueModifier<parser::OmpRefModifier>(mods)) - return convert2(t->v); + return convert3(t->v); return std::nullopt; }(); >From a7476b1ca983ecf0244b71ab873d3775343e5c46 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <krzysztof.parzys...@amd.com> Date: Wed, 16 Jul 2025 12:43:14 -0500 Subject: [PATCH 3/3] Fix unit test --- .../Frontend/OpenMPDecompositionTest.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp index 6189d0954891b..95c26b10c9a0c 100644 --- a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp +++ b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp @@ -431,8 +431,8 @@ TEST_F(OpenMPDecompositionTest, Firstprivate3) { std::string Dir0 = stringify(Dec.output[0]); std::string Dir1 = stringify(Dec.output[1]); std::string Dir2 = stringify(Dec.output[2]); - ASSERT_EQ(Dir0, "target map(2, , , , (x))"); // (12), (27) - ASSERT_EQ(Dir1, "teams shared(x)"); // (6), (17) + ASSERT_EQ(Dir0, "target map(2, , , , , (x))"); // (12), (27) + ASSERT_EQ(Dir1, "teams shared(x)"); // (6), (17) ASSERT_EQ(Dir2, "distribute firstprivate(x) lastprivate(, (x))"); // (5), (21) } @@ -574,9 +574,9 @@ TEST_F(OpenMPDecompositionTest, Lastprivate3) { std::string Dir0 = stringify(Dec.output[0]); std::string Dir1 = stringify(Dec.output[1]); std::string Dir2 = stringify(Dec.output[2]); - ASSERT_EQ(Dir0, "target map(2, , , , (x))"); // (21), (27) - ASSERT_EQ(Dir1, "parallel shared(x)"); // (22) - ASSERT_EQ(Dir2, "do lastprivate(, (x))"); // (21) + ASSERT_EQ(Dir0, "target map(2, , , , , (x))"); // (21), (27) + ASSERT_EQ(Dir1, "parallel shared(x)"); // (22) + ASSERT_EQ(Dir2, "do lastprivate(, (x))"); // (21) } // SHARED @@ -984,9 +984,9 @@ TEST_F(OpenMPDecompositionTest, Reduction7) { std::string Dir0 = stringify(Dec.output[0]); std::string Dir1 = stringify(Dec.output[1]); std::string Dir2 = stringify(Dec.output[2]); - ASSERT_EQ(Dir0, "target map(2, , , , (x))"); // (36), (10) - ASSERT_EQ(Dir1, "parallel shared(x)"); // (36), (1), (4) - ASSERT_EQ(Dir2, "do reduction(, (3), (x))"); // (36) + ASSERT_EQ(Dir0, "target map(2, , , , , (x))"); // (36), (10) + ASSERT_EQ(Dir1, "parallel shared(x)"); // (36), (1), (4) + ASSERT_EQ(Dir2, "do reduction(, (3), (x))"); // (36) } // IF _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits