Skip to content

Commit 14c3bf7

Browse files
committed
Fix for SWDEV-533000, remove descriptor map type from has_device_addr clauses
Currently we apply OMP_MAP_DESCRIPTOR to has_device_addr, which we should avoid as it doesn't require the same treatment as regular data maps from map clauses and isn't actually mapped like the other descriptors either (it skips mapping the base address data). Ideally we'll cut out the runtime modifications from these descriptor flags that affect the runtime correctness behaviour in the future, and leave them there for downstream optimization toggles. I'll be looking into this in the near future.
1 parent 3cd08b5 commit 14c3bf7

File tree

2 files changed

+19
-9
lines changed

2 files changed

+19
-9
lines changed

flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,17 +262,26 @@ class MapInfoFinalizationPass
262262
/// allowing `to` mappings, and `target update` not allowing both `to` and
263263
/// `from` simultaneously. We currently try to maintain the `implicit` flag
264264
/// where necessary, although it does not seem strictly required.
265+
///
266+
/// Currently, if it is a has_device_addr clause, we opt to not apply the
267+
/// descriptor tag to it as it's used differently to a regular mapping
268+
/// and some of the runtime descriptor behaviour at the moment can cause
269+
/// issues.
265270
unsigned long getDescriptorMapType(unsigned long mapTypeFlag,
266-
mlir::Operation *target) {
271+
mlir::Operation *target,
272+
bool IsHasDeviceAddr) {
267273
using mapFlags = llvm::omp::OpenMPOffloadMappingFlags;
274+
268275
if (llvm::isa_and_nonnull<mlir::omp::TargetExitDataOp,
269-
mlir::omp::TargetUpdateOp>(target))
270-
return llvm::to_underlying(
271-
mapFlags(mapTypeFlag) |
272-
llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DESCRIPTOR);
276+
mlir::omp::TargetUpdateOp>(target)) {
277+
mapFlags flags = mapFlags(mapTypeFlag);
278+
if (!IsHasDeviceAddr)
279+
flags |= mapFlags::OMP_MAP_DESCRIPTOR;
280+
return llvm::to_underlying(flags);
281+
}
273282

274283
mapFlags flags = mapFlags::OMP_MAP_TO |
275-
(mapFlags(mapTypeFlag) & mapFlags::OMP_MAP_IMPLICIT);
284+
(mapFlags(mapTypeFlag) & mapFlags::OMP_MAP_IMPLICIT);
276285
// Descriptors for objects will always be copied. This is because the
277286
// descriptor can be rematerialized by the compiler, and so the addres
278287
// of the descriptor for a given object at one place in the code may
@@ -286,7 +295,8 @@ class MapInfoFinalizationPass
286295
mapFlags::OMP_MAP_CLOSE)
287296
? mapFlags::OMP_MAP_CLOSE
288297
: mapFlags::OMP_MAP_ALWAYS;
289-
flags |= mapFlags::OMP_MAP_DESCRIPTOR;
298+
if (!IsHasDeviceAddr)
299+
flags |= mapFlags::OMP_MAP_DESCRIPTOR;
290300
return llvm::to_underlying(flags);
291301
}
292302

@@ -381,7 +391,7 @@ class MapInfoFinalizationPass
381391
mlir::TypeAttr::get(fir::unwrapRefType(descriptor.getType())),
382392
builder.getIntegerAttr(
383393
builder.getIntegerType(64, false),
384-
getDescriptorMapType(op.getMapType(), target)),
394+
getDescriptorMapType(op.getMapType(), target, IsHasDeviceAddr)),
385395
op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, newMembers,
386396
newMembersAttr, /*bounds=*/mlir::SmallVector<mlir::Value>{},
387397
/*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(),

flang/test/Lower/OpenMP/has_device_addr-mapinfo.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ integer function s(a)
1717

1818
! Check that the map.info for `a` only takes a single parameter.
1919

20-
!CHECK-DAG: %[[MAP_A:[0-9]+]] = "omp.map.info"(%[[STORAGE_A:[0-9#]+]]) <{map_capture_type = #omp<variable_capture_kind(ByRef)>, map_type = 16901 : ui64, name = "a", operandSegmentSizes = array<i32: 1, 0, 0, 0>, partial_map = false, var_type = !fir.box<!fir.array<?xi32>>}> : (!fir.ref<!fir.box<!fir.array<?xi32>>>) -> !fir.ref<!fir.array<?xi32>>
20+
!CHECK-DAG: %[[MAP_A:[0-9]+]] = "omp.map.info"(%[[STORAGE_A:[0-9#]+]]) <{map_capture_type = #omp<variable_capture_kind(ByRef)>, map_type = 517 : ui64, name = "a", operandSegmentSizes = array<i32: 1, 0, 0, 0>, partial_map = false, var_type = !fir.box<!fir.array<?xi32>>}> : (!fir.ref<!fir.box<!fir.array<?xi32>>>) -> !fir.ref<!fir.array<?xi32>>
2121
!CHECK-DAG: %[[MAP_T:[0-9]+]] = "omp.map.info"(%[[STORAGE_T:[0-9#]+]]) <{map_capture_type = #omp<variable_capture_kind(ByRef)>, map_type = 2 : ui64, name = "t", operandSegmentSizes = array<i32: 1, 0, 0, 0>, partial_map = false, var_type = i32}> : (!fir.ref<i32>) -> !fir.ref<i32>
2222

2323
!CHECK: "omp.target"(%[[MAP_A]], %[[MAP_T]])

0 commit comments

Comments
 (0)