[flang-commits] [flang] [Flang][OpenMP] Fix broken offload test caused by MapInfoFinalization code movement (PR #226384)
via flang-commits
flang-commits at lists.llvm.org
Fri Sep 25 00:17:43 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp
Author: agozillon
<details>
<summary>Changes</summary>
This change addresses the breakage in target-map-nested-dtype-allocatable-member.f90 I inadvertently broke it by removing the excess bit of implicit mapping code in the MapInfoFinalizaiton pass without correctly addressing it.
This PR tries to do so by allowing mappers to be generated for member maps such as var1%b, var2%b, which were previously being blocked.
Currently (and conveniently for this patch) we do avoid duplicate maps in the scenario where we specify something along the lines of:
map(tofrom: var1, var1%b, var2, var2%b)
But that is through the fact that we completely negate var1/var2's mapper attachment in this scenario, which I do not believe is the correct behavior. But it's the existing behavior and facilitates this patch, but it is something I'll see if I can address in a follow up PR in the near future, the logic might need a bit of debating. For now this simply seeks to amend the existing test without rocking the boat more than required (that's the hope at least).
---
Full diff: https://github.com/llvm/llvm-project/pull/226384.diff
4 Files Affected:
- (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+1-1)
- (modified) flang/lib/Lower/OpenMP/Utils.cpp (+7-6)
- (modified) flang/lib/Lower/OpenMP/Utils.h (+6-10)
- (added) flang/test/Lower/OpenMP/target-map-nested-dtype-allocatable-member.f90 (+31)
``````````diff
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index b871f630d3dae..e41223b5b16c0 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1918,7 +1918,7 @@ void ClauseProcessor::processMapObjects(
mlir::FlatSymbolRefAttr mapperId =
resolveMapperId(converter, clauseLocation, object, mapperIdNameRef,
- mapTypeBits, directive, parentObj.has_value());
+ mapTypeBits, directive);
// Explicit map captures are captured ByRef by default,
// optimisation passes may alter this to ByCopy or other capture
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index b52c03ff63fed..964c3a2592a2b 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -1229,12 +1229,11 @@ resolveMapperId(Fortran::lower::AbstractConverter &converter,
mlir::Location loc, const omp::Object &object,
llvm::StringRef mapperIdNameRef,
mlir::omp::ClauseMapFlags mapTypeBits,
- llvm::omp::Directive directive, bool hasParentObj) {
+ llvm::omp::Directive directive) {
const semantics::DerivedTypeSpec *objectTypeSpec =
getSymbolDerivedType(*object.sym());
if (!objectTypeSpec)
return mlir::FlatSymbolRefAttr();
-
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
mlir::FlatSymbolRefAttr mapperId;
std::string mapperIdName = mapperIdNameRef.str();
@@ -1259,10 +1258,11 @@ resolveMapperId(Fortran::lower::AbstractConverter &converter,
// specification.
auto *userDefinedDefault =
converter.getModuleOp().lookupSymbol(mapperIdName);
- if (!userDefinedDefault && !hasParentObj &&
- (directive != llvm::omp::Directive::OMPD_target_enter_data &&
- directive != llvm::omp::Directive::OMPD_target_exit_data &&
- directive != llvm::omp::Directive::OMPD_target_update)) {
+
+ if (!userDefinedDefault &&
+ directive != llvm::omp::Directive::OMPD_target_enter_data &&
+ directive != llvm::omp::Directive::OMPD_target_exit_data &&
+ directive != llvm::omp::Directive::OMPD_target_update) {
bool isAllocOrPointer =
semantics::IsAllocatableOrObjectPointer(object.sym());
bool isPointer = semantics::IsPointer(*object.sym());
@@ -1271,6 +1271,7 @@ resolveMapperId(Fortran::lower::AbstractConverter &converter,
mlir::omp::ClauseMapFlags::implicit;
bool needsDefaultMapper =
requiresImplicitDefaultDeclareMapper(*objectTypeSpec);
+
// For implicit captures, avoid synthesizing default mappers for
// pointer entities (which can over-map pointer payloads) and for
// plain non-allocatable/non-pointer entities. Keep implicit mapper
diff --git a/flang/lib/Lower/OpenMP/Utils.h b/flang/lib/Lower/OpenMP/Utils.h
index 7fde1825b006a..4560c9df349b2 100644
--- a/flang/lib/Lower/OpenMP/Utils.h
+++ b/flang/lib/Lower/OpenMP/Utils.h
@@ -236,9 +236,8 @@ mlir::Value genIteratorCoordinate(Fortran::lower::AbstractConverter &converter,
/// Resolve the declare mapper symbol to attach to a mapped object.
///
/// The default mapper path first looks for a user-defined mapper. If none
-/// exists, it may synthesize a compiler-generated mapper, except for mapped
-/// members whose parent object is also mapped and for target enter data,
-/// target exit data, and target update directives.
+/// exists, it may synthesize a compiler-generated mapper, except for target
+/// enter data, target exit data, and target update directives.
///
/// \param converter The converter used to query and generate mapper symbols.
/// \param loc The location to use when generating an implicit mapper.
@@ -248,15 +247,12 @@ mlir::Value genIteratorCoordinate(Fortran::lower::AbstractConverter &converter,
/// \param mapTypeBits The map flags used when deciding whether an implicit
/// mapper should be generated.
/// \param directive The enclosing OpenMP directive.
-/// \param hasParentObj True if a mapped parent object already owns this object.
/// \return A symbol reference to the resolved mapper, or a null attribute when
/// no mapper applies.
-mlir::FlatSymbolRefAttr
-resolveMapperId(Fortran::lower::AbstractConverter &converter,
- mlir::Location loc, const omp::Object &object,
- llvm::StringRef mapperIdName,
- mlir::omp::ClauseMapFlags mapTypeBits,
- llvm::omp::Directive directive, bool hasParentObj);
+mlir::FlatSymbolRefAttr resolveMapperId(
+ Fortran::lower::AbstractConverter &converter, mlir::Location loc,
+ const omp::Object &object, llvm::StringRef mapperIdName,
+ mlir::omp::ClauseMapFlags mapTypeBits, llvm::omp::Directive directive);
std::optional<llvm::SmallVector<mlir::Value>> getIteratorElementIndices(
Fortran::lower::AbstractConverter &converter, const omp::Object &object,
diff --git a/flang/test/Lower/OpenMP/target-map-nested-dtype-allocatable-member.f90 b/flang/test/Lower/OpenMP/target-map-nested-dtype-allocatable-member.f90
new file mode 100644
index 0000000000000..9d39de0c4ee51
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-map-nested-dtype-allocatable-member.f90
@@ -0,0 +1,31 @@
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! Check that explicit maps of derived-type components can still get an
+! implicit default mapper when the component type requires deep-copy mapping.
+! The synthetic parent maps for var1/var2 are only structural containers for
+! the explicit var1%b/var2%b list items and must not suppress mapper emission
+! for those list items.
+
+subroutine target_map_nested_dtype_allocatable_member
+ type :: inner_type
+ integer, allocatable :: a
+ end type inner_type
+
+ type :: outer_type
+ type(inner_type) :: b
+ end type outer_type
+
+ type(outer_type) :: var1, var2
+
+ !$omp target map(tofrom: var1%b, var2%b)
+ var1%b%a = var2%b%a
+ !$omp end target
+end subroutine
+
+! CHECK: omp.declare_mapper @{{.*}}inner_type_omp_default_mapper
+
+! CHECK-LABEL: func.func @_QPtarget_map_nested_dtype_allocatable_member
+! CHECK: %[[VAR1_B:.*]] = omp.map.info {{.*}}map_clauses(tofrom){{.*}}mapper(@{{.*}}inner_type_omp_default_mapper){{.*}}name("var1%b")
+! CHECK: %[[VAR2_B:.*]] = omp.map.info {{.*}}map_clauses(tofrom){{.*}}mapper(@{{.*}}inner_type_omp_default_mapper){{.*}}name("var2%b")
+! CHECK: omp.map.info {{.*}}map_clauses(storage){{.*}}members(%[[VAR1_B]] : [0] :{{.*}}){{.*}}name("var1"){{.*}}partial_map(true)
+! CHECK: omp.map.info {{.*}}map_clauses(storage){{.*}}members(%[[VAR2_B]] : [0] :{{.*}}){{.*}}name("var2"){{.*}}partial_map(true)
``````````
</details>
https://github.com/llvm/llvm-project/pull/226384
More information about the flang-commits
mailing list