llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-fir-hlfir Author: None (agozillon) <details> <summary>Changes</summary> This patch is one in a series of four patches that seeks to refactor slightly and extend the current record type map support that was put in place for Fortran's descriptor types to handle explicit member mapping for record types at a single level of depth. For example, the below case where two members of a Fortran derived type are mapped explicitly: '''' type :: scalar_and_array real(4) :: real integer(4) :: array(10) integer(4) :: int end type scalar_and_array type(scalar_and_array) :: scalar_arr !$omp target map(tofrom: scalar_arr%int, scalar_arr%real) '''' Current cases of derived type mapping left for future work are: > explicit member mapping of nested members (e.g. two layers of record types where we explicitly map a member from the internal record type) > Fortran's automagical mapping of all elements and nested elements of a derived type > explicit member mapping of a derived type and then constituient members (redundant in Fortran due to former case but still legal as far as I am aware) > explicit member mapping of a record type (may be handled reasonably, just not fully tested in this iteration) > explicit member mapping for Fortran allocatable types (a variation of nested record types) This patch seeks to support this by extending the Flang-new OpenMP lowering to support generation of this newly required information, creating the neccessary parent <-to-> member map_info links, calculating the member indices and setting if it's a partial map. The OMPDescriptorMapInfoGen pass has also been generalized into a map finalization phase, now named OMPMapInfoFinalization. This pass was extended to support the insertion of member maps into the BlockArg and MapOperands of relevant map carrying operations. Similar to the method in which descriptor types are expanded and constituient members inserted. --- Patch is 98.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81511.diff 20 Files Affected: - (modified) flang/docs/OpenMP-descriptor-management.md (+2-2) - (modified) flang/include/flang/Optimizer/Transforms/Passes.h (+1-1) - (modified) flang/include/flang/Optimizer/Transforms/Passes.td (+3-3) - (modified) flang/include/flang/Tools/CLOptions.inc (+1-1) - (modified) flang/lib/Lower/OpenMP.cpp (+210-25) - (modified) flang/lib/Optimizer/Transforms/CMakeLists.txt (+1-1) - (removed) flang/lib/Optimizer/Transforms/OMPDescriptorMapInfoGen.cpp (-168) - (added) flang/lib/Optimizer/Transforms/OMPMapInfoFinalization.cpp (+262) - (modified) flang/test/Fir/convert-to-llvm-openmp-and-fir.fir (+31-4) - (modified) flang/test/Integration/OpenMP/map-types-and-sizes.f90 (+153-1) - (modified) flang/test/Lower/OpenMP/FIR/array-bounds.f90 (+2-2) - (modified) flang/test/Lower/OpenMP/FIR/map-component-ref.f90 (+2-2) - (modified) flang/test/Lower/OpenMP/FIR/target.f90 (+2-2) - (modified) flang/test/Lower/OpenMP/allocatable-array-bounds.f90 (+6-6) - (modified) flang/test/Lower/OpenMP/allocatable-map.f90 (+4-4) - (modified) flang/test/Lower/OpenMP/array-bounds.f90 (+4-4) - (added) flang/test/Lower/OpenMP/derived-type-map.f90 (+105) - (modified) flang/test/Lower/OpenMP/map-component-ref.f90 (+1-1) - (modified) flang/test/Lower/OpenMP/target.f90 (+2-2) - (renamed) flang/test/Transforms/omp-map-info-finalization.fir (+29-5) ``````````diff diff --git a/flang/docs/OpenMP-descriptor-management.md b/flang/docs/OpenMP-descriptor-management.md index 90a20282e05126..af02b3a99cb07d 100644 --- a/flang/docs/OpenMP-descriptor-management.md +++ b/flang/docs/OpenMP-descriptor-management.md @@ -44,7 +44,7 @@ Currently, Flang will lower these descriptor types in the OpenMP lowering (lower to all other map types, generating an omp.MapInfoOp containing relevant information required for lowering the OpenMP dialect to LLVM-IR during the final stages of the MLIR lowering. However, after the lowering to FIR/HLFIR has been performed an OpenMP dialect specific pass for Fortran, -`OMPDescriptorMapInfoGenPass` (Optimizer/OMPDescriptorMapInfoGen.cpp) will expand the +`OMPMapInfoFinalizationPass` (Optimizer/OMPMapInfoFinalization.cpp) will expand the `omp.MapInfoOp`'s containing descriptors (which currently will be a `BoxType` or `BoxAddrOp`) into multiple mappings, with one extra per pointer member in the descriptor that is supported on top of the original descriptor map operation. These pointers members are linked to the parent descriptor by adding them to @@ -52,7 +52,7 @@ the member field of the original descriptor map operation, they are then inserte owning operation's (`omp.TargetOp`, `omp.DataOp` etc.) map operand list and in cases where the owning operation is `IsolatedFromAbove`, it also inserts them as `BlockArgs` to canonicalize the mappings and simplify lowering. -An example transformation by the `OMPDescriptorMapInfoGenPass`: +An example transformation by the `OMPMapInfoFinalizationPass`: ``` diff --git a/flang/include/flang/Optimizer/Transforms/Passes.h b/flang/include/flang/Optimizer/Transforms/Passes.h index e1d22c8c986da7..fc9a098c3931d3 100644 --- a/flang/include/flang/Optimizer/Transforms/Passes.h +++ b/flang/include/flang/Optimizer/Transforms/Passes.h @@ -76,7 +76,7 @@ std::unique_ptr<mlir::Pass> createAlgebraicSimplificationPass(const mlir::GreedyRewriteConfig &config); std::unique_ptr<mlir::Pass> createPolymorphicOpConversionPass(); -std::unique_ptr<mlir::Pass> createOMPDescriptorMapInfoGenPass(); +std::unique_ptr<mlir::Pass> createOMPMapInfoFinalizationPass(); std::unique_ptr<mlir::Pass> createOMPFunctionFilteringPass(); std::unique_ptr<mlir::OperationPass<mlir::ModuleOp>> createOMPMarkDeclareTargetPass(); diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td index 5fb576fd876254..0638ae49f5f4ea 100644 --- a/flang/include/flang/Optimizer/Transforms/Passes.td +++ b/flang/include/flang/Optimizer/Transforms/Passes.td @@ -318,15 +318,15 @@ def LoopVersioning : Pass<"loop-versioning", "mlir::func::FuncOp"> { let dependentDialects = [ "fir::FIROpsDialect" ]; } -def OMPDescriptorMapInfoGenPass - : Pass<"omp-descriptor-map-info-gen", "mlir::func::FuncOp"> { +def OMPMapInfoFinalizationPass + : Pass<"omp-map-info-finalization", "mlir::func::FuncOp"> { let summary = "expands OpenMP MapInfo operations containing descriptors"; let description = [{ Expands MapInfo operations containing descriptor types into multiple MapInfo's for each pointer element in the descriptor that requires explicit individual mapping by the OpenMP runtime. }]; - let constructor = "::fir::createOMPDescriptorMapInfoGenPass()"; + let constructor = "::fir::createOMPMapInfoFinalizationPass()"; let dependentDialects = ["mlir::omp::OpenMPDialect"]; } diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc index 68e504d0ccb512..ec3d634ac0264b 100644 --- a/flang/include/flang/Tools/CLOptions.inc +++ b/flang/include/flang/Tools/CLOptions.inc @@ -274,7 +274,7 @@ inline void createHLFIRToFIRPassPipeline( /// rather than the host device. inline void createOpenMPFIRPassPipeline( mlir::PassManager &pm, bool isTargetDevice) { - pm.addPass(fir::createOMPDescriptorMapInfoGenPass()); + pm.addPass(fir::createOMPMapInfoFinalizationPass()); pm.addPass(fir::createOMPMarkDeclareTargetPass()); if (isTargetDevice) pm.addPass(fir::createOMPFunctionFilteringPass()); diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp index fd18b212bad515..86fdc51602cf12 100644 --- a/flang/lib/Lower/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP.cpp @@ -49,14 +49,16 @@ using DeclareTargetCapturePair = //===----------------------------------------------------------------------===// static Fortran::semantics::Symbol * -getOmpObjectSymbol(const Fortran::parser::OmpObject &ompObject) { +getOmpObjParentSymbol(const Fortran::parser::OmpObject &ompObject) { Fortran::semantics::Symbol *sym = nullptr; std::visit( Fortran::common::visitors{ [&](const Fortran::parser::Designator &designator) { - if (auto *arrayEle = - Fortran::parser::Unwrap<Fortran::parser::ArrayElement>( - designator)) { + if (auto *structComp = Fortran::parser::Unwrap< + Fortran::parser::StructureComponent>(designator)) { + sym = GetFirstName(structComp->base).symbol; + } else if (auto *arrayEle = Fortran::parser::Unwrap< + Fortran::parser::ArrayElement>(designator)) { sym = GetFirstName(arrayEle->base).symbol; } else if (auto *structComp = Fortran::parser::Unwrap< Fortran::parser::StructureComponent>(designator)) { @@ -72,6 +74,29 @@ getOmpObjectSymbol(const Fortran::parser::OmpObject &ompObject) { return sym; } +static Fortran::semantics::Symbol * +getOmpObjectSymbol(const Fortran::parser::OmpObject &ompObject) { + Fortran::semantics::Symbol *sym = nullptr; + std::visit( + Fortran::common::visitors{ + [&](const Fortran::parser::Designator &designator) { + if (auto *structComp = Fortran::parser::Unwrap< + Fortran::parser::StructureComponent>(designator)) { + sym = structComp->component.symbol; + } else if (auto *arrayEle = Fortran::parser::Unwrap< + Fortran::parser::ArrayElement>(designator)) { + sym = GetLastName(arrayEle->base).symbol; + } else if (const Fortran::parser::Name *name = + Fortran::semantics::getDesignatorNameIfDataRef( + designator)) { + sym = name->symbol; + } + }, + [&](const Fortran::parser::Name &name) { sym = name.symbol; }}, + ompObject.u); + return sym; +} + static void genObjectList(const Fortran::parser::OmpObjectList &objectList, Fortran::lower::AbstractConverter &converter, llvm::SmallVectorImpl<mlir::Value> &operands) { @@ -1829,9 +1854,10 @@ static mlir::omp::MapInfoOp createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc, mlir::Value baseAddr, mlir::Value varPtrPtr, std::string name, mlir::SmallVector<mlir::Value> bounds, - mlir::SmallVector<mlir::Value> members, uint64_t mapType, + mlir::SmallVector<mlir::Value> members, + mlir::ArrayAttr membersIndex, uint64_t mapType, mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type retTy, - bool isVal = false) { + bool partialMap = false) { if (auto boxTy = baseAddr.getType().dyn_cast<fir::BaseBoxType>()) { baseAddr = builder.create<fir::BoxAddrOp>(loc, baseAddr); retTy = baseAddr.getType(); @@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc, llvm::cast<mlir::omp::PointerLikeType>(retTy).getElementType()); mlir::omp::MapInfoOp op = builder.create<mlir::omp::MapInfoOp>( - loc, retTy, baseAddr, varType, varPtrPtr, members, bounds, + loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds, builder.getIntegerAttr(builder.getIntegerType(64, false), mapType), builder.getAttr<mlir::omp::VariableCaptureKindAttr>(mapCaptureType), - builder.getStringAttr(name)); + builder.getStringAttr(name), builder.getBoolAttr(partialMap)); return op; } +int findComponenetMemberPlacement( + const Fortran::semantics::Symbol *dTypeSym, + const Fortran::semantics::Symbol *componentSym) { + int placement = -1; + if (const auto *derived{ + dTypeSym->detailsIf<Fortran::semantics::DerivedTypeDetails>()}) { + for (auto t : derived->componentNames()) { + placement++; + if (t == componentSym->name()) + return placement; + } + } + return placement; +} + +static void +checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter, + llvm::omp::OpenMPOffloadMappingFlags &mapFlags, + Fortran::semantics::Symbol *symbol) { + mlir::Operation *op = + converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol)); + if (op) + if (auto declareTargetOp = + llvm::dyn_cast<mlir::omp::DeclareTargetInterface>(op)) { + // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause + // functions fairly different. + if (declareTargetOp.getDeclareTargetCaptureClause() == + mlir::omp::DeclareTargetCaptureClause::link) + mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ; + } +} + +static void insertChildMapInfoIntoParent( + Fortran::lower::AbstractConverter &converter, + llvm::SmallVector<const Fortran::semantics::Symbol *> &memberParentSyms, + llvm::SmallVector<mlir::Value> &memberMaps, + llvm::SmallVector<mlir::Attribute> &memberPlacementIndices, + llvm::SmallVectorImpl<mlir::Value> &mapOperands, + llvm::SmallVectorImpl<mlir::Type> *mapSymTypes, + llvm::SmallVectorImpl<mlir::Location> *mapSymLocs, + llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *mapSymbols) { + // TODO: For multi-nested record types the top level parent is currently + // the containing parent for all member operations. + for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) { + bool parentExists = false; + size_t parentIdx = 0; + for (size_t i = 0; i < mapSymbols->size(); ++i) { + if ((*mapSymbols)[i] == sym) { + parentExists = true; + parentIdx = i; + } + } + + if (parentExists) { + // found a parent, append. + if (auto mapOp = mlir::dyn_cast<mlir::omp::MapInfoOp>( + mapOperands[parentIdx].getDefiningOp())) { + mapOp.getMembersMutable().append(memberMaps[idx]); + llvm::SmallVector<mlir::Attribute> memberIndexTmp{ + mapOp.getMembersIndexAttr().begin(), + mapOp.getMembersIndexAttr().end()}; + memberIndexTmp.push_back(memberPlacementIndices[idx]); + mapOp.setMembersIndexAttr(mlir::ArrayAttr::get( + converter.getFirOpBuilder().getContext(), memberIndexTmp)); + } + } else { + // NOTE: We take the map type of the first child, this may not + // be the correct thing to do, however, we shall see. For the moment + // it allows this to work with enter and exit without causing MLIR + // verification issues. The more appropriate thing may be to take + // the "main" map type clause from the directive being used. + uint64_t mapType = 0; + if (auto mapOp = mlir::dyn_cast<mlir::omp::MapInfoOp>( + memberMaps[idx].getDefiningOp())) + mapType = mapOp.getMapType().value_or(0); + + // create parent to emplace and bind members + auto origSymbol = converter.getSymbolAddress(*sym); + mlir::Value mapOp = createMapInfoOp( + converter.getFirOpBuilder(), + converter.getFirOpBuilder().getUnknownLoc(), origSymbol, + mlir::Value(), sym->name().ToString(), {}, {memberMaps[idx]}, + mlir::ArrayAttr::get(converter.getFirOpBuilder().getContext(), + memberPlacementIndices[idx]), + mapType, mlir::omp::VariableCaptureKind::ByRef, origSymbol.getType(), + true); + + mapOperands.push_back(mapOp); + if (mapSymTypes) + mapSymTypes->push_back(mapOp.getType()); + if (mapSymLocs) + mapSymLocs->push_back(mapOp.getLoc()); + if (mapSymbols) + mapSymbols->push_back(sym); + } + } +} + bool ClauseProcessor::processMap( mlir::Location currentLocation, const llvm::omp::Directive &directive, Fortran::semantics::SemanticsContext &semanticsContext, @@ -1859,7 +1983,13 @@ bool ClauseProcessor::processMap( llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *mapSymbols) const { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); - return findRepeatableClause<ClauseTy::Map>( + + llvm::SmallVector<mlir::Value> memberMaps; + llvm::SmallVector<mlir::Attribute> memberPlacementIndices; + llvm::SmallVector<const Fortran::semantics::Symbol *> memberParentSyms, + mapSyms; + + bool clauseFound = findRepeatableClause<ClauseTy::Map>( [&](const ClauseTy::Map *mapClause, const Fortran::parser::CharBlock &source) { mlir::Location clauseLocation = converter.genLocation(source); @@ -1906,8 +2036,22 @@ bool ClauseProcessor::processMap( for (const Fortran::parser::OmpObject &ompObject : std::get<Fortran::parser::OmpObjectList>(mapClause->v.t).v) { + llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits = mapTypeBits; + checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits, + getOmpObjectSymbol(ompObject)); + llvm::SmallVector<mlir::Value> bounds; std::stringstream asFortran; + const Fortran::semantics::Symbol *parentSym = nullptr; + + if (getOmpObjectSymbol(ompObject)->owner().IsDerivedType()) { + memberPlacementIndices.push_back( + firOpBuilder.getI64IntegerAttr(findComponenetMemberPlacement( + getOmpObjectSymbol(ompObject)->owner().symbol(), + getOmpObjectSymbol(ompObject)))); + parentSym = getOmpObjParentSymbol(ompObject); + memberParentSyms.push_back(parentSym); + } Fortran::lower::AddrAndBoundsInfo info = Fortran::lower::gatherDataOperandAddrAndBounds< @@ -1927,22 +2071,33 @@ bool ClauseProcessor::processMap( // types to optimise mlir::Value mapOp = createMapInfoOp( firOpBuilder, clauseLocation, symAddr, mlir::Value{}, - asFortran.str(), bounds, {}, + asFortran.str(), bounds, {}, mlir::ArrayAttr{}, static_cast< std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>( - mapTypeBits), + objectsMapTypeBits), mlir::omp::VariableCaptureKind::ByRef, symAddr.getType()); - mapOperands.push_back(mapOp); - if (mapSymTypes) - mapSymTypes->push_back(symAddr.getType()); - if (mapSymLocs) - mapSymLocs->push_back(symAddr.getLoc()); - - if (mapSymbols) - mapSymbols->push_back(getOmpObjectSymbol(ompObject)); + if (parentSym) { + memberMaps.push_back(mapOp); + } else { + mapOperands.push_back(mapOp); + mapSyms.push_back(getOmpObjectSymbol(ompObject)); + if (mapSymTypes) + mapSymTypes->push_back(symAddr.getType()); + if (mapSymLocs) + mapSymLocs->push_back(symAddr.getLoc()); + } } }); + + insertChildMapInfoIntoParent(converter, memberParentSyms, memberMaps, + memberPlacementIndices, mapOperands, mapSymTypes, + mapSymLocs, &mapSyms); + + if (mapSymbols) + *mapSymbols = mapSyms; + + return clauseFound; } bool ClauseProcessor::processReduction( @@ -2021,7 +2176,12 @@ bool ClauseProcessor::processMotionClauses( Fortran::semantics::SemanticsContext &semanticsContext, Fortran::lower::StatementContext &stmtCtx, llvm::SmallVectorImpl<mlir::Value> &mapOperands) { - return findRepeatableClause<T>( + llvm::SmallVector<mlir::Value> memberMaps; + llvm::SmallVector<mlir::Attribute> memberPlacementIndices; + llvm::SmallVector<const Fortran::semantics::Symbol *> memberParentSyms, + mapSymbols; + + bool clauseFound = findRepeatableClause<T>( [&](const T *motionClause, const Fortran::parser::CharBlock &source) { mlir::Location clauseLocation = converter.genLocation(source); fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); @@ -2036,8 +2196,23 @@ bool ClauseProcessor::processMotionClauses( : llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM; for (const Fortran::parser::OmpObject &ompObject : motionClause->v.v) { + llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits = mapTypeBits; + checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits, + getOmpObjectSymbol(ompObject)); + llvm::SmallVector<mlir::Value> bounds; std::stringstream asFortran; + const Fortran::semantics::Symbol *parentSym = nullptr; + + if (getOmpObjectSymbol(ompObject)->owner().IsDerivedType()) { + memberPlacementIndices.push_back( + firOpBuilder.getI64IntegerAttr(findComponenetMemberPlacement( + getOmpObjectSymbol(ompObject)->owner().symbol(), + getOmpObjectSymbol(ompObject)))); + parentSym = getOmpObjParentSymbol(ompObject); + memberParentSyms.push_back(parentSym); + } + Fortran::lower::AddrAndBoundsInfo info = Fortran::lower::gatherDataOperandAddrAndBounds< Fortran::parser::OmpObject, mlir::omp::DataBoundsOp, @@ -2056,15 +2231,25 @@ bool ClauseProcessor::processMotionClauses( // types to optimise mlir::Value mapOp = createMapInfoOp( firOpBuilder, clauseLocation, symAddr, mlir::Value{}, - asFortran.str(), bounds, {}, + asFortran.str(), bounds, {}, mlir::ArrayAttr{}, static_cast< std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>( - mapTypeBits), + objectsMapTypeBits), mlir::omp::VariableCaptureKind::ByRef, symAddr.getType()); - mapOperands.push_back(mapOp); + if (parentSym) { + memberMaps.push_back(mapOp); + } else { + mapOperands.push_back(mapOp); + mapSymbols.push_back(getOmpObjectSymbol(ompObject)); + } } }); + + insertChildMapInfoIntoParent(converter, memberParentSyms, memberMaps, + memberPlacementIndices, mapOperands, nullptr, + nullptr, &mapSymbols); + return clauseFound; } template <typename... Ts> @@ -2882,7 +3067,7 @@ static void genBodyOfTargetOp( firOpBuilder.setInsertionPoint(targetOp); mlir::Value mapOp = createMapInfoOp( firOpBuilder, copyVal.getLoc(), copyVal, mlir::Value{}, name.str(), - bounds, llvm::SmallVector<mlir::Value>{}, + bounds, llvm::SmallVector<mlir::Value>{}, mlir::ArrayAttr{}, static_cast< std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>( llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT), @@ -3018,7 +3203,7 @@ genTargetOp(Fortran::lower::AbstractConverter &converter, mlir::Value mapOp = createMapInfoOp( converter.getFirOpBuilder(), baseOp.getLoc(), baseOp, mlir::Value{}, - name.str(), bounds, {}, + name.str(), bounds, {}, mlir::ArrayAttr{}, static_cast< std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>( mapFlag), diff --git a/flang/lib/Optimizer/Transforms/CMakeLists.txt b/flang/lib/Optimizer/Transforms/CMakeLists.txt index ba2e267996150e..ce5ce3ed1bc48d 100644 --- a/flang/lib/Optimizer/Transforms/CMakeLists.txt +++ b/flang/lib/Optimizer/Transforms/CMakeLists.txt @@ -17,7 +17,7 @@ add_flang_library(FIRTransforms AddDebugFoundation.cpp PolymorphicOpConversion.cpp... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/81511 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits