[flang-commits] [flang] [llvm] [Flang][OpenMP] Remove close map type member removal code from MapInfoFinalization (PR #219438)
via flang-commits
flang-commits at lists.llvm.org
Tue Sep 15 13:58:21 PDT 2026
https://github.com/agozillon updated https://github.com/llvm/llvm-project/pull/219438
>From 5ca0b0f27bab837c38b18ce6f629580e8ace41ec Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Fri, 28 Aug 2026 05:51:26 -0500
Subject: [PATCH] [Flang][OpenMP] Remove close removal code from
MapInfoFinalization
This should no longer be required as we removed the application of close
automatically in USM mode in a prior PR. So they bug this was originally
put in place for is now addressed without the need for this PR. And whilst
it was fine to put in place for a bug the user had no control of (automatic
application of close to descriptor members), it's not our place to intervene
in scenarios where a user is making explicit errors in their mapping, e.g.
map(to: dtype) map(close, to: dtype%x, dtypey)
A user should be left to shoot themselves in the foot if that's there goal,
we'd ideally prevent this as a compiler warning or error as opposed to fixing
the problem under-the-hood for them. Otherwise we help create unportable code
for the user, alongside forcing ourselves to maintain the underlying fix.
---
flang/lib/Lower/OpenMP/Utils.cpp | 39 ++++++
.../Optimizer/OpenMP/MapInfoFinalization.cpp | 35 -----
flang/lib/Semantics/check-omp-structure.cpp | 128 ++++++++++++++++++
flang/lib/Semantics/check-omp-structure.h | 1 +
.../OpenMP/close-map-derived-members.f90 | 47 +++++++
flang/test/Semantics/OpenMP/map-modifiers.f90 | 40 ++++++
.../usm_map_close_derived_type_members.f90 | 42 ++++++
7 files changed, 297 insertions(+), 35 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/close-map-derived-members.f90
create mode 100644 offload/test/offloading/fortran/usm_map_close_derived_type_members.f90
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index 4244662ad4290f..b52c03ff63fed0 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -573,6 +573,33 @@ void insertChildMapInfoIntoParent(
llvm::SmallVectorImpl<mlir::Value> &mapOperands,
llvm::SmallVectorImpl<Object> &mapObjects) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+
+ // If any of our children have the close map type applied and our parent
+ // does not, we must remove it as a user has specified only the members
+ // get close e.g.
+ //
+ // map(close, to: x%y, x%z)
+ //
+ // This will generate invalid code that will crash at runtime. As we must
+ // allocate this parent, to then transfer individual members, and the
+ // parent has not been specified to have close mapping. As close is
+ // considered a hint, we simply remove it/ignore it.
+ //
+ // The alternative would be to promote the parent to close in these
+ // scenarios, the downside would be cases such as map(close, to: x%x, x%z)
+ // any intermediate member that was not specified in the parent between
+ // the x and z members (e.g. x.y in prior example) would also get close
+ // mapping from the parent. But this way we would still be able to have
+ // close map apply to member mappings, without having to allocate the whole
+ // record type.
+ auto removeCloseMapType = [](OmpMapParentAndMemberData &data) {
+ for (mlir::omp::MapInfoOp memberMap : data.memberMap)
+ if ((memberMap.getMapType() & mlir::omp::ClauseMapFlags::close) ==
+ mlir::omp::ClauseMapFlags::close)
+ memberMap.setMapType(memberMap.getMapType() &
+ ~mlir::omp::ClauseMapFlags::close);
+ };
+
for (auto indices : parentMemberIndices) {
auto *parentIter =
llvm::find_if(mapObjects, [&indices](const Object &object) {
@@ -589,6 +616,13 @@ void insertChildMapInfoIntoParent(
if (!indices.second.memberMap.empty() && mapOp.getMapperIdAttr())
mapOp.setMapperIdAttr(nullptr);
+ // If the explicit parent map itself does not have close mapping, then
+ // close mapping for its explicitly mapped members must be ignored. The
+ // semantic checker emits the corresponding user warning.
+ if ((mapOp.getMapType() & mlir::omp::ClauseMapFlags::close) !=
+ mlir::omp::ClauseMapFlags::close)
+ removeCloseMapType(indices.second);
+
// NOTE: To maintain appropriate SSA ordering, we move the parent map
// which will now have references to its children after the last
// of its members to be generated. This is necessary when a user
@@ -617,6 +651,11 @@ void insertChildMapInfoIntoParent(
mlir::omp::ClauseMapFlags::present)
mapType |= mlir::omp::ClauseMapFlags::present;
+ // A synthesized parent map cannot request close mapping. Remove close
+ // mapping from its explicitly mapped members. The semantic checker emits
+ // the corresponding user warning.
+ removeCloseMapType(indices.second);
+
llvm::SmallVector<mlir::Value> members;
members.reserve(indices.second.memberMap.size());
for (mlir::omp::MapInfoOp memberMap : indices.second.memberMap)
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 881be0064a7f94..3b264a196aa4ac 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -1510,41 +1510,6 @@ class MapInfoFinalizationPass
}
});
- func->walk([&](mlir::omp::MapInfoOp op) {
- // If a record type is not mapped with the `close` modifier while some
- // of its members are (e.g. descriptor maps), then in USM mode, the
- // memory for the record will be allocated in unified memory while the
- // the members might be allocated in device memory. This creates an
- // inconsistent map for the record type where some of its members are
- // allocated in different address spaces.
- //
- // This fixes this issue by taking a conservative approach and removing
- // the `close` flag from members if it is not used for mapping the
- // parent record.
- if (op.getMembers().empty())
- return;
-
- mlir::Type varTy = fir::unwrapRefType(op.getVarPtr().getType());
- if (!mlir::isa<fir::RecordType>(varTy))
- return;
-
- auto mapFlag = op.getMapType();
- bool hasClose = (mapFlag & mlir::omp::ClauseMapFlags::close) ==
- mlir::omp::ClauseMapFlags::close;
-
- if (hasClose)
- return;
-
- for (auto member : op.getMembers()) {
- if (auto memberOp = llvm::dyn_cast_if_present<mlir::omp::MapInfoOp>(
- member.getDefiningOp())) {
- auto memberMapFlag =
- memberOp.getMapType() & ~mlir::omp::ClauseMapFlags::close;
- memberOp.setMapType(memberMapFlag);
- }
- }
- });
-
// Now that we've expanded all of our boxes into a descriptor and base
// address map where necessary, we check if the map owner is an
// enter/exit/target data directive, and if they are we drop the initial
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index dc12d06082bc04..f282140e47b6b6 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -72,6 +72,94 @@ void IterateOverMembers(const llvm::omp::Clauses &set,
}
}
+static const parser::Name *GetBaseName(const parser::DataRef &dataRef);
+
+static bool ContainsStructureComponent(const parser::DataRef &dataRef) {
+ return common::visit(
+ common::visitors{
+ [](const parser::Name &) { return false; },
+ [](const common::Indirection<parser::StructureComponent> &) {
+ return true;
+ },
+ [](const common::Indirection<parser::ArrayElement> &x) {
+ return ContainsStructureComponent(x.value().Base());
+ },
+ [](const common::Indirection<parser::CoindexedNamedObject> &x) {
+ return ContainsStructureComponent(
+ std::get<parser::DataRef>(x.value().t));
+ },
+ },
+ dataRef.u);
+}
+
+static bool ContainsStructureComponent(const parser::OmpObject &object) {
+ if (const auto *designator{GetDesignatorFromObj(object)}) {
+ return common::visit(common::visitors{
+ [](const parser::DataRef &dataRef) {
+ return ContainsStructureComponent(dataRef);
+ },
+ [](const parser::Substring &substring) {
+ return ContainsStructureComponent(
+ std::get<parser::DataRef>(substring.t));
+ },
+ },
+ designator->u);
+ }
+ return false;
+}
+
+static const parser::Name *GetBaseName(const parser::DataRef &dataRef) {
+ return common::visit(
+ common::visitors{
+ [](const parser::Name &name) { return &name; },
+ [](const common::Indirection<parser::StructureComponent> &x) {
+ return GetBaseName(x.value().Base());
+ },
+ [](const common::Indirection<parser::ArrayElement> &x) {
+ return GetBaseName(x.value().Base());
+ },
+ [](const common::Indirection<parser::CoindexedNamedObject> &x) {
+ return GetBaseName(std::get<parser::DataRef>(x.value().t));
+ },
+ },
+ dataRef.u);
+}
+
+static const Symbol *GetBaseObjectSymbol(const parser::OmpObject &object) {
+ if (const parser::Name *name{GetCommonBlockFromObj(object)}) {
+ return name->symbol ? &name->symbol->GetUltimate() : nullptr;
+ }
+
+ if (const parser::Designator *designator{GetDesignatorFromObj(object)}) {
+ const parser::Name *name{common::visit(
+ common::visitors{
+ [](const parser::DataRef &dataRef) { return GetBaseName(dataRef); },
+ [](const parser::Substring &substring) {
+ return GetBaseName(std::get<parser::DataRef>(substring.t));
+ },
+ },
+ designator->u)};
+ return name && name->symbol ? &name->symbol->GetUltimate() : nullptr;
+ }
+
+ return nullptr;
+}
+
+static bool HasCloseMapModifier(const parser::OmpMapClause &clause) {
+ const auto &modifiers{OmpGetModifiers(clause)};
+ if (OmpGetUniqueModifier<parser::OmpCloseModifier>(modifiers)) {
+ return true;
+ }
+
+ for (const parser::OmpMapTypeModifier *modifier :
+ OmpGetRepeatableModifier<parser::OmpMapTypeModifier>(modifiers))
+ if (modifier->v == parser::OmpMapTypeModifier::Value::Close) {
+ return true;
+ }
+
+ return false;
+}
+
OmpStructureChecker::OmpStructureChecker(SemanticsContext &context)
: DirectiveStructureChecker(context,
#define GEN_FLANG_DIRECTIVE_CLAUSE_MAP
@@ -3796,6 +3884,7 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &x) {
// Semantic checks related to presence of multiple list items within the same
// clause
CheckMultListItems();
+ CheckCloseModifierOnMapMembers();
if (GetContext().directive == llvm::omp::Directive::OMPD_task) {
if (auto *detachClause{FindClause(llvm::omp::Clause::OMPC_detach)}) {
@@ -4811,6 +4900,45 @@ void OmpStructureChecker::CheckAllowedMapTypes(parser::OmpMapType::Value type,
llvm::join(names, ", "), ContextDirectiveAsFortran());
}
+void OmpStructureChecker::CheckCloseModifierOnMapMembers() {
+ std::set<const Symbol *> closeMappedParents;
+ llvm::SmallVector<std::pair<const Symbol *, parser::CharBlock>>
+ closeMappedMembers;
+
+ for (auto [_, clause] : FindClauses(llvm::omp::Clause::OMPC_map)) {
+ const auto &mapClause{std::get<parser::OmpClause::Map>(clause->u).v};
+
+ if (!HasCloseMapModifier(mapClause)) {
+ continue;
+ }
+
+ const parser::OmpObjectList &objects{
+ std::get<parser::OmpObjectList>(mapClause.t)};
+ for (const parser::OmpObject &object : objects.v) {
+ const Symbol *base{GetBaseObjectSymbol(object)};
+ if (!base) {
+ continue;
+ }
+
+ if (ContainsStructureComponent(object)) {
+ std::optional<parser::CharBlock> source{GetObjectSource(object)};
+ closeMappedMembers.emplace_back(base, source.value_or(clause->source));
+ } else {
+ closeMappedParents.insert(base);
+ }
+ }
+ }
+
+ std::set<const Symbol *> warnedParents;
+ for (const auto &[parent, source] : closeMappedMembers) {
+ if (closeMappedParents.count(parent) == 0 &&
+ warnedParents.insert(parent).second) {
+ context_.Say(source,
+ "OpenMP CLOSE map modifier ignored for structure component; map the base object with CLOSE to apply the modifier"_warn_en_US);
+ }
+ }
+}
+
void OmpStructureChecker::Enter(const parser::OmpClause::Map &x) {
auto &modifiers{OmpGetModifiers(x.v)};
llvm::omp::Version version{context_.langOptions().getOpenMPVersion()};
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 41b866337c4f1b..e0a86b09a5d9a8 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -419,6 +419,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
bool HasRequires(llvm::omp::Clause req);
void CheckAllowedMapTypes(
parser::OmpMapType::Value, llvm::ArrayRef<parser::OmpMapType::Value>);
+ void CheckCloseModifierOnMapMembers();
llvm::StringRef getClauseName(llvm::omp::Clause clause) override;
llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override;
diff --git a/flang/test/Lower/OpenMP/close-map-derived-members.f90 b/flang/test/Lower/OpenMP/close-map-derived-members.f90
new file mode 100644
index 00000000000000..f986b017c7926b
--- /dev/null
+++ b/flang/test/Lower/OpenMP/close-map-derived-members.f90
@@ -0,0 +1,47 @@
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 %s -o - | FileCheck %s
+
+subroutine close_member_no_parent()
+ type t
+ integer :: x, y
+ end type
+ type(t) :: s
+
+! CHECK-LABEL: func.func @_QPclose_member_no_parent()
+! CHECK-NOT: map_clauses(close
+! CHECK: %[[MEMBER:.*]] = omp.map.info {{.*}} map_clauses(tofrom) {{.*}} name("s%x") -> !fir.ref<i32>
+! CHECK: %[[PARENT:.*]] = omp.map.info {{.*}} map_clauses(storage) {{.*}} members(%[[MEMBER]] : [0] : !fir.ref<i32>) name("s") partial_map(true)
+! CHECK-NOT: map_clauses(close
+ !$omp target map(close, tofrom: s%x)
+ s%x = s%y
+ !$omp end target
+end
+
+subroutine close_member_close_parent()
+ type t
+ integer :: x, y
+ end type
+ type(t) :: s
+
+! CHECK-LABEL: func.func @_QPclose_member_close_parent()
+! CHECK: %[[MEMBER:.*]] = omp.map.info {{.*}} map_clauses(close, tofrom) {{.*}} name("s%x") -> !fir.ref<i32>
+! CHECK: %[[PARENT:.*]] = omp.map.info {{.*}} map_clauses(close, tofrom) {{.*}} members(%[[MEMBER]] : [0] : !fir.ref<i32>) name("s")
+ !$omp target map(close, tofrom: s) map(close, tofrom: s%x)
+ s%x = s%y
+ !$omp end target
+end
+
+subroutine close_member_nonclose_parent()
+ type t
+ integer :: x, y
+ end type
+ type(t) :: s
+
+! CHECK-LABEL: func.func @_QPclose_member_nonclose_parent()
+! CHECK-NOT: map_clauses(close
+! CHECK: %[[MEMBER:.*]] = omp.map.info {{.*}} map_clauses(tofrom) {{.*}} name("s%x") -> !fir.ref<i32>
+! CHECK: %[[PARENT:.*]] = omp.map.info {{.*}} map_clauses(tofrom) {{.*}} members(%[[MEMBER]] : [0] : !fir.ref<i32>) name("s")
+! CHECK-NOT: map_clauses(close
+ !$omp target map(tofrom: s) map(close, tofrom: s%x)
+ s%x = s%y
+ !$omp end target
+end
diff --git a/flang/test/Semantics/OpenMP/map-modifiers.f90 b/flang/test/Semantics/OpenMP/map-modifiers.f90
index 3851336c33f3d5..5c87378561c02c 100644
--- a/flang/test/Semantics/OpenMP/map-modifiers.f90
+++ b/flang/test/Semantics/OpenMP/map-modifiers.f90
@@ -104,3 +104,43 @@ subroutine f24(x)
x = x + 1
!$omp end target
end
+
+subroutine f25()
+ type t
+ integer :: x, y
+ end type
+ type(t) :: s
+
+!WARNING: OpenMP CLOSE map modifier ignored for structure component; map the base object with CLOSE to apply the modifier
+ !$omp target map(close, tofrom: s%x, s%y)
+ s%x = s%y
+ !$omp end target
+end
+
+subroutine f26()
+ type t
+ integer :: x, y
+ end type
+ type(t) :: s
+
+!WARNING: OpenMP CLOSE map modifier ignored for structure component; map the base object with CLOSE to apply the modifier
+ !$omp target map(tofrom: s) map(close, tofrom: s%x)
+ s%x = s%y
+ !$omp end target
+end
+
+subroutine f27()
+ type t
+ integer :: x, y
+ end type
+ type(t) :: s
+
+ !$omp target map(close, tofrom: s) map(close, tofrom: s%x)
+ s%x = s%y
+ !$omp end target
+
+!WARNING: OpenMP CLOSE map modifier ignored for structure component; map the base object with CLOSE to apply the modifier
+ !$omp target map(close, tofrom: s%x)
+ s%x = s%y
+ !$omp end target
+end
diff --git a/offload/test/offloading/fortran/usm_map_close_derived_type_members.f90 b/offload/test/offloading/fortran/usm_map_close_derived_type_members.f90
new file mode 100644
index 00000000000000..36777b11147956
--- /dev/null
+++ b/offload/test/offloading/fortran/usm_map_close_derived_type_members.f90
@@ -0,0 +1,42 @@
+! Offloading test checking map(close, ...) with explicit derived type
+! component mappings in unified shared memory mode. When CLOSE is specified
+! only on structure components and the base object is not also mapped with
+! CLOSE, the compiler drops the CLOSE hint from the component mappings to avoid
+! invalid runtime mappings.
+! REQUIRES: flang, amdgpu, unified_shared_memory
+!
+! RUN: %libomptarget-compile-fortran-generic
+! RUN: env HSA_XNACK=1 %libomptarget-run-generic | %fcheck-generic
+
+program main
+ !$omp requires unified_shared_memory
+ type t
+ integer :: x, y
+ end type
+ type(t) :: member_only = t(1, 2)
+ type(t) :: close_parent = t(3, 4)
+ type(t) :: nonclose_parent = t(5, 6)
+
+ !$omp target map(close, tofrom: member_only%x, member_only%y)
+ member_only%x = 10
+ member_only%y = 20
+ !$omp end target
+
+ !$omp target map(close, tofrom: close_parent) map(close, tofrom: close_parent%x)
+ close_parent%x = 30
+ close_parent%y = 40
+ !$omp end target
+
+ !$omp target map(tofrom: nonclose_parent) map(close, tofrom: nonclose_parent%x)
+ nonclose_parent%x = 50
+ nonclose_parent%y = 60
+ !$omp end target
+
+ print *, member_only%x, member_only%y
+ print *, close_parent%x, close_parent%y
+ print *, nonclose_parent%x, nonclose_parent%y
+end program
+
+! CHECK: 10 20
+! CHECK: 30 40
+! CHECK: 50 60
More information about the flang-commits
mailing list