[flang] [llvm] Revert "[Flang][OpenMP] Implicitly map nested allocatable components in derived types" (PR #160759)
Akash Banerjee via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 25 11:53:55 PDT 2025
https://github.com/TIFitis created https://github.com/llvm/llvm-project/pull/160759
Reverts llvm/llvm-project#160116
>From 265c8d94a8a0e3cf227237d005ca541453b30a9a Mon Sep 17 00:00:00 2001
From: Akash Banerjee <akashbanerjeeab at icloud.com>
Date: Thu, 25 Sep 2025 19:53:37 +0100
Subject: [PATCH] =?UTF-8?q?Revert=20"[Flang][OpenMP]=20Implicitly=20map=20?=
=?UTF-8?q?nested=20allocatable=20components=20in=20deriv=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit b4f1e0e5b1735bc6bef32e93da1c9bbb428e2dd9.
---
.../Optimizer/OpenMP/MapInfoFinalization.cpp | 180 ++++++------------
flang/test/Lower/OpenMP/declare-mapper.f90 | 38 ----
...rget-declare-mapper-parent-allocatable.f90 | 43 -----
3 files changed, 55 insertions(+), 206 deletions(-)
delete mode 100644 offload/test/offloading/fortran/target-declare-mapper-parent-allocatable.f90
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 3659218f91ff6..57be863cfa1b8 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -701,28 +701,40 @@ class MapInfoFinalizationPass
auto recordType = mlir::cast<fir::RecordType>(underlyingType);
llvm::SmallVector<mlir::Value> newMapOpsForFields;
- llvm::SmallVector<llvm::SmallVector<int64_t>> newMemberIndexPaths;
+ llvm::SmallVector<int64_t> fieldIndicies;
- auto appendMemberMap = [&](mlir::Location loc, mlir::Value coordRef,
- mlir::Type memTy,
- llvm::ArrayRef<int64_t> indexPath,
- llvm::StringRef memberName) {
- // Check if already mapped (index path equality).
+ for (auto fieldMemTyPair : recordType.getTypeList()) {
+ auto &field = fieldMemTyPair.first;
+ auto memTy = fieldMemTyPair.second;
+
+ bool shouldMapField =
+ llvm::find_if(mapVarForwardSlice, [&](mlir::Operation *sliceOp) {
+ if (!fir::isAllocatableType(memTy))
+ return false;
+
+ auto designateOp = mlir::dyn_cast<hlfir::DesignateOp>(sliceOp);
+ if (!designateOp)
+ return false;
+
+ return designateOp.getComponent() &&
+ designateOp.getComponent()->strref() == field;
+ }) != mapVarForwardSlice.end();
+
+ // TODO Handle recursive record types. Adapting
+ // `createParentSymAndGenIntermediateMaps` to work direclty on MLIR
+ // entities might be helpful here.
+
+ if (!shouldMapField)
+ continue;
+
+ int32_t fieldIdx = recordType.getFieldIndex(field);
bool alreadyMapped = [&]() {
if (op.getMembersIndexAttr())
for (auto indexList : op.getMembersIndexAttr()) {
auto indexListAttr = mlir::cast<mlir::ArrayAttr>(indexList);
- if (indexListAttr.size() != indexPath.size())
- continue;
- bool allEq = true;
- for (auto [i, attr] : llvm::enumerate(indexListAttr)) {
- if (mlir::cast<mlir::IntegerAttr>(attr).getInt() !=
- indexPath[i]) {
- allEq = false;
- break;
- }
- }
- if (allEq)
+ if (indexListAttr.size() == 1 &&
+ mlir::cast<mlir::IntegerAttr>(indexListAttr[0]).getInt() ==
+ fieldIdx)
return true;
}
@@ -730,128 +742,42 @@ class MapInfoFinalizationPass
}();
if (alreadyMapped)
- return;
+ continue;
builder.setInsertionPoint(op);
+ fir::IntOrValue idxConst =
+ mlir::IntegerAttr::get(builder.getI32Type(), fieldIdx);
+ auto fieldCoord = fir::CoordinateOp::create(
+ builder, op.getLoc(), builder.getRefType(memTy), op.getVarPtr(),
+ llvm::SmallVector<fir::IntOrValue, 1>{idxConst});
fir::factory::AddrAndBoundsInfo info =
- fir::factory::getDataOperandBaseAddr(builder, coordRef,
- /*isOptional=*/false, loc);
+ fir::factory::getDataOperandBaseAddr(
+ builder, fieldCoord, /*isOptional=*/false, op.getLoc());
llvm::SmallVector<mlir::Value> bounds =
fir::factory::genImplicitBoundsOps<mlir::omp::MapBoundsOp,
mlir::omp::MapBoundsType>(
builder, info,
- hlfir::translateToExtendedValue(loc, builder,
- hlfir::Entity{coordRef})
+ hlfir::translateToExtendedValue(op.getLoc(), builder,
+ hlfir::Entity{fieldCoord})
.first,
- /*dataExvIsAssumedSize=*/false, loc);
+ /*dataExvIsAssumedSize=*/false, op.getLoc());
mlir::omp::MapInfoOp fieldMapOp = mlir::omp::MapInfoOp::create(
- builder, loc, coordRef.getType(), coordRef,
- mlir::TypeAttr::get(fir::unwrapRefType(coordRef.getType())),
+ builder, op.getLoc(), fieldCoord.getResult().getType(),
+ fieldCoord.getResult(),
+ mlir::TypeAttr::get(
+ fir::unwrapRefType(fieldCoord.getResult().getType())),
op.getMapTypeAttr(),
builder.getAttr<mlir::omp::VariableCaptureKindAttr>(
mlir::omp::VariableCaptureKind::ByRef),
/*varPtrPtr=*/mlir::Value{}, /*members=*/mlir::ValueRange{},
/*members_index=*/mlir::ArrayAttr{}, bounds,
/*mapperId=*/mlir::FlatSymbolRefAttr(),
- builder.getStringAttr(op.getNameAttr().strref() + "." +
- memberName + ".implicit_map"),
+ builder.getStringAttr(op.getNameAttr().strref() + "." + field +
+ ".implicit_map"),
/*partial_map=*/builder.getBoolAttr(false));
newMapOpsForFields.emplace_back(fieldMapOp);
- newMemberIndexPaths.emplace_back(indexPath.begin(), indexPath.end());
- };
-
- // 1) Handle direct top-level allocatable fields (existing behavior).
- for (auto fieldMemTyPair : recordType.getTypeList()) {
- auto &field = fieldMemTyPair.first;
- auto memTy = fieldMemTyPair.second;
-
- if (!fir::isAllocatableType(memTy))
- continue;
-
- bool referenced = llvm::any_of(mapVarForwardSlice, [&](auto *opv) {
- auto designateOp = mlir::dyn_cast<hlfir::DesignateOp>(opv);
- return designateOp && designateOp.getComponent() &&
- designateOp.getComponent()->strref() == field;
- });
- if (!referenced)
- continue;
-
- int32_t fieldIdx = recordType.getFieldIndex(field);
- builder.setInsertionPoint(op);
- fir::IntOrValue idxConst =
- mlir::IntegerAttr::get(builder.getI32Type(), fieldIdx);
- auto fieldCoord = fir::CoordinateOp::create(
- builder, op.getLoc(), builder.getRefType(memTy), op.getVarPtr(),
- llvm::SmallVector<fir::IntOrValue, 1>{idxConst});
- appendMemberMap(op.getLoc(), fieldCoord, memTy, {fieldIdx}, field);
- }
-
- // Handle nested allocatable fields along any component chain
- // referenced in the region via HLFIR designates.
- for (mlir::Operation *sliceOp : mapVarForwardSlice) {
- auto designateOp = mlir::dyn_cast<hlfir::DesignateOp>(sliceOp);
- if (!designateOp || !designateOp.getComponent())
- continue;
- llvm::SmallVector<llvm::StringRef> compPathReversed;
- compPathReversed.push_back(designateOp.getComponent()->strref());
- mlir::Value curBase = designateOp.getMemref();
- bool rootedAtMapArg = false;
- while (true) {
- if (auto parentDes = curBase.getDefiningOp<hlfir::DesignateOp>()) {
- if (!parentDes.getComponent())
- break;
- compPathReversed.push_back(parentDes.getComponent()->strref());
- curBase = parentDes.getMemref();
- continue;
- }
- if (auto decl = curBase.getDefiningOp<hlfir::DeclareOp>()) {
- if (auto barg =
- mlir::dyn_cast<mlir::BlockArgument>(decl.getMemref()))
- rootedAtMapArg = (barg == opBlockArg);
- } else if (auto blockArg =
- mlir::dyn_cast_or_null<mlir::BlockArgument>(
- curBase)) {
- rootedAtMapArg = (blockArg == opBlockArg);
- }
- break;
- }
- if (!rootedAtMapArg || compPathReversed.size() < 2)
- continue;
- builder.setInsertionPoint(op);
- llvm::SmallVector<int64_t> indexPath;
- mlir::Type curTy = underlyingType;
- mlir::Value coordRef = op.getVarPtr();
- bool validPath = true;
- for (llvm::StringRef compName : llvm::reverse(compPathReversed)) {
- auto recTy = mlir::dyn_cast<fir::RecordType>(curTy);
- if (!recTy) {
- validPath = false;
- break;
- }
- int32_t idx = recTy.getFieldIndex(compName);
- if (idx < 0) {
- validPath = false;
- break;
- }
- indexPath.push_back(idx);
- mlir::Type memTy = recTy.getType(idx);
- fir::IntOrValue idxConst =
- mlir::IntegerAttr::get(builder.getI32Type(), idx);
- coordRef = fir::CoordinateOp::create(
- builder, op.getLoc(), builder.getRefType(memTy), coordRef,
- llvm::SmallVector<fir::IntOrValue, 1>{idxConst});
- curTy = memTy;
- }
- if (!validPath)
- continue;
- if (auto finalRefTy =
- mlir::dyn_cast<fir::ReferenceType>(coordRef.getType())) {
- mlir::Type eleTy = finalRefTy.getElementType();
- if (fir::isAllocatableType(eleTy))
- appendMemberMap(op.getLoc(), coordRef, eleTy, indexPath,
- compPathReversed.front());
- }
+ fieldIndicies.emplace_back(fieldIdx);
}
if (newMapOpsForFields.empty())
@@ -859,8 +785,10 @@ class MapInfoFinalizationPass
op.getMembersMutable().append(newMapOpsForFields);
llvm::SmallVector<llvm::SmallVector<int64_t>> newMemberIndices;
- if (mlir::ArrayAttr oldAttr = op.getMembersIndexAttr())
- for (mlir::Attribute indexList : oldAttr) {
+ mlir::ArrayAttr oldMembersIdxAttr = op.getMembersIndexAttr();
+
+ if (oldMembersIdxAttr)
+ for (mlir::Attribute indexList : oldMembersIdxAttr) {
llvm::SmallVector<int64_t> listVec;
for (mlir::Attribute index : mlir::cast<mlir::ArrayAttr>(indexList))
@@ -868,8 +796,10 @@ class MapInfoFinalizationPass
newMemberIndices.emplace_back(std::move(listVec));
}
- for (auto &path : newMemberIndexPaths)
- newMemberIndices.emplace_back(path);
+
+ for (int64_t newFieldIdx : fieldIndicies)
+ newMemberIndices.emplace_back(
+ llvm::SmallVector<int64_t>(1, newFieldIdx));
op.setMembersIndexAttr(builder.create2DI64ArrayAttr(newMemberIndices));
op.setPartialMap(true);
diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90
index 1c51666c80f8a..8a98c68a8d582 100644
--- a/flang/test/Lower/OpenMP/declare-mapper.f90
+++ b/flang/test/Lower/OpenMP/declare-mapper.f90
@@ -6,7 +6,6 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-3.f90 -o - | FileCheck %t/omp-declare-mapper-3.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-4.f90 -o - | FileCheck %t/omp-declare-mapper-4.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-5.f90 -o - | FileCheck %t/omp-declare-mapper-5.f90
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=51 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
!--- omp-declare-mapper-1.f90
subroutine declare_mapper_1
@@ -263,40 +262,3 @@ subroutine use_inner()
!$omp end target
end subroutine
end program declare_mapper_5
-
-!--- omp-declare-mapper-6.f90
-subroutine declare_mapper_nested_parent
- type :: inner_t
- real, allocatable :: deep_arr(:)
- end type inner_t
-
- type, abstract :: base_t
- real, allocatable :: base_arr(:)
- type(inner_t) :: inner
- end type base_t
-
- type, extends(base_t) :: real_t
- real, allocatable :: real_arr(:)
- end type real_t
-
- !$omp declare mapper (custommapper : real_t :: t) map(tofrom: t%base_arr, t%real_arr)
-
- type(real_t) :: r
-
- allocate(r%base_arr(10))
- allocate(r%inner%deep_arr(10))
- allocate(r%real_arr(10))
- r%base_arr = 1.0
- r%inner%deep_arr = 4.0
- r%real_arr = 0.0
-
- ! CHECK: omp.target
- ! Check implicit maps for nested parent and deep nested allocatable payloads
- ! CHECK-DAG: omp.map.info {{.*}} {name = "r.base_arr.implicit_map"}
- ! CHECK-DAG: omp.map.info {{.*}} {name = "r.deep_arr.implicit_map"}
- ! The declared mapper's own allocatable is still mapped implicitly
- ! CHECK-DAG: omp.map.info {{.*}} {name = "r.real_arr.implicit_map"}
- !$omp target map(mapper(custommapper), tofrom: r)
- r%real_arr = r%base_arr(1) + r%inner%deep_arr(1)
- !$omp end target
-end subroutine declare_mapper_nested_parent
diff --git a/offload/test/offloading/fortran/target-declare-mapper-parent-allocatable.f90 b/offload/test/offloading/fortran/target-declare-mapper-parent-allocatable.f90
deleted file mode 100644
index bd5e6828bc991..0000000000000
--- a/offload/test/offloading/fortran/target-declare-mapper-parent-allocatable.f90
+++ /dev/null
@@ -1,43 +0,0 @@
-! This test validates that declare mapper for a derived type that extends
-! a parent type with an allocatable component correctly maps the nested
-! allocatable payload via the mapper when the whole object is mapped on
-! target.
-
-! REQUIRES: flang, amdgpu
-
-! RUN: %libomptarget-compile-fortran-run-and-check-generic
-
-program target_declare_mapper_parent_allocatable
- implicit none
-
- type, abstract :: base_t
- real, allocatable :: base_arr(:)
- end type base_t
-
- type, extends(base_t) :: real_t
- real, allocatable :: real_arr(:)
- end type real_t
- !$omp declare mapper(custommapper: real_t :: t) map(t%base_arr, t%real_arr)
-
- type(real_t) :: r
- integer :: i
- allocate(r%base_arr(10), source=1.0)
- allocate(r%real_arr(10), source=1.0)
-
- !$omp target map(tofrom: r)
- do i = 1, size(r%base_arr)
- r%base_arr(i) = 2.0
- r%real_arr(i) = 3.0
- r%real_arr(i) = r%base_arr(1)
- end do
- !$omp end target
-
-
- !CHECK: base_arr: 2. 2. 2. 2. 2. 2. 2. 2. 2. 2.
- print*, "base_arr: ", r%base_arr
- !CHECK: real_arr: 2. 2. 2. 2. 2. 2. 2. 2. 2. 2.
- print*, "real_arr: ", r%real_arr
-
- deallocate(r%real_arr)
- deallocate(r%base_arr)
-end program target_declare_mapper_parent_allocatable
More information about the llvm-commits
mailing list