diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 9cdd46137adbf..b1af11110b423 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -1311,8 +1311,6 @@ bool ClauseProcessor::processMap( mlir::Location clauseLocation = converter.genLocation(source); const auto &[mapType, typeMods, attachMod, refMod, mappers, iterator, objects] = clause.t; - if (attachMod) - TODO(currentLocation, "ATTACH modifier is not implemented yet"); mlir::omp::ClauseMapFlags mapTypeBits = mlir::omp::ClauseMapFlags::none; std::string mapperIdName = "__implicit_mapper"; // If the map type is specified, then process it else set the appropriate @@ -1357,6 +1355,34 @@ bool ClauseProcessor::processMap( mapTypeBits |= mlir::omp::ClauseMapFlags::ompx_hold; } + if (refMod) { + switch (*refMod) { + case Map::RefModifier::RefPtee: + mapTypeBits |= mlir::omp::ClauseMapFlags::ref_ptee; + break; + case Map::RefModifier::RefPtr: + mapTypeBits |= mlir::omp::ClauseMapFlags::ref_ptr; + break; + case Map::RefModifier::RefPtrPtee: + mapTypeBits |= mlir::omp::ClauseMapFlags::ref_ptr_ptee; + break; + } + } + + if (attachMod) { + switch (*attachMod) { + case Map::AttachModifier::Always: + mapTypeBits |= mlir::omp::ClauseMapFlags::attach_always; + break; + case Map::AttachModifier::Never: + mapTypeBits |= mlir::omp::ClauseMapFlags::attach_never; + break; + case Map::AttachModifier::Auto: + mapTypeBits |= mlir::omp::ClauseMapFlags::attach_auto; + break; + } + } + if (iterator) { TODO(currentLocation, "Support for iterator modifiers is not implemented yet"); diff --git a/flang/lib/Optimizer/OpenMP/AutomapToTargetData.cpp b/flang/lib/Optimizer/OpenMP/AutomapToTargetData.cpp index 5793d46a192a7..eeb08ebf51191 100644 --- a/flang/lib/Optimizer/OpenMP/AutomapToTargetData.cpp +++ b/flang/lib/Optimizer/OpenMP/AutomapToTargetData.cpp @@ -124,6 +124,7 @@ class AutomapToTargetDataPass builder.getAttr( omp::VariableCaptureKind::ByCopy), /*var_ptr_ptr=*/mlir::Value{}, + /*var_ptr_ptr_type=*/mlir::TypeAttr{}, /*members=*/SmallVector{}, /*members_index=*/ArrayAttr{}, bounds, /*mapperId=*/mlir::FlatSymbolRefAttr(), globalOp.getSymNameAttr(), diff --git a/flang/lib/Optimizer/OpenMP/LowerWorkdistribute.cpp b/flang/lib/Optimizer/OpenMP/LowerWorkdistribute.cpp index 7b61539984232..d877d850f6d82 100644 --- a/flang/lib/Optimizer/OpenMP/LowerWorkdistribute.cpp +++ b/flang/lib/Optimizer/OpenMP/LowerWorkdistribute.cpp @@ -839,6 +839,7 @@ static TempOmpVar allocateTempOmpVar(Location loc, Type ty, rewriter.getAttr( omp::VariableCaptureKind::ByRef), /*varPtrPtr=*/Value{}, + /*varPtrPtrType=*/mlir::TypeAttr{}, /*members=*/SmallVector{}, /*member_index=*/mlir::ArrayAttr{}, /*bounds=*/ValueRange(), diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp index ee4b56d5c7d5f..64695db38c136 100644 --- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp +++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp @@ -200,7 +200,7 @@ class MapInfoFinalizationPass op.getMapTypeAttr(), builder.getAttr( mlir::omp::VariableCaptureKind::ByRef), - /*varPtrPtr=*/mlir::Value{}, /*members=*/mlir::ValueRange{}, + /*varPtrPtr=*/mlir::Value{}, /*varPtrPtr=*/mlir::TypeAttr{}, /*members=*/mlir::ValueRange{}, /*members_index=*/mlir::ArrayAttr{}, bounds, /*mapperId=*/mlir::FlatSymbolRefAttr(), builder.getStringAttr(op.getNameAttr().strref() + "." + memberName + @@ -308,7 +308,7 @@ class MapInfoFinalizationPass fir::FirOpBuilder &builder, bool &canDescBeDeferred) { mlir::Value descriptor = boxMap.getVarPtr(); - if (!fir::isTypeWithDescriptor(boxMap.getVarType())) + if (!fir::isTypeWithDescriptor(boxMap.getVarPtrType())) if (auto addrOp = mlir::dyn_cast_if_present( boxMap.getVarPtr().getDefiningOp())) descriptor = addrOp.getVal(); @@ -377,22 +377,26 @@ class MapInfoFinalizationPass mlir::Value baseAddrAddr = fir::BoxOffsetOp::create( builder, loc, descriptor, fir::BoxFieldAttr::base_addr); - mlir::Type underlyingVarType = + mlir::Type underlyingBaseAddrType = llvm::cast( fir::unwrapRefType(baseAddrAddr.getType())) .getElementType(); - if (auto seqType = llvm::dyn_cast(underlyingVarType)) + if (auto seqType = + llvm::dyn_cast(underlyingBaseAddrType)) if (seqType.hasDynamicExtents()) - underlyingVarType = seqType.getEleTy(); + underlyingBaseAddrType = seqType.getEleTy(); + + mlir::Type underlyingDescType = fir::unwrapRefType(descriptor.getType()); // Member of the descriptor pointing at the allocated data return mlir::omp::MapInfoOp::create( builder, loc, baseAddrAddr.getType(), descriptor, - mlir::TypeAttr::get(underlyingVarType), + mlir::TypeAttr::get(underlyingDescType), builder.getAttr(mapType), builder.getAttr( mlir::omp::VariableCaptureKind::ByRef), - baseAddrAddr, /*members=*/mlir::SmallVector{}, + baseAddrAddr, mlir::TypeAttr::get(underlyingBaseAddrType), + /*members=*/mlir::SmallVector{}, /*membersIndex=*/mlir::ArrayAttr{}, bounds, /*mapperId*/ mlir::FlatSymbolRefAttr(), /*name=*/builder.getStringAttr(""), @@ -443,6 +447,228 @@ class MapInfoFinalizationPass baseAddrIndex); } + // This functions aims to insert new maps derived from existing maps into the + // corresponding clause list, interlinking it correctly with block arguments + // where required . + void addDerivedMemberToTarget(mlir::omp::MapInfoOp owner, + mlir::omp::MapInfoOp derived, + llvm::SmallVectorImpl &mapMemberUsers, + fir::FirOpBuilder &builder, + mlir::Operation *target) { + auto addOperands = [&](mlir::MutableOperandRange &mapVarsArr, + mlir::Operation *directiveOp, + unsigned blockArgInsertIndex = 0) { + // Check we're inserting into the correct MapInfoOp list + if (!llvm::is_contained(mapVarsArr.getAsOperandRange(), + mapMemberUsers.empty() + ? owner.getResult() + : mapMemberUsers[0].parent.getResult())) + return; + + // Check we're not inserting a duplicate map. + if (llvm::is_contained(mapVarsArr.getAsOperandRange(), + derived.getResult())) + return; + + // There doesn't appear to be a simple way to convert MutableOperandRange + // to a vector currently, so we instead use a for_each to populate our + // vector. + llvm::SmallVector newMapOps; + newMapOps.reserve(mapVarsArr.size()); + llvm::for_each( + mapVarsArr.getAsOperandRange(), + [&newMapOps](mlir::Value oper) { newMapOps.push_back(oper); }); + + newMapOps.push_back(derived); + if (directiveOp) { + directiveOp->getRegion(0).insertArgument( + blockArgInsertIndex, derived.getType(), derived.getLoc()); + blockArgInsertIndex++; + } + + mapVarsArr.assign(newMapOps); + }; + + auto argIface = + llvm::dyn_cast(target); + + if (auto mapClauseOwner = + llvm::dyn_cast(target)) { + mlir::MutableOperandRange mapVarsArr = mapClauseOwner.getMapVarsMutable(); + unsigned blockArgInsertIndex = + argIface + ? argIface.getMapBlockArgsStart() + argIface.numMapBlockArgs() + : 0; + addOperands(mapVarsArr, + llvm::dyn_cast_if_present( + argIface.getOperation()), + blockArgInsertIndex); + } + + if (auto targetDataOp = llvm::dyn_cast(target)) { + mlir::MutableOperandRange useDevAddrMutableOpRange = + targetDataOp.getUseDeviceAddrVarsMutable(); + addOperands(useDevAddrMutableOpRange, target, + argIface.getUseDeviceAddrBlockArgsStart() + + argIface.numUseDeviceAddrBlockArgs()); + + mlir::MutableOperandRange useDevPtrMutableOpRange = + targetDataOp.getUseDevicePtrVarsMutable(); + addOperands(useDevPtrMutableOpRange, target, + argIface.getUseDevicePtrBlockArgsStart() + + argIface.numUseDevicePtrBlockArgs()); + } else if (auto targetOp = llvm::dyn_cast(target)) { + mlir::MutableOperandRange hasDevAddrMutableOpRange = + targetOp.getHasDeviceAddrVarsMutable(); + addOperands(hasDevAddrMutableOpRange, target, + argIface.getHasDeviceAddrBlockArgsStart() + + argIface.numHasDeviceAddrBlockArgs()); + } + } + + // We add all mapped record members not directly used in the target region + // to the block arguments in front of their parent and we place them into + // the map operands list for consistency. + // + // These indirect uses (via accesses to their parent) will still be + // mapped individually in most cases, and a parent mapping doesn't + // guarantee the parent will be mapped in its totality, partial + // mapping is common. + // + // For example: + // map(tofrom: x%y) + // + // Will generate a mapping for "x" (the parent) and "y" (the member). + // The parent "x" will not be mapped, but the member "y" will. + // However, we must have the parent as a BlockArg and MapOperand + // in these cases, to maintain the correct uses within the region and + // to help tracking that the member is part of a larger object. + // + // In the case of: + // map(tofrom: x%y, x%z) + // + // The parent member becomes more critical, as we perform a partial + // structure mapping where we link the mapping of the members y + // and z together via the parent x. We do this at a kernel argument + // level in LLVM IR and not just MLIR, which is important to maintain + // similarity to Clang and for the runtime to do the correct thing. + // However, we still do not map the structure in its totality but + // rather we generate an un-sized "binding" map entry for it. + // + // In the case of: + // map(tofrom: x, x%y, x%z) + // + // We do actually map the entirety of "x", so the explicit mapping of + // x%y, x%z becomes unnecessary. It is redundant to write this from a + // Fortran OpenMP perspective (although it is legal), as even if the + // members were allocatables or pointers, we are mandated by the + // specification to map these (and any recursive components) in their + // entirety, which is different to the C++ equivalent, which requires + // explicit mapping of these segments. +void addImplicitMembersToTarget(mlir::omp::MapInfoOp op, + fir::FirOpBuilder &builder, + mlir::Operation *target) { + auto mapClauseOwner = + llvm::dyn_cast_if_present( + target); + // TargetDataOp is technically a MapClauseOwningOpInterface, so we + // do not need to explicitly check for the extra cases here for use_device + // addr/ptr + if (!mapClauseOwner) + return; + + auto addOperands = [&](mlir::MutableOperandRange &mapVarsArr, + mlir::Operation *directiveOp, + unsigned blockArgInsertIndex = 0) { + if (!llvm::is_contained(mapVarsArr.getAsOperandRange(), op.getResult())) + return; + + // There doesn't appear to be a simple way to convert MutableOperandRange + // to a vector currently, so we instead use a for_each to populate our + // vector. + llvm::SmallVector newMapOps; + newMapOps.reserve(mapVarsArr.size()); + llvm::for_each( + mapVarsArr.getAsOperandRange(), + [&newMapOps](mlir::Value oper) { newMapOps.push_back(oper); }); + + for (auto mapMember : op.getMembers()) { + if (llvm::is_contained(mapVarsArr.getAsOperandRange(), mapMember)) + continue; + newMapOps.push_back(mapMember); + if (directiveOp) { + directiveOp->getRegion(0).insertArgument( + blockArgInsertIndex, mapMember.getType(), mapMember.getLoc()); + blockArgInsertIndex++; + } + } + + mapVarsArr.assign(newMapOps); + }; + + auto argIface = + llvm::dyn_cast(target); + + if (auto mapClauseOwner = + llvm::dyn_cast(target)) { + mlir::MutableOperandRange mapVarsArr = mapClauseOwner.getMapVarsMutable(); + unsigned blockArgInsertIndex = + argIface + ? argIface.getMapBlockArgsStart() + argIface.numMapBlockArgs() + : 0; + addOperands(mapVarsArr, + llvm::dyn_cast_if_present( + argIface.getOperation()), + blockArgInsertIndex); + } + + if (auto targetDataOp = llvm::dyn_cast(target)) { + mlir::MutableOperandRange useDevAddrMutableOpRange = + targetDataOp.getUseDeviceAddrVarsMutable(); + addOperands(useDevAddrMutableOpRange, target, + argIface.getUseDeviceAddrBlockArgsStart() + + argIface.numUseDeviceAddrBlockArgs()); + + mlir::MutableOperandRange useDevPtrMutableOpRange = + targetDataOp.getUseDevicePtrVarsMutable(); + addOperands(useDevPtrMutableOpRange, target, + argIface.getUseDevicePtrBlockArgsStart() + + argIface.numUseDevicePtrBlockArgs()); + } else if (auto targetOp = llvm::dyn_cast(target)) { + mlir::MutableOperandRange hasDevAddrMutableOpRange = + targetOp.getHasDeviceAddrVarsMutable(); + addOperands(hasDevAddrMutableOpRange, target, + argIface.getHasDeviceAddrBlockArgsStart() + + argIface.numHasDeviceAddrBlockArgs()); + } + } + + // We retrieve the first user that is a Target operation, of which + // there should only be one currently. Every MapInfoOp can be tied to + // at most one Target operation and at the minimum no operations. + // This may change in the future with IR cleanups/modifications, + // in which case this pass will need updating to support cases + // where a map can have more than one user and more than one of + // those users can be a Target operation. For now, we simply + // return the first target operation encountered, which may + // be on the parent MapInfoOp in the case of a member mapping. + // In that case, we traverse the MapInfoOp chain until we + // find the first TargetOp user. + mlir::Operation *getFirstTargetUser(mlir::omp::MapInfoOp mapOp) { + for (auto *user : mapOp->getUsers()) { + if (llvm::isa(user)) + return user; + + if (auto mapUser = llvm::dyn_cast(user)) + return getFirstTargetUser(mapUser); + } + + return nullptr; + } + /// Adjusts the descriptor's map type. The main alteration that is done /// currently is transforming the map type to `OMP_MAP_TO` where possible. /// This is because we will always need to map the descriptor to device @@ -468,10 +694,32 @@ class MapInfoFinalizationPass /// issues. mlir::omp::ClauseMapFlags getDescriptorMapType(mlir::omp::ClauseMapFlags mapTypeFlag, - mlir::Operation *target, bool isHasDeviceAddr) { + mlir::Operation *target) { using mapFlags = mlir::omp::ClauseMapFlags; mapFlags flags = mapFlags::none; - if (!isHasDeviceAddr) + + if (llvm::isa_and_nonnull(target)) { + flags |= mapTypeFlag | mapFlags::descriptor; + return flags; + } + + flags |= mapFlags::to | mapFlags::descriptor | mapFlags::always | + (mapTypeFlag & mapFlags::implicit); + + if (moduleRequiresUSM(target->getParentOfType())) + flags |= mapFlags::close; + return flags; + } + + mlir::omp::ClauseMapFlags + getDescriptorMapType(mlir::omp::ClauseMapFlags mapTypeFlag, + mlir::Operation *target, bool isHasDeviceAddr, + bool isAttachNever = false) { + using mapFlags = mlir::omp::ClauseMapFlags; + mapFlags flags = mapFlags::none; + + if (!isHasDeviceAddr && !isAttachNever) flags |= mapFlags::attach; if (llvm::isa_and_nonnull &mapMemberUsers, + mlir::Operation *target, fir::FirOpBuilder &builder, + mlir::omp::ClauseMapFlags refFlagType, bool isAttachAlways = false) { + // auto implicitAttachMap = mlir::omp::MapInfoOp::create( + // builder, descMapOp->getLoc(), descMapOp.getResult().getType(), + // descriptor, + // mlir::TypeAttr::get(fir::unwrapRefType(descriptor.getType())), + // builder.getAttr( + // mlir::omp::ClauseMapFlags::attach | refFlagType | + // (isAttachAlways ? mlir::omp::ClauseMapFlags::always + // : mlir::omp::ClauseMapFlags::none)), + // descMapOp.getMapCaptureTypeAttr(), /*varPtrPtr=*/ + // fir::BoxOffsetOp::create(builder, descMapOp->getLoc(), descriptor, + // fir::BoxFieldAttr::base_addr), + // /*members=*/mlir::SmallVector{}, + // /*membersIndex=*/mlir::ArrayAttr{}, + // /*bounds=*/mlir::SmallVector{}, + // /*mapperId*/ mlir::FlatSymbolRefAttr(), descMapOp.getNameAttr(), + // /*partial_map=*/builder.getBoolAttr(false)); + + // AG NOTE: If we keep this make it a helper function + auto baseAddrAddr = fir::BoxOffsetOp::create( + builder, descMapOp->getLoc(), descriptor, fir::BoxFieldAttr::base_addr); + mlir::Type underlyingVarType = + llvm::cast( + fir::unwrapRefType(baseAddrAddr.getType())) + .getElementType(); + if (auto seqType = llvm::dyn_cast(underlyingVarType)) + if (seqType.hasDynamicExtents()) + underlyingVarType = seqType.getEleTy(); + + auto implicitAttachMap = mlir::omp::MapInfoOp::create( + builder, descMapOp->getLoc(), descMapOp.getResult().getType(), + descriptor, mlir::TypeAttr::get(fir::unwrapRefType(descriptor.getType())), + builder.getAttr( + mlir::omp::ClauseMapFlags::attach | refFlagType | + (isAttachAlways ? mlir::omp::ClauseMapFlags::always + : mlir::omp::ClauseMapFlags::none)), + descMapOp.getMapCaptureTypeAttr(), /*varPtrPtr=*/ + baseAddrAddr, mlir::TypeAttr::get(underlyingVarType), + /*members=*/mlir::SmallVector{}, + /*membersIndex=*/mlir::ArrayAttr{}, + /*bounds=*/descMapOp.getBounds(), + /*mapperId*/ mlir::FlatSymbolRefAttr(), descMapOp.getNameAttr(), + /*partial_map=*/builder.getBoolAttr(false)); + + // Has to be added to the target immediately, as we expect all maps + // processed by this pass to have a user that is a target. + addDerivedMemberToTarget(descMapOp, implicitAttachMap, mapMemberUsers, + builder, target); + } + // Expand mappings of type(C_PTR) to map their `__address` field explicitly // as a single pointer-sized member (USM-gated at callsite). This helps in // USM scenarios to ensure the pointer-sized mapping is used. @@ -563,7 +865,7 @@ class MapInfoFinalizationPass mlir::TypeAttr::get(fir::unwrapRefType(coord.getType())), mapTypeAttr, builder.getAttr( mlir::omp::VariableCaptureKind::ByRef), - /*varPtrPtr=*/mlir::Value{}, + /*varPtrPtr=*/mlir::Value{}, mlir::TypeAttr{}, /*members=*/llvm::SmallVector{}, /*member_index=*/mlir::ArrayAttr{}, /*bounds=*/op.getBounds(), @@ -574,8 +876,8 @@ class MapInfoFinalizationPass // Rebuild the parent as a container with the `__address` member. mlir::omp::MapInfoOp newParent = mlir::omp::MapInfoOp::create( builder, op.getLoc(), op.getResult().getType(), op.getVarPtr(), - op.getVarTypeAttr(), mapTypeAttr, op.getMapCaptureTypeAttr(), - /*varPtrPtr=*/mlir::Value{}, + op.getVarPtrTypeAttr(), mapTypeAttr, op.getMapCaptureTypeAttr(), + /*varPtrPtr=*/mlir::Value{}, mlir::TypeAttr{}, /*members=*/llvm::SmallVector{memberMap}, /*member_index=*/newMembersAttr, /*bounds=*/llvm::SmallVector{}, @@ -586,28 +888,6 @@ class MapInfoFinalizationPass return newParent; } - mlir::omp::MapInfoOp genDescriptorMemberMaps(mlir::omp::MapInfoOp op, - fir::FirOpBuilder &builder, - mlir::Operation *target) { - llvm::SmallVector mapMemberUsers; - getMemberUserList(op, mapMemberUsers); - - // TODO: map the addendum segment of the descriptor, similarly to the - // base address/data pointer member. - bool descCanBeDeferred = false; - mlir::Value descriptor = - getDescriptorFromBoxMap(op, builder, descCanBeDeferred); - - mlir::ArrayAttr newMembersAttr; - mlir::SmallVector newMembers; - llvm::SmallVector> memberIndices; - bool isHasDeviceAddrFlag = isHasDeviceAddr(op, *target); - - if (!mapMemberUsers.empty() || !op.getMembers().empty()) - getMemberIndicesAsVectors( - !mapMemberUsers.empty() ? mapMemberUsers[0].parent : op, - memberIndices); - // If the operation that we are expanding with a descriptor has a user // (parent), then we have to expand the parent's member indices to reflect // the adjusted member indices for the base address insertion. However, if @@ -621,6 +901,14 @@ class MapInfoFinalizationPass // from the descriptor to be used verbatim, i.e. without additional // remapping. To avoid this remapping, simply don't generate any map // information for the descriptor members. + void createBaseAddrInsertion( + fir::FirOpBuilder &builder, mlir::omp::MapInfoOp parentOp, + mlir::omp::MapInfoOp baseAddr, + llvm::SmallVectorImpl &mapMemberUsers, + mlir::ArrayAttr &newMembersAttr, + mlir::SmallVectorImpl &newMembers, + llvm::SmallVector> &memberIndices, + bool isHasDeviceAddrFlag) { if (!mapMemberUsers.empty()) { // Currently, there should only be one user per map when this pass // is executed. Either a parent map, holding the current map in its @@ -631,194 +919,224 @@ class MapInfoFinalizationPass assert(mapMemberUsers.size() == 1 && "OMPMapInfoFinalization currently only supports single users of a " "MapInfoOp"); - auto baseAddr = - genBaseAddrMap(descriptor, op.getBounds(), op.getMapType(), builder); ParentAndPlacement mapUser = mapMemberUsers[0]; adjustMemberIndices(memberIndices, mapUser); llvm::SmallVector newMemberOps; for (auto v : mapUser.parent.getMembers()) { newMemberOps.push_back(v); - if (v == op) + if (v == parentOp) newMemberOps.push_back(baseAddr); } mapUser.parent.getMembersMutable().assign(newMemberOps); mapUser.parent.setMembersIndexAttr( builder.create2DI64ArrayAttr(memberIndices)); } else if (!isHasDeviceAddrFlag) { - auto baseAddr = - genBaseAddrMap(descriptor, op.getBounds(), op.getMapType(), builder); newMembers.push_back(baseAddr); - if (!op.getMembers().empty()) { + if (!parentOp.getMembers().empty()) { for (auto &indices : memberIndices) indices.insert(indices.begin(), 0); memberIndices.insert(memberIndices.begin(), {0}); newMembersAttr = builder.create2DI64ArrayAttr(memberIndices); - newMembers.append(op.getMembers().begin(), op.getMembers().end()); + newMembers.append(parentOp.getMembers().begin(), + parentOp.getMembers().end()); } else { llvm::SmallVector> memberIdx = {{0}}; newMembersAttr = builder.create2DI64ArrayAttr(memberIdx); } } - - mlir::omp::MapInfoOp newDescParentMapOp = mlir::omp::MapInfoOp::create( - builder, op->getLoc(), op.getResult().getType(), descriptor, - mlir::TypeAttr::get(fir::unwrapRefType(descriptor.getType())), - builder.getAttr( - getDescriptorMapType(op.getMapType(), target, isHasDeviceAddrFlag)), - op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, newMembers, - newMembersAttr, /*bounds=*/mlir::SmallVector{}, - /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(), - /*partial_map=*/builder.getBoolAttr(false)); - op.replaceAllUsesWith(newDescParentMapOp.getResult()); - op->erase(); - - if (descCanBeDeferred) - deferrableDesc.push_back(newDescParentMapOp); - - return newDescParentMapOp; } - // We add all mapped record members not directly used in the target region - // to the block arguments in front of their parent and we place them into - // the map operands list for consistency. - // - // These indirect uses (via accesses to their parent) will still be - // mapped individually in most cases, and a parent mapping doesn't - // guarantee the parent will be mapped in its totality, partial - // mapping is common. - // - // For example: - // map(tofrom: x%y) - // - // Will generate a mapping for "x" (the parent) and "y" (the member). - // The parent "x" will not be mapped, but the member "y" will. - // However, we must have the parent as a BlockArg and MapOperand - // in these cases, to maintain the correct uses within the region and - // to help tracking that the member is part of a larger object. - // - // In the case of: - // map(tofrom: x%y, x%z) - // - // The parent member becomes more critical, as we perform a partial - // structure mapping where we link the mapping of the members y - // and z together via the parent x. We do this at a kernel argument - // level in LLVM IR and not just MLIR, which is important to maintain - // similarity to Clang and for the runtime to do the correct thing. - // However, we still do not map the structure in its totality but - // rather we generate an un-sized "binding" map entry for it. - // - // In the case of: - // map(tofrom: x, x%y, x%z) - // - // We do actually map the entirety of "x", so the explicit mapping of - // x%y, x%z becomes unnecessary. It is redundant to write this from a - // Fortran OpenMP perspective (although it is legal), as even if the - // members were allocatables or pointers, we are mandated by the - // specification to map these (and any recursive components) in their - // entirety, which is different to the C++ equivalent, which requires - // explicit mapping of these segments. - void addImplicitMembersToTarget(mlir::omp::MapInfoOp op, - fir::FirOpBuilder &builder, - mlir::Operation *target) { - auto mapClauseOwner = - llvm::dyn_cast_if_present( - target); - // TargetDataOp is technically a MapClauseOwningOpInterface, so we - // do not need to explicitly check for the extra cases here for use_device - // addr/ptr - if (!mapClauseOwner) - return; - - auto addOperands = [&](mlir::MutableOperandRange &mapVarsArr, - mlir::Operation *directiveOp, - unsigned blockArgInsertIndex = 0) { - if (!llvm::is_contained(mapVarsArr.getAsOperandRange(), op.getResult())) - return; - - // There doesn't appear to be a simple way to convert MutableOperandRange - // to a vector currently, so we instead use a for_each to populate our - // vector. - llvm::SmallVector newMapOps; - newMapOps.reserve(mapVarsArr.size()); - llvm::for_each( - mapVarsArr.getAsOperandRange(), - [&newMapOps](mlir::Value oper) { newMapOps.push_back(oper); }); - - for (auto mapMember : op.getMembers()) { - if (llvm::is_contained(mapVarsArr.getAsOperandRange(), mapMember)) - continue; - newMapOps.push_back(mapMember); - if (directiveOp) { - directiveOp->getRegion(0).insertArgument( - blockArgInsertIndex, mapMember.getType(), mapMember.getLoc()); - blockArgInsertIndex++; - } - } - - mapVarsArr.assign(newMapOps); - }; - - auto argIface = - llvm::dyn_cast(target); - - if (auto mapClauseOwner = - llvm::dyn_cast(target)) { - mlir::MutableOperandRange mapVarsArr = mapClauseOwner.getMapVarsMutable(); - unsigned blockArgInsertIndex = - argIface - ? argIface.getMapBlockArgsStart() + argIface.numMapBlockArgs() - : 0; - addOperands(mapVarsArr, - llvm::dyn_cast_if_present( - argIface.getOperation()), - blockArgInsertIndex); - } - - if (auto targetDataOp = llvm::dyn_cast(target)) { - mlir::MutableOperandRange useDevAddrMutableOpRange = - targetDataOp.getUseDeviceAddrVarsMutable(); - addOperands(useDevAddrMutableOpRange, target, - argIface.getUseDeviceAddrBlockArgsStart() + - argIface.numUseDeviceAddrBlockArgs()); - - mlir::MutableOperandRange useDevPtrMutableOpRange = - targetDataOp.getUseDevicePtrVarsMutable(); - addOperands(useDevPtrMutableOpRange, target, - argIface.getUseDevicePtrBlockArgsStart() + - argIface.numUseDevicePtrBlockArgs()); - } else if (auto targetOp = llvm::dyn_cast(target)) { - mlir::MutableOperandRange hasDevAddrMutableOpRange = - targetOp.getHasDeviceAddrVarsMutable(); - addOperands(hasDevAddrMutableOpRange, target, - argIface.getHasDeviceAddrBlockArgsStart() + - argIface.numHasDeviceAddrBlockArgs()); - } - } + void genDescriptorMaps(mlir::omp::MapInfoOp op, fir::FirOpBuilder &builder, + mlir::Operation *target) { + bool descCanBeDeferred = false; + llvm::SmallVector mapMemberUsers; + getMemberUserList(op, mapMemberUsers); - // We retrieve the first user that is a Target operation, of which - // there should only be one currently. Every MapInfoOp can be tied to - // at most one Target operation and at the minimum no operations. - // This may change in the future with IR cleanups/modifications, - // in which case this pass will need updating to support cases - // where a map can have more than one user and more than one of - // those users can be a Target operation. For now, we simply - // return the first target operation encountered, which may - // be on the parent MapInfoOp in the case of a member mapping. - // In that case, we traverse the MapInfoOp chain until we - // find the first TargetOp user. - mlir::Operation *getFirstTargetUser(mlir::omp::MapInfoOp mapOp) { - for (auto *user : mapOp->getUsers()) { - if (llvm::isa(user)) - return user; + // TODO: map the addendum segment of the descriptor, similarly to the + // base address/data pointer member. + bool isHasDeviceAddrFlag = isHasDeviceAddr(op, *target); + bool isAttachNever = + (op.getMapType() & mlir::omp::ClauseMapFlags::attach_never) == + mlir::omp::ClauseMapFlags::attach_never; + bool isAttachAlways = + (op.getMapType() & mlir::omp::ClauseMapFlags::attach_always) == + mlir::omp::ClauseMapFlags::attach_always; - if (auto mapUser = llvm::dyn_cast(user)) - return getFirstTargetUser(mapUser); + mlir::Value descriptor = + getDescriptorFromBoxMap(op, builder, descCanBeDeferred); + if ((op.getMapType() & mlir::omp::ClauseMapFlags::ref_ptr) == + mlir::omp::ClauseMapFlags::ref_ptr) { + // For ref_ptr, we generate a map of the descriptor with user specified map types and + // in the default auto attach case, we generate an additional attach map which indicates + // to the runtime to try and attach the base address to the descriptor if it's available + // and it's the first time the ref_ptr has been allocated on the device. + auto newMapInfoOp = mlir::omp::MapInfoOp::create( + builder, op->getLoc(), op.getResult().getType(), op.getVarPtr(), + op.getVarPtrTypeAttr(), + builder.getAttr( + op.getMapType() & ~mlir::omp::ClauseMapFlags::ref_ptr), + op.getMapCaptureTypeAttr(), /*varPtrPtr=*/op.getVarPtrPtr(), + /*varPtrPtrType=*/op.getVarPtrPtrTypeAttr(), op.getMembers(), + op.getMembersIndexAttr(), + /*bounds=*/mlir::SmallVector{}, + /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(), + /*partial_map=*/builder.getBoolAttr(false)); + + // If we're a map exiting construct we skip the generation of the + // attach map, it should be unnecessary in these cases as it exists to + // bind the pointer and pointee and shouldn't increment or decrement + // the ref counter on its own. However, equally having it doesn't + // cause issues either, it's just ideal to remove the noise where + // feasible. + // TODO: Extend this to perhaps check for target updates and target data + // with release and from applied. + if (!llvm::isa(target) && !isAttachNever) + genImplicitAttachMap(op, descriptor, mapMemberUsers, target, builder, + mlir::omp::ClauseMapFlags::ref_ptr/*_ptee*/, + isAttachAlways); + op.replaceAllUsesWith(newMapInfoOp.getResult()); + op->erase(); + } else if ((op.getMapType() & mlir::omp::ClauseMapFlags::ref_ptee) == + mlir::omp::ClauseMapFlags::ref_ptee) { + + // TODO/FIX: + // 0) ii) test ref_ptr_ptee + // iii) test some other ref_ptr/ptee examples (for example the stack + // descriptor getting stuck in memory example), and perhaps ask michael + // if he has any ideas iiiii) Make sure it works for all existing map + // tests and check-offload, check-smoke without breakages iiiiii) Add + // these tests to check-smoke with a script that will verify the + // runtime trace and check attaches are happening at the right time as + // well as transfers of the various bits and pieces... + // 1) Look into the possibility of teaching the backend to only require + // ATTACH map to do the more + // unique mapping of accessing varPtr/varPtrPtr fields for + // basePointer/offloadPointer as opposed to using the ref_ptee/ref_ptr + // types... + // 2) Remove the always map on the attach in lowering and try to find/fix + // the problem case so it's no + // longer required... + // 3) Implement attach always/auto/none + // - we'll want to apply ALWAYS when map always is given, and never + // apply when (perhaps in subsequent patch so as not to overload + // everything at once) attach none is provided, but the default case we + // will just generate this map. + // 4) Test and fix if neccessary the derived type case for ref_ptr/ptee + // where we apply it to multiple nested + // allocatables, the usual stuff needed to check and fix for descriptor + // derived type related stuff + // 5) look at tidying everything up more. + // 6) - Can we simplify the attach map AND the way we bind the + // descriptors by using ref_ptr_ptee to indicate + // we're a binding map, or would this cause issues? Or perhaps there's a + // better way to simplify the backend mappings by splitting them into 3 + // seperate maps in the else clause below, but it might cause issues for + // derived types need to test first.... + // 7) - If 6 works then re-viist if we need attach maps on exit directives + // for the main combined map, it may have been tied to the fact the + // lowering + // found itn eccessary + // 8) Can perhaps revisit if we can detach the need for adding + // base_addresses to the members_of, but might be better to leave that + // to the future for a while as it's more of a tidying up scenario + // than a neccessity. + // 9) ask Michael what he thinks about turning off the descriptor deferral + // stuff for newer OpenMP versions by default + // and having to provide a flag in those cases. Just so we can nudge + // people towards using ref_ptr/ptee in those cases (and so we can test + // the ref_ptr/ptee test trivially) - Might not really matter for + // downstream as we have a flag to toggle it, but I don't think upstream + // has it + + // For ref_ptee, we generate a map of the base address with user specified map types and + // in the default auto attach case, we generate an additional attach map which indicates + // to the runtime to try and attach the base address to the descriptor if it's available + // and it's the first time the ref_ptee has been allocated on the device. + auto newMapInfoOp = genBaseAddrMap( + descriptor, op.getBounds(), + op.getMapType() & ~mlir::omp::ClauseMapFlags::ref_ptee, builder); + + // If we're a map exiting construct we skip the generation of the attach + // map, it should be unnecessary in these cases as it exists to bind the + // pointer and pointee and shouldn't increment or decrement the ref counter + // on its own. However, equally having it doesn't cause issues either, it's + // just ideal to remove the noise where feasible. + // TODO: Extend this to perhaps check for target updates and target data + // with release and from applied. + if (!llvm::isa(target) && !isAttachNever) + genImplicitAttachMap(op, descriptor, mapMemberUsers, target, builder, + mlir::omp::ClauseMapFlags::ref_ptr_ptee, + isAttachAlways); + op.replaceAllUsesWith(newMapInfoOp.getResult()); + op->erase(); + } else { + + // TODO: Split this into multiple mappings like above, instead of a parent + // member mapping to try and simplify this and later lowering. + bool isRefPtrPtee = + (op.getMapType() & mlir::omp::ClauseMapFlags::ref_ptr_ptee) == + mlir::omp::ClauseMapFlags::ref_ptr_ptee; + + mlir::ArrayAttr newMembersAttr; + mlir::SmallVector newMembers; + llvm::SmallVector> memberIndices; + + if (!mapMemberUsers.empty() || !op.getMembers().empty()) + getMemberIndicesAsVectors( + !mapMemberUsers.empty() ? mapMemberUsers[0].parent : op, + memberIndices); + + mlir::omp::MapInfoOp baseAddr = + genBaseAddrMap(descriptor, op.getBounds(), op.getMapType(), builder); + createBaseAddrInsertion(builder, op, baseAddr, mapMemberUsers, + newMembersAttr, newMembers, memberIndices, + isHasDeviceAddrFlag); + + // If we have been provided RefPtrPtee, utilise the user specified map + // types as best we can only providing the additional map types necessary, + // otherwise, use the default descriptor map type. + + // NOTE: For the moment isAttachAlways, for this case is handled by not + // removing attach_always, and letting the later lowering handle it, the + // regular always map type isn't equivalent to attach_always at this + // level. At least yet, I have plans to refactor this all in a subsequent + // commit. + auto mapType = + isRefPtrPtee + ? (op.getMapType() | mlir::omp::ClauseMapFlags::descriptor) + : getDescriptorMapType(op.getMapType(), target); + + // auto mapType = + // isRefPtrPtee + // ? (op.getMapType() | mlir::omp::ClauseMapFlags::descriptor | + // (isAttachNever ? mlir::omp::ClauseMapFlags::none + // : mlir::omp::ClauseMapFlags::attach)) + // : getDescriptorMapType(op.getMapType(), target, + // isHasDeviceAddrFlag, isAttachNever); + auto newMapInfoOp = mlir::omp::MapInfoOp::create( + builder, op->getLoc(), op.getResult().getType(), descriptor, + mlir::TypeAttr::get(fir::unwrapRefType(descriptor.getType())), + builder.getAttr(mapType), + op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, + /*varPtrPtTyper=*/mlir::TypeAttr{}, newMembers, newMembersAttr, + /*bounds=*/mlir::SmallVector{}, + /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(), + /*partial_map=*/builder.getBoolAttr(false)); + + // do we make this the unique case of not having this implicit attach map + // for now...? + // - as there's a lot of broken tests... 171 vs 150... + if (!llvm::isa(target) && !isAttachNever) + genImplicitAttachMap(op, descriptor, mapMemberUsers, target, builder, + mlir::omp::ClauseMapFlags::ref_ptr_ptee, + isAttachAlways); + + op.replaceAllUsesWith(newMapInfoOp.getResult()); + op->erase(); + + if (descCanBeDeferred) + deferrableDesc.push_back(newMapInfoOp); } - - return nullptr; } void addImplicitDescriptorMapToTargetDataOp(mlir::omp::MapInfoOp op, @@ -863,12 +1181,13 @@ class MapInfoFinalizationPass mlir::omp::MapInfoOp newDescParentMapOp = mlir::omp::MapInfoOp::create( builder, op->getLoc(), op.getResult().getType(), op.getVarPtr(), - op.getVarTypeAttr(), + op.getVarPtrTypeAttr(), builder.getAttr( mlir::omp::ClauseMapFlags::to | mlir::omp::ClauseMapFlags::always | mlir::omp::ClauseMapFlags::descriptor), op.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, - mlir::SmallVector{}, mlir::ArrayAttr{}, + /*varPtrPtrType=*/mlir::TypeAttr{}, mlir::SmallVector{}, + mlir::ArrayAttr{}, /*bounds=*/mlir::SmallVector{}, /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(), /*partial_map=*/builder.getBoolAttr(false)); @@ -925,8 +1244,9 @@ class MapInfoFinalizationPass builder.loadIfRef(op->getLoc(), baseAddr.getVarPtrPtr()); mlir::omp::MapInfoOp newBaseAddrMapOp = mlir::omp::MapInfoOp::create( builder, op->getLoc(), loadBaseAddr.getType(), loadBaseAddr, - baseAddr.getVarTypeAttr(), baseAddr.getMapTypeAttr(), - baseAddr.getMapCaptureTypeAttr(), mlir::Value{}, members, membersAttr, + baseAddr.getVarPtrTypeAttr(), baseAddr.getMapTypeAttr(), + baseAddr.getMapCaptureTypeAttr(), /*varPtrPtr=*/mlir::Value{}, + /*varPtrPtrType=*/mlir::TypeAttr{}, members, membersAttr, baseAddr.getBounds(), /*mapperId*/ mlir::FlatSymbolRefAttr(), op.getNameAttr(), /*partial_map=*/builder.getBoolAttr(false)); @@ -1193,11 +1513,11 @@ class MapInfoFinalizationPass "of a MapInfoOp"); if (hasADescriptor(op.getVarPtr().getDefiningOp(), - fir::unwrapRefType(op.getVarType()))) { + fir::unwrapRefType(op.getVarPtrType()))) { builder.setInsertionPoint(op); mlir::Operation *targetUser = getFirstTargetUser(op); assert(targetUser && "expected user of map operation was not found"); - genDescriptorMemberMaps(op, builder, targetUser); + genDescriptorMaps(op, builder, targetUser); } }); diff --git a/flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp b/flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp index 1d10c5b8dec41..70f36b12d4330 100644 --- a/flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp +++ b/flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp @@ -132,7 +132,7 @@ class MapsForPrivatizedSymbolsPass llvm::cast(varType).getElementType()), builder.getAttr(mapFlag), builder.getAttr(captureKind), - /*varPtrPtr=*/Value{}, + /*varPtrPtr=*/Value{}, /*varPtrPtrType=*/TypeAttr{}, /*members=*/SmallVector{}, /*member_index=*/mlir::ArrayAttr{}, /*bounds=*/boundsOps, diff --git a/flang/lib/Utils/OpenMP.cpp b/flang/lib/Utils/OpenMP.cpp index b07caf853191a..50fc7b5ab3cf8 100644 --- a/flang/lib/Utils/OpenMP.cpp +++ b/flang/lib/Utils/OpenMP.cpp @@ -26,26 +26,31 @@ mlir::omp::MapInfoOp createMapInfoOp(mlir::OpBuilder &builder, mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type retTy, bool partialMap, mlir::FlatSymbolRefAttr mapperId) { + auto getPtrVarType = [](mlir::Type ptrType) { + mlir::TypeAttr varType = mlir::TypeAttr::get( + llvm::cast(ptrType).getElementType()); + + // For types with unknown extents such as <2x?xi32> we discard the + // incomplete type info and only retain the base type. The correct + // dimensions are later recovered through the bounds info. + if (auto seqType = llvm::dyn_cast(varType.getValue())) + if (seqType.hasDynamicExtents()) + varType = mlir::TypeAttr::get(seqType.getEleTy()); + return varType; + }; + if (auto boxTy = llvm::dyn_cast(baseAddr.getType())) { baseAddr = fir::BoxAddrOp::create(builder, loc, baseAddr); retTy = baseAddr.getType(); } - mlir::TypeAttr varType = mlir::TypeAttr::get( - llvm::cast(retTy).getElementType()); - - // For types with unknown extents such as <2x?xi32> we discard the incomplete - // type info and only retain the base type. The correct dimensions are later - // recovered through the bounds info. - if (auto seqType = llvm::dyn_cast(varType.getValue())) - if (seqType.hasDynamicExtents()) - varType = mlir::TypeAttr::get(seqType.getEleTy()); - + auto varPtrType = getPtrVarType(retTy); + auto varPtrPtrTy = varPtrPtr ? getPtrVarType(varPtrPtr.getType()) : mlir::TypeAttr{}; mlir::omp::MapInfoOp op = - mlir::omp::MapInfoOp::create(builder, loc, retTy, baseAddr, varType, + mlir::omp::MapInfoOp::create(builder, loc, retTy, baseAddr, varPtrType, builder.getAttr(mapType), builder.getAttr(mapCaptureType), - varPtrPtr, members, membersIndex, bounds, mapperId, + varPtrPtr, varPtrPtrTy, members, membersIndex, bounds, mapperId, builder.getStringAttr(name), builder.getBoolAttr(partialMap)); return op; } diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td index ada3a3edd8a30..f4b0c6ecb4733 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td @@ -123,7 +123,7 @@ def ClauseMapFlagsPresent : I32BitEnumAttrCaseBit<"present", 10>; def ClauseMapFlagsOMPXHold : I32BitEnumAttrCaseBit<"ompx_hold", 11>; def ClauseMapFlagsAttach : I32BitEnumAttrCaseBit<"attach", 12>; def ClauseMapFlagsAttachAlways : I32BitEnumAttrCaseBit<"attach_always", 13>; -def ClauseMapFlagsAttachNone : I32BitEnumAttrCaseBit<"attach_none", 14>; +def ClauseMapFlagsAttachNever : I32BitEnumAttrCaseBit<"attach_never", 14>; def ClauseMapFlagsAttachAuto : I32BitEnumAttrCaseBit<"attach_auto", 15>; def ClauseMapFlagsRefPtr : I32BitEnumAttrCaseBit<"ref_ptr", 16>; def ClauseMapFlagsRefPtee : I32BitEnumAttrCaseBit<"ref_ptee", 17>; @@ -148,7 +148,7 @@ def ClauseMapFlags : OpenMP_BitEnumAttr< ClauseMapFlagsOMPXHold, ClauseMapFlagsAttach, ClauseMapFlagsAttachAlways, - ClauseMapFlagsAttachNone, + ClauseMapFlagsAttachNever, ClauseMapFlagsAttachAuto, ClauseMapFlagsRefPtr, ClauseMapFlagsRefPtee, diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td index 377f1febf6b8f..292e5da465c86 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -1197,10 +1197,11 @@ def MapBoundsOp : OpenMP_Op<"map.bounds", def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> { let arguments = (ins OpenMP_PointerLikeType:$var_ptr, - TypeAttr:$var_type, + TypeAttr:$var_ptr_type, ClauseMapFlagsAttr:$map_type, VariableCaptureKindAttr:$map_capture_type, Optional:$var_ptr_ptr, + OptionalAttr:$var_ptr_ptr_type, Variadic:$members, OptionalAttr:$members_index, Variadic:$bounds, /* rank-0 to rank-{n-1} */ @@ -1239,7 +1240,7 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> { Description of arguments: - `var_ptr`: The address of variable to copy. - - `var_type`: The type of the variable to copy. + - `var_ptr_type`: The type of the var_ptr variable to copy. - 'map_type': OpenMP map type for this map capture, for example: from, to and always. It's a bitfield composed of the OpenMP runtime flags stored in OpenMPOffloadMappingFlags. @@ -1247,6 +1248,8 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> { this can affect how the variable is lowered. - `var_ptr_ptr`: Used when the variable copied is a member of a class, structure or derived type and refers to the originating struct. + - `var_ptr_ptr_type`: Used when the variable copied is a member of a class, structure + or derived type and refers to the originating struct type. - `members`: Used to indicate mapped child members for the current MapInfoOp, represented as other MapInfoOp's, utilised in cases where a parent structure type and members of the structure type are being mapped at the same time. @@ -1267,11 +1270,11 @@ def MapInfoOp : OpenMP_Op<"map.info", [AttrSizedOperandSegments]> { }]; let assemblyFormat = [{ - `var_ptr` `(` $var_ptr `:` type($var_ptr) `,` $var_type `)` + `var_ptr` `(` $var_ptr `:` type($var_ptr) `,` $var_ptr_type `)` `map_clauses` `(` custom($map_type) `)` `capture` `(` custom($map_capture_type) `)` oilist( - `var_ptr_ptr` `(` $var_ptr_ptr `:` type($var_ptr_ptr) `)` + `var_ptr_ptr` `(` $var_ptr_ptr `:` type($var_ptr_ptr) `,` $var_ptr_ptr_type`)` | `mapper` `(` $mapper_id `)` | `members` `(` $members `:` custom($members_index) `:` type($members) `)` | `bounds` `(` $bounds `)` diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp index 172f21ff1779e..93e0bfeb01a5e 100644 --- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp +++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp @@ -1806,8 +1806,8 @@ static ParseResult parseMapClause(OpAsmParser &parser, if (mapTypeMod == "attach_always") mapTypeBits |= ClauseMapFlags::attach_always; - if (mapTypeMod == "attach_none") - mapTypeBits |= ClauseMapFlags::attach_none; + if (mapTypeMod == "attach_never") + mapTypeBits |= ClauseMapFlags::attach_never; if (mapTypeMod == "attach_auto") mapTypeBits |= ClauseMapFlags::attach_auto; @@ -1882,8 +1882,8 @@ static void printMapClause(OpAsmPrinter &p, Operation *op, mapTypeStrs.push_back("attach"); if (mapTypeToBool(mapFlags, ClauseMapFlags::attach_always)) mapTypeStrs.push_back("attach_always"); - if (mapTypeToBool(mapFlags, ClauseMapFlags::attach_none)) - mapTypeStrs.push_back("attach_none"); + if (mapTypeToBool(mapFlags, ClauseMapFlags::attach_never)) + mapTypeStrs.push_back("attach_never"); if (mapTypeToBool(mapFlags, ClauseMapFlags::attach_auto)) mapTypeStrs.push_back("attach_auto"); if (mapTypeToBool(mapFlags, ClauseMapFlags::ref_ptr)) @@ -2007,6 +2007,8 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapVars) { bool close = mapTypeToBool(mapTypeBits, ClauseMapFlags::close); bool implicit = mapTypeToBool(mapTypeBits, ClauseMapFlags::implicit); + bool attach = mapTypeToBool(mapTypeBits, ClauseMapFlags::attach); + if ((isa(op) || isa(op)) && del) return emitError(op->getLoc(), "to, from, tofrom and alloc map types are permitted"); @@ -2025,10 +2027,11 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapVars) { "specified, other map types are not permitted"); } - if (!to && !from) { - return emitError(op->getLoc(), - "at least one of to or from map types must be " - "specified, other map types are not permitted"); + if (!to && !from && !attach) { + return emitError( + op->getLoc(), + "at least one of to or from or attach map types must be " + "specified, other map types are not permitted"); } auto updateVar = mapInfoOp.getVarPtr(); @@ -2046,7 +2049,19 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapVars) { "present, mapper and iterator map type modifiers are permitted"); } - to ? updateToVars.insert(updateVar) : updateFromVars.insert(updateVar); + // It's possible we have an attach map, in which case if there is no to + // or from tied to it, we skip insertion. + if (to || from) { + to ? updateToVars.insert(updateVar) + : updateFromVars.insert(updateVar); + } + } + + if ((mapInfoOp.getVarPtrPtr() && !mapInfoOp.getVarPtrPtrType()) || + (!mapInfoOp.getVarPtrPtr() && mapInfoOp.getVarPtrPtrType())) { + return emitError( + op->getLoc(), + "both the varPtrPtr and varPtrPtrType must be present"); } } else if (!isa(op)) { return emitError(op->getLoc(), diff --git a/mlir/lib/Dialect/OpenMP/Transforms/OpenMPOffloadPrivatizationPrepare.cpp b/mlir/lib/Dialect/OpenMP/Transforms/OpenMPOffloadPrivatizationPrepare.cpp index c117d9b034b7a..d3182a51b3a3f 100644 --- a/mlir/lib/Dialect/OpenMP/Transforms/OpenMPOffloadPrivatizationPrepare.cpp +++ b/mlir/lib/Dialect/OpenMP/Transforms/OpenMPOffloadPrivatizationPrepare.cpp @@ -127,7 +127,7 @@ class PrepareForOMPOffloadPrivatizationPass // For boxchars this won't be a pointer. But, MapsForPrivatizedSymbols // should have mapped the pointer to the boxchar so use that as varPtr. Value varPtr = mapInfoOp.getVarPtr(); - Type varType = mapInfoOp.getVarType(); + Type varType = mapInfoOp.getVarPtrType(); bool isPrivatizedByValue = !isa(privVar.getType()); diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index 242532e5879e6..32b1fb70debe8 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -3873,56 +3873,57 @@ static llvm::Value *getSizeInBytes(DataLayout &dl, const mlir::Type &type, return builder.getInt64(dl.getTypeSizeInBits(type) / 8); } +static bool checkHasClauseMapFlag(omp::ClauseMapFlags flag, + omp::ClauseMapFlags checkFlag) { + return (flag & checkFlag) == checkFlag; +} + // Convert the MLIR map flag set to the runtime map flag set for embedding // in LLVM-IR. This is important as the two bit-flag lists do not correspond // 1-to-1 as there's flags the runtime doesn't care about and vice versa. // Certain flags are discarded here such as RefPtee and co. static llvm::omp::OpenMPOffloadMappingFlags convertClauseMapFlags(omp::ClauseMapFlags mlirFlags) { - auto mapTypeToBool = [&mlirFlags](omp::ClauseMapFlags flag) { - return (mlirFlags & flag) == flag; - }; - llvm::omp::OpenMPOffloadMappingFlags mapType = llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE; - if (mapTypeToBool(omp::ClauseMapFlags::to)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::to)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO; - if (mapTypeToBool(omp::ClauseMapFlags::from)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::from)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; - if (mapTypeToBool(omp::ClauseMapFlags::always)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::always)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS; - if (mapTypeToBool(omp::ClauseMapFlags::del)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::del)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE; - if (mapTypeToBool(omp::ClauseMapFlags::return_param)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::return_param)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_RETURN_PARAM; - if (mapTypeToBool(omp::ClauseMapFlags::priv)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::priv)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRIVATE; - if (mapTypeToBool(omp::ClauseMapFlags::literal)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::literal)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL; - if (mapTypeToBool(omp::ClauseMapFlags::implicit)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::implicit)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT; - if (mapTypeToBool(omp::ClauseMapFlags::close)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::close)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_CLOSE; - if (mapTypeToBool(omp::ClauseMapFlags::present)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::present)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PRESENT; - if (mapTypeToBool(omp::ClauseMapFlags::ompx_hold)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::ompx_hold)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_OMPX_HOLD; - if (mapTypeToBool(omp::ClauseMapFlags::attach)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::attach)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH; - if (mapTypeToBool(omp::ClauseMapFlags::descriptor)) + if (checkHasClauseMapFlag(mlirFlags, omp::ClauseMapFlags::descriptor)) mapType |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DESCRIPTOR; return mapType; @@ -3934,6 +3935,16 @@ static void collectMapDataFromMapOperands( llvm::IRBuilderBase &builder, ArrayRef useDevPtrOperands = {}, ArrayRef useDevAddrOperands = {}, ArrayRef hasDevAddrOperands = {}) { + + auto checkRefPtrOrPteeMapWithAttach = [](omp::ClauseMapFlags mapType) { + bool hasRefType = + checkHasClauseMapFlag(mapType, omp::ClauseMapFlags::ref_ptr) || + checkHasClauseMapFlag(mapType, omp::ClauseMapFlags::ref_ptee) || + checkHasClauseMapFlag(mapType, omp::ClauseMapFlags::ref_ptr_ptee); + return hasRefType && + checkHasClauseMapFlag(mapType, omp::ClauseMapFlags::attach); + }; + auto checkIsAMember = [](const auto &mapVars, auto mapOp) { // Check if this is a member mapping and correctly assign that it is, if // it is a member of a larger object. @@ -3954,10 +3965,13 @@ static void collectMapDataFromMapOperands( // Process MapOperands for (Value mapValue : mapVars) { auto mapOp = cast(mapValue.getDefiningOp()); - Value offloadPtr = - mapOp.getVarPtrPtr() ? mapOp.getVarPtrPtr() : mapOp.getVarPtr(); + bool isRefPtrOrPteeMapWithAttach = checkRefPtrOrPteeMapWithAttach(mapOp.getMapType()); + Value offloadPtr = (mapOp.getVarPtrPtr() && !isRefPtrOrPteeMapWithAttach) ? mapOp.getVarPtrPtr() + : mapOp.getVarPtr(); mapData.OriginalValue.push_back(moduleTranslation.lookupValue(offloadPtr)); - mapData.Pointers.push_back(mapData.OriginalValue.back()); + mapData.Pointers.push_back( + isRefPtrOrPteeMapWithAttach ? moduleTranslation.lookupValue(mapOp.getVarPtrPtr()) + : mapData.OriginalValue.back()); // if is declare target link OR to/enter in USM mode if (llvm::Value *refPtr = @@ -3972,11 +3986,27 @@ static void collectMapDataFromMapOperands( mapData.BasePointers.push_back(mapData.OriginalValue.back()); } - mapData.BaseType.push_back( - moduleTranslation.convertType(mapOp.getVarType())); - mapData.Sizes.push_back( - getSizeInBytes(dl, mapOp.getVarType(), mapOp, mapData.Pointers.back(), - mapData.BaseType.back(), builder, moduleTranslation)); + // In every situation we currently have if we have a varPtrPtr present + // we wish to utilise it's type for the base type, main cases are + // currently Fortran descriptor base address maps and attach maps. + mapData.BaseType.push_back(moduleTranslation.convertType( + mapOp.getVarPtrPtr() ? mapOp.getVarPtrPtrType().value() + : mapOp.getVarPtrType())); + + // For the attach map cases, it's a little odd, as we effectively have to + // utilise the base address (including all bounds offsets) for the pointer + // field, the pointer address for the base address field, and the pointer + // not the data (base addresses) size. So we end up with a mix of base + // types and sizes we wish to insert here. + mlir::Type sizeType = (isRefPtrOrPteeMapWithAttach || !mapOp.getVarPtrPtr()) + ? mapOp.getVarPtrType() + : mapOp.getVarPtrPtrType().value(); + + mapData.Sizes.push_back(getSizeInBytes( + dl, sizeType, isRefPtrOrPteeMapWithAttach ? nullptr : mapOp, + mapData.Pointers.back(), moduleTranslation.convertType(sizeType), + builder, moduleTranslation)); + mapData.MapClause.push_back(mapOp.getOperation()); mapData.Types.push_back(convertClauseMapFlags(mapOp.getMapType())); mapData.Names.push_back(LLVM::createMappingInformation( @@ -4031,7 +4061,7 @@ static void collectMapDataFromMapOperands( mapData.IsDeclareTarget.push_back(false); mapData.BasePointers.push_back(mapData.OriginalValue.back()); mapData.BaseType.push_back( - moduleTranslation.convertType(mapOp.getVarType())); + moduleTranslation.convertType(mapOp.getVarPtrType())); // If we're an attach map, we need to maintain the size currently, even // if we're not sending data, as the runtime (at least currently) @@ -4040,7 +4070,7 @@ static void collectMapDataFromMapOperands( llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH) == llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH) { mapData.Sizes.push_back(getSizeInBytes( - dl, mapOp.getVarType(), mapOp, mapData.Pointers.back(), + dl, mapOp.getVarPtrType(), mapOp, mapData.Pointers.back(), mapData.BaseType.back(), builder, moduleTranslation)); } else { mapData.Sizes.push_back(builder.getInt64(0)); @@ -4075,9 +4105,9 @@ static void collectMapDataFromMapOperands( mapData.Pointers.push_back(origValue); mapData.IsDeclareTarget.push_back(false); mapData.BaseType.push_back( - moduleTranslation.convertType(mapOp.getVarType())); + moduleTranslation.convertType(mapOp.getVarPtrType())); mapData.Sizes.push_back( - builder.getInt64(dl.getTypeSize(mapOp.getVarType()))); + builder.getInt64(dl.getTypeSize(mapOp.getVarPtrType()))); mapData.MapClause.push_back(mapOp.getOperation()); if (llvm::to_underlying(mapType & mapTypeAlways)) { // Descriptors are mapped with the ALWAYS flag, since they can get @@ -4383,7 +4413,15 @@ processIndividualMap(llvm::IRBuilderBase &builder, !isPtrTy) mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_LITERAL; - if (memberOfFlag != llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE) + // if we have a pointer and it's part of a MEMBER_OF mapping we do not apply + // MEMBER_OF, as the runtime currently has a work-around that utilises + // MEMBER_OF to prevent reference updating in certain scenarios instead of + // target_param, however, this causes a noticable issue in cases where we + // map some data (Fortran descriptor primarily at the moment), alter it on + // the host, and then expect it to not be updated in a subsequent impliict map + // (such as an implicit map on a target). + if (memberOfFlag != llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE && + !isPtrTy) ompBuilder.setCorrectMemberOfFlag(mapFlag, memberOfFlag); // if we're provided a mapDataParentIdx, then the data being mapped is @@ -4481,14 +4519,13 @@ gatherImmediateMembers(llvm::SmallVector &immediateMapDataIdxs, } } -void processAttachMap(LLVM::ModuleTranslation &moduleTranslation, - llvm::IRBuilderBase &builder, - llvm::OpenMPIRBuilder &ompBuilder, DataLayout &dl, - MapInfosTy &combinedInfo, MapInfoData &mapData, - uint64_t mapDataIndex, bool parentMap, - llvm::SmallVectorImpl &immediateMapDataIdxs, - llvm::omp::OpenMPOffloadMappingFlags memberOfFlag, - TargetDirective targetDirective) { +static void processAttachMap( + LLVM::ModuleTranslation &moduleTranslation, llvm::IRBuilderBase &builder, + llvm::OpenMPIRBuilder &ompBuilder, DataLayout &dl, MapInfosTy &combinedInfo, + MapInfoData &mapData, uint64_t mapDataIndex, bool parentMap, + llvm::SmallVectorImpl &immediateMapDataIdxs, + llvm::omp::OpenMPOffloadMappingFlags memberOfFlag, + TargetDirective targetDirective, bool alwaysAttach = false) { assert(!ompBuilder.Config.isTargetDevice() && "function only supported for host device codegen"); auto parentClause = @@ -4570,7 +4607,8 @@ void processAttachMap(LLVM::ModuleTranslation &moduleTranslation, // still blocked. combinedInfo.Types.emplace_back( llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH | - llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS); + ((alwaysAttach) ? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS + : llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE)); combinedInfo.DevicePointers.emplace_back( llvm::OpenMPIRBuilder::DeviceInfoTy::None); combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]); @@ -4696,7 +4734,29 @@ static void mapParentWithMembers( llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_CLOSE; ompBuilder.setCorrectMemberOfFlag(mapFlag, memberOfFlag); - if (targetDirective == TargetDirective::TargetUpdate || hasMapClose) { + // AG NOTE: Forcing it into the top path works fine, we could temporarily + // force any mappings with attach into this path till we work out a + // solution, which will leave some avenue for the optimization... we would + // have to make the attach a member of the derived type then, and it's not + // really designed to have more members at the current index. Another method + // might be to traverse the masp and look for other members with a shared + // descriptor/address and check... + // part of the issue here is likely that the base address (element 0) + // and the descriptors address, share the same initial address offset, + // so if we calcualte the size from them for the overlap, we end up with a size of + /// 0, which is fine normally but the NewlyAllocated check takes into account size... + // - What we can do: + // 1) adjust the lowering to take into account <= for the size part... unsure + // how accurate that would be long term + // 2) Adjust the size calculation here to use the size of the object if we have it pre-computed + // rather than self-calculate it / a conditional to select it. The issue with this, is that + // we likely haven't calculated the reality that it's a ptr and we're going to feed it 48, + // which technically isn't correrct... it should be 8... or nothing at all as it's either the + // data which isn't part of the object or the pointer in the descriptor, and it shouldn't really + // be both. + // AG NOTE: We will want to remove the target_param component of + // any processIndividualMap's that have ATTACH just to be safe. + // if (targetDirective == TargetDirective::TargetUpdate || hasMapClose) { combinedInfo.Types.emplace_back(mapFlag); combinedInfo.DevicePointers.emplace_back( mapData.DevicePointers[mapDataIndex]); @@ -4707,66 +4767,66 @@ static void mapParentWithMembers( mapData.BasePointers[mapDataIndex]); combinedInfo.Pointers.emplace_back(mapData.Pointers[mapDataIndex]); combinedInfo.Sizes.emplace_back(mapData.Sizes[mapDataIndex]); - } else { - llvm::SmallVector overlapIdxs; - // Find all of the members that "overlap", i.e. occlude other members that - // were mapped alongside the parent, e.g. member [0], occludes - getOverlappedMembers(overlapIdxs, mapData, parentClause); - // We need to make sure the overlapped members are sorted in order of - // lowest address to highest address - sortMapIndices(overlapIdxs, parentClause); - - lowAddr = builder.CreatePointerCast(mapData.Pointers[mapDataIndex], - builder.getPtrTy()); - highAddr = builder.CreatePointerCast( - builder.CreateConstGEP1_32(mapData.BaseType[mapDataIndex], - mapData.Pointers[mapDataIndex], 1), - builder.getPtrTy()); - - // TODO: We may want to skip arrays/array sections in this as Clang does - // so it appears to be an optimisation rather than a neccessity though, - // but this requires further investigation. However, we would have to make - // sure to not exclude maps with bounds that ARE pointers, as these are - // processed as seperate components, i.e. pointer + data. - for (auto v : overlapIdxs) { - auto mapDataOverlapIdx = getMapDataMemberIdx( - mapData, - cast(parentClause.getMembers()[v].getDefiningOp())); - combinedInfo.Types.emplace_back(mapFlag); - combinedInfo.DevicePointers.emplace_back( - mapData.DevicePointers[mapDataOverlapIdx]); - combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataOverlapIdx]); - combinedInfo.Names.emplace_back(LLVM::createMappingInformation( - mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder)); - combinedInfo.BasePointers.emplace_back( - mapData.BasePointers[mapDataIndex]); - combinedInfo.Pointers.emplace_back(lowAddr); - combinedInfo.Sizes.emplace_back(builder.CreateIntCast( - builder.CreatePtrDiff(builder.getInt8Ty(), - mapData.OriginalValue[mapDataOverlapIdx], - lowAddr), - builder.getInt64Ty(), /*isSigned=*/true)); - lowAddr = builder.CreateConstGEP1_32( - checkIfPointerMap(llvm::cast( - mapData.MapClause[mapDataOverlapIdx])) - ? builder.getPtrTy() - : mapData.BaseType[mapDataOverlapIdx], - mapData.BasePointers[mapDataOverlapIdx], 1); - } - - combinedInfo.Types.emplace_back(mapFlag); - combinedInfo.DevicePointers.emplace_back( - mapData.DevicePointers[mapDataIndex]); - combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]); - combinedInfo.Names.emplace_back(LLVM::createMappingInformation( - mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder)); - combinedInfo.BasePointers.emplace_back( - mapData.BasePointers[mapDataIndex]); - combinedInfo.Pointers.emplace_back(lowAddr); - combinedInfo.Sizes.emplace_back(builder.CreateIntCast( - builder.CreatePtrDiff(builder.getInt8Ty(), highAddr, lowAddr), - builder.getInt64Ty(), true)); - } + // } else { + // llvm::SmallVector overlapIdxs; + // // Find all of the members that "overlap", i.e. occlude other members that + // // were mapped alongside the parent, e.g. member [0], occludes + // getOverlappedMembers(overlapIdxs, mapData, parentClause); + // // We need to make sure the overlapped members are sorted in order of + // // lowest address to highest address + // sortMapIndices(overlapIdxs, parentClause); + + // lowAddr = builder.CreatePointerCast(mapData.Pointers[mapDataIndex], + // builder.getPtrTy()); + // highAddr = builder.CreatePointerCast( + // builder.CreateConstGEP1_32(mapData.BaseType[mapDataIndex], + // mapData.Pointers[mapDataIndex], 1), + // builder.getPtrTy()); + + // // TODO: We may want to skip arrays/array sections in this as Clang does + // // so it appears to be an optimisation rather than a neccessity though, + // // but this requires further investigation. However, we would have to make + // // sure to not exclude maps with bounds that ARE pointers, as these are + // // processed as seperate components, i.e. pointer + data. + // for (auto v : overlapIdxs) { + // auto mapDataOverlapIdx = getMapDataMemberIdx( + // mapData, + // cast(parentClause.getMembers()[v].getDefiningOp())); + // combinedInfo.Types.emplace_back(mapFlag); + // combinedInfo.DevicePointers.emplace_back( + // mapData.DevicePointers[mapDataOverlapIdx]); + // combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataOverlapIdx]); + // combinedInfo.Names.emplace_back(LLVM::createMappingInformation( + // mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder)); + // combinedInfo.BasePointers.emplace_back( + // mapData.BasePointers[mapDataIndex]); + // combinedInfo.Pointers.emplace_back(lowAddr); + // combinedInfo.Sizes.emplace_back(builder.CreateIntCast( + // builder.CreatePtrDiff(builder.getInt8Ty(), + // mapData.OriginalValue[mapDataOverlapIdx], + // lowAddr), + // builder.getInt64Ty(), /*isSigned=*/true)); + // lowAddr = builder.CreateConstGEP1_32( + // checkIfPointerMap(llvm::cast( + // mapData.MapClause[mapDataOverlapIdx])) + // ? builder.getPtrTy() + // : mapData.BaseType[mapDataOverlapIdx], + // mapData.BasePointers[mapDataOverlapIdx], 1); + // } + + // combinedInfo.Types.emplace_back(mapFlag); + // combinedInfo.DevicePointers.emplace_back( + // mapData.DevicePointers[mapDataIndex]); + // combinedInfo.Mappers.emplace_back(mapData.Mappers[mapDataIndex]); + // combinedInfo.Names.emplace_back(LLVM::createMappingInformation( + // mapData.MapClause[mapDataIndex]->getLoc(), ompBuilder)); + // combinedInfo.BasePointers.emplace_back( + // mapData.BasePointers[mapDataIndex]); + // combinedInfo.Pointers.emplace_back(lowAddr); + // combinedInfo.Sizes.emplace_back(builder.CreateIntCast( + // builder.CreatePtrDiff(builder.getInt8Ty(), highAddr, lowAddr), + // builder.getInt64Ty(), true)); + // } } } @@ -4834,9 +4894,15 @@ static void processMapWithMembersOf(LLVM::ModuleTranslation &moduleTranslation, llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH) { llvm::SmallVector immediateMapDataIdxs; gatherImmediateMembers(immediateMapDataIdxs, mapData, parentClause, map); - processAttachMap(moduleTranslation, builder, ompBuilder, dl, combinedInfo, - mapData, mapInfoIdx, i == 0, immediateMapDataIdxs, - memberOfFlag, targetDirective); + // TODO: Handle attach_always more elegantly in the frontend, we will have + // to split up the ref_ptr_ptee/default Fortran pointer and allocatable + // mapping into their constituient components to do so. + processAttachMap( + moduleTranslation, builder, ompBuilder, dl, combinedInfo, mapData, + mapInfoIdx, i == 0, immediateMapDataIdxs, memberOfFlag, + targetDirective, + checkHasClauseMapFlag(map.getMapType(), + omp::ClauseMapFlags::attach_always)); i++; continue; } @@ -4871,9 +4937,16 @@ createAlteredByCaptureMap(MapInfoData &mapData, assert(!moduleTranslation.getOpenMPBuilder()->Config.isTargetDevice() && "function only supported for host device codegen"); for (size_t i = 0; i < mapData.MapClause.size(); ++i) { - // if it's declare target, skip it, it's handled separately. - if (!mapData.IsDeclareTarget[i]) { - auto mapOp = cast(mapData.MapClause[i]); + auto mapOp = cast(mapData.MapClause[i]); + bool isAttachMap = + ((convertClauseMapFlags(mapOp.getMapType()) & + llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH) == + llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ATTACH); + + // if it's declare target, skip it, it's handled separately. However, if + // it's declare target, and an attach map, we want to calculate the exact + // address offset so that we attach correctly. + if (!mapData.IsDeclareTarget[i] || (mapData.IsDeclareTarget[i] && isAttachMap)) { omp::VariableCaptureKind captureKind = mapOp.getMapCaptureType(); bool isPtrTy = checkIfPointerMap(mapOp); @@ -4960,8 +5033,6 @@ static void genMapInfos(llvm::IRBuilderBase &builder, // utilise the size from any component of MapInfoData, if we can't // something is missing from the initial MapInfoData construction. for (size_t i = 0; i < mapData.MapClause.size(); ++i) { - // NOTE/TODO: We currently do not support arbitrary depth record - // type mapping. if (mapData.IsAMember[i]) continue; @@ -5642,7 +5713,7 @@ createDeviceArgumentAccessor(MapInfoData &mapData, llvm::Argument &arg, capture = mapOp.getMapCaptureType(); // Get information of alignment of mapped object alignmentValue = typeToLLVMIRTranslator.getPreferredAlignment( - mapOp.getVarType(), ompBuilder.M.getDataLayout()); + mapOp.getVarPtrType(), ompBuilder.M.getDataLayout()); break; } @@ -6114,7 +6185,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder, // So, we don't store it in any datastructure. Instead, we just // do some sanity checks on it right now. auto mapInfoOp = mappedValue.getDefiningOp(); - [[maybe_unused]] Type varType = mapInfoOp.getVarType(); + [[maybe_unused]] Type varType = mapInfoOp.getVarPtrType(); // Check #1: Check that the type of the private variable matches // the type of the variable being mapped.