[Mlir-commits] [llvm] [mlir] [MLIR][OpenMP] Fix mapper being attached to partial maps. (PR #178247)
Akash Banerjee
llvmlistbot at llvm.org
Tue Jan 27 08:16:44 PST 2026
https://github.com/TIFitis created https://github.com/llvm/llvm-project/pull/178247
Fix OpenMP mapper lowering by attaching user-defined/default mappers only to the base parent entry, not combined/segment entries. This prevents mapper calls with partial sizes. Added relevant tests.
>From 9c541635a23c96933c350a716c73c0f5989f1368 Mon Sep 17 00:00:00 2001
From: Akash Banerjee <Akash.Banerjee at amd.com>
Date: Tue, 27 Jan 2026 16:04:02 +0000
Subject: [PATCH] [MLIR][OpenMP] Fix mapper being attached to partial maps.
Fix OpenMP mapper lowering by attaching user-defined/default mappers only to the base parent entry, not combined/segment entries.
This prevents mapper calls with partial sizes. Added relevant tests.
---
.../OpenMP/OpenMPToLLVMIRTranslation.cpp | 23 ++++----
.../omptarget-mapper-combined-entry.mlir | 42 +++++++++++++++
...pper-derived-enter-data-teams-collapse.f90 | 53 +++++++++++++++++++
3 files changed, 109 insertions(+), 9 deletions(-)
create mode 100644 mlir/test/Target/LLVMIR/omptarget-mapper-combined-entry.mlir
create mode 100644 offload/test/offloading/fortran/default-mapper-derived-enter-data-teams-collapse.f90
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 022322502a755..ccefe1715604b 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4864,6 +4864,11 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
assert(!ompBuilder.Config.isTargetDevice() &&
"function only supported for host device codegen");
+ auto parentClause =
+ llvm::cast<omp::MapInfoOp>(mapData.MapClause[mapDataIndex]);
+
+ auto *parentMapper = mapData.Mappers[mapDataIndex];
+
// Map the first segment of the parent. If a user-defined mapper is attached,
// include the parent's to/from-style bits (and common modifiers) in this
// base entry so the mapper receives correct copy semantics via its 'type'
@@ -4874,9 +4879,7 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TARGET_PARAM
: llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE;
- // Detect if this mapping uses a user-defined mapper.
- bool hasUserMapper = mapData.Mappers[mapDataIndex] != nullptr;
- if (hasUserMapper) {
+ if (parentMapper) {
using mapFlags = llvm::omp::OpenMPOffloadMappingFlags;
// Preserve relevant map-type bits from the parent clause. These include
// the copy direction (TO/FROM), as well as commonly used modifiers that
@@ -4891,7 +4894,11 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
combinedInfo.Types.emplace_back(baseFlag);
combinedInfo.DevicePointers.emplace_back(
mapData.DevicePointers[mapDataIndex]);
- combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]);
+ // Only attach the mapper to the base entry when we are mapping the whole
+ // parent. Combined/segment entries must not carry a mapper; otherwise the
+ // mapper can be invoked with a partial size, which is undefined behaviour.
+ combinedInfo.Mappers.emplace_back(
+ parentMapper && !parentClause.getPartialMap() ? parentMapper : nullptr);
combinedInfo.Names.emplace_back(LLVM::createMappingInformation(
mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder));
combinedInfo.BasePointers.emplace_back(mapData.BasePointers[mapDataIndex]);
@@ -4903,8 +4910,6 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
// Fortran pointers and allocatables, the mapping of the pointed to
// data by the descriptor (which itself, is a structure containing
// runtime information on the dynamically allocated data).
- auto parentClause =
- llvm::cast<omp::MapInfoOp>(mapData.MapClause[mapDataIndex]);
llvm::Value *lowAddr, *highAddr;
if (!parentClause.getPartialMap()) {
lowAddr = builder.CreatePointerCast(mapData.Pointers[mapDataIndex],
@@ -4965,7 +4970,7 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
mapData.BasePointers[mapDataIndex]);
combinedInfo.Pointers.emplace_back(mapData.Pointers[mapDataIndex]);
combinedInfo.Sizes.emplace_back(mapData.Sizes[mapDataIndex]);
- combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]);
+ combinedInfo.Mappers.emplace_back(nullptr);
} else {
llvm::SmallVector<size_t> overlapIdxs;
// Find all of the members that "overlap", i.e. occlude other members that
@@ -4999,7 +5004,7 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder));
combinedInfo.BasePointers.emplace_back(
mapData.BasePointers[mapDataIndex]);
- combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]);
+ combinedInfo.Mappers.emplace_back(nullptr);
combinedInfo.Pointers.emplace_back(lowAddr);
combinedInfo.Sizes.emplace_back(builder.CreateIntCast(
builder.CreatePtrDiff(builder.getInt8Ty(),
@@ -5021,7 +5026,7 @@ static llvm::omp::OpenMPOffloadMappingFlags mapParentWithMembers(
mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder));
combinedInfo.BasePointers.emplace_back(
mapData.BasePointers[mapDataIndex]);
- combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]);
+ combinedInfo.Mappers.emplace_back(nullptr);
combinedInfo.Pointers.emplace_back(lowAddr);
combinedInfo.Sizes.emplace_back(builder.CreateIntCast(
builder.CreatePtrDiff(builder.getInt8Ty(), highAddr, lowAddr),
diff --git a/mlir/test/Target/LLVMIR/omptarget-mapper-combined-entry.mlir b/mlir/test/Target/LLVMIR/omptarget-mapper-combined-entry.mlir
new file mode 100644
index 0000000000000..1cf16183b943b
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-mapper-combined-entry.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Ensure that user-defined mappers are only attached to the base entry of a
+// combined parent mapping. Attaching the mapper to segment entries can invoke
+// the mapper with a partial size, which is undefined behaviour.
+
+module attributes {omp.target_triples = ["amdgcn-amd-amdhsa"]} {
+ omp.declare_mapper @mapper : !llvm.struct<"S", (i32, i32)> {
+ ^bb0(%arg0: !llvm.ptr):
+ %field0 = llvm.getelementptr %arg0[0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<"S", (i32, i32)>
+ %map_field0 = omp.map.info var_ptr(%field0 : !llvm.ptr, i32)
+ map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ %map_parent = omp.map.info var_ptr(%arg0 : !llvm.ptr, !llvm.struct<"S", (i32, i32)>)
+ map_clauses(tofrom) capture(ByRef) members(%map_field0 : [0] : !llvm.ptr) -> !llvm.ptr
+ {name = "", partial_map = true}
+ omp.declare_mapper.info map_entries(%map_parent, %map_field0 : !llvm.ptr, !llvm.ptr)
+ }
+
+ llvm.func @test_mapper_combined_entries() {
+ %one = llvm.mlir.constant(1 : i64) : i64
+ %s = llvm.alloca %one x !llvm.struct<"S", (i32, i32)> : (i64) -> !llvm.ptr
+ %field0 = llvm.getelementptr %s[0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<"S", (i32, i32)>
+ %map_field0 = omp.map.info var_ptr(%field0 : !llvm.ptr, i32)
+ map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = "s.f0"}
+ %map_parent = omp.map.info var_ptr(%s : !llvm.ptr, !llvm.struct<"S", (i32, i32)>)
+ map_clauses(tofrom) capture(ByRef) mapper(@mapper) members(%map_field0 : [0] : !llvm.ptr) -> !llvm.ptr
+ {name = "s"}
+ omp.target map_entries(%map_parent -> %arg0, %map_field0 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+ omp.terminator
+ }
+ llvm.return
+ }
+}
+
+// CHECK-LABEL: define void @test_mapper_combined_entries
+// CHECK: %[[MAPPERS:.*offload_mappers.*]] = alloca [4 x ptr]
+// CHECK: %[[MAPPER0:.*]] = getelementptr inbounds [4 x ptr], ptr %[[MAPPERS]], i64 0, i64 0
+// CHECK: store ptr @.omp_mapper.mapper, ptr %[[MAPPER0]]
+// CHECK: %[[MAPPER1:.*]] = getelementptr inbounds [4 x ptr], ptr %[[MAPPERS]], i64 0, i64 1
+// CHECK: store ptr null, ptr %[[MAPPER1]]
+// CHECK: %[[MAPPER2:.*]] = getelementptr inbounds [4 x ptr], ptr %[[MAPPERS]], i64 0, i64 2
+// CHECK: store ptr null, ptr %[[MAPPER2]]
diff --git a/offload/test/offloading/fortran/default-mapper-derived-enter-data-teams-collapse.f90 b/offload/test/offloading/fortran/default-mapper-derived-enter-data-teams-collapse.f90
new file mode 100644
index 0000000000000..6b87e81b472e6
--- /dev/null
+++ b/offload/test/offloading/fortran/default-mapper-derived-enter-data-teams-collapse.f90
@@ -0,0 +1,53 @@
+! Regression test for default mappers on nested derived types with allocatable
+! members when mapping a parent object and running an optimized target region.
+
+! REQUIRES: flang, amdgpu
+
+! RUN: %libomptarget-compile-fortran-generic -O3
+! RUN: %libomptarget-run-generic | %fcheck-generic
+
+program test_default_mapper_enter_data_teams_collapse
+ implicit none
+
+ type inner_type
+ real, allocatable :: data(:)
+ end type inner_type
+
+ type outer_type
+ type(inner_type) :: inner
+ character(len=19) :: desc = ' '
+ end type outer_type
+
+ type(outer_type) :: obj
+ integer, parameter :: n = 10
+ integer :: i, j
+ real :: expected, actual
+
+ allocate(obj%inner%data(n))
+ obj%inner%data = 0.0
+
+ !$omp target enter data map(to: obj)
+
+ !$omp target teams distribute parallel do collapse(2)
+ do i = 1, n
+ do j = 1, n
+ obj%inner%data(i) = real(i)
+ end do
+ end do
+ !$omp end target teams distribute parallel do
+
+ !$omp target exit data map(from: obj)
+
+ expected = real(n * (n + 1)) / 2.0
+ actual = sum(obj%inner%data)
+
+ if (abs(actual - expected) < 1.0e-6) then
+ print *, "PASS"
+ else
+ print *, "FAIL", actual, expected
+ end if
+
+ deallocate(obj%inner%data)
+end program test_default_mapper_enter_data_teams_collapse
+
+! CHECK: PASS
More information about the Mlir-commits
mailing list