[Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang and OpenMP dialect #81328
Conversation
…ng and OpenMP dialect
This patch 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 OpenMPToLLVMIRTranslation phase
to more generally support record types, building on the prior groundwork in the
Fortran allocatables/pointers patch. It now supports different kinds of record type
mapping, in this case full record type mapping and then explicit member mapping
in which there is a special case for certain types when mapped individually to not
require any parent map link in the kernel argument structure. To facilitate this
required:
> The movement of the setting of the map flag type "ptr_and_obj" to respective
frontends, now supporting it as a possible flag that can be read and printed
in mlir form. Some minor changes to declare target map type setting was
neccesary for this.
> The addition of a member index array operand, which tracks the position
of the member in the parent, required for caclulating the appropriate size
to offload to the target, alongside the parents offload pointer (always the
first member currently being mapped).
> A partial mapping attribute operand, to indicate if the entire record type is
being mapped or just member components, aiding the ability to lower
record types in the different manners that are possible.
> Refactoring bounds calculation for record types and general arrays to one
location (as well as load/store generation prior to assigning to the kernel
argument structure), as a side affect enter/exit/update/data mapping
should now be more correct and fully support bounds mapping, previously
this would have only worked for target.
It also extends 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 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.
Otherwise, this patch contains a large amount of tests, both runtime and
compilation related, which will hopefully help showcase the cases this
patch aims to cover and the IR changes neccesary.
agozillon
requested review from
ergawy,
skatrak,
jsjodin,
kiranchandramohan,
raghavendhra,
DominikAdamski,
dpalermo,
TIFitis,
kiranktp and
kparzysz
llvmbot
added
mlir:llvm
mlir
flang
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-mlir Author: None (agozillon) ChangesThis patch 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: '''' !$omp target map(tofrom: scalar_arr%int, scalar_arr%real) Current cases of derived type mapping left for future work are: This patch seeks to support this by extending the OpenMPToLLVMIRTranslation phase to more generally support record types, building on the prior groundwork in the Fortran allocatables/pointers patch. It now supports different kinds of record type mapping, in this case full record type mapping and then explicit member mapping in which there is a special case for certain types when mapped individually to not require any parent map link in the kernel argument structure. To facilitate this required: It also extends the Flang-new OpenMP lowering to support generation of this newly required information, creating the necessary parent <-to-> member map_info links, calculating the member indices and setting if it's a partial map. The OMPDescriptorMapInfoGen pass has 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 constituent members inserted. Otherwise, this patch contains a large amount of tests, both runtime and compilation related, which will hopefully help showcase the cases this patch aims to cover and the IR changes necessary. Patch is 185.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81328.diff 47 Files Affected:
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]
|
|
@llvm/pr-subscribers-mlir-llvm Author: None (agozillon) ChangesThis patch 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: '''' !$omp target map(tofrom: scalar_arr%int, scalar_arr%real) Current cases of derived type mapping left for future work are: This patch seeks to support this by extending the OpenMPToLLVMIRTranslation phase to more generally support record types, building on the prior groundwork in the Fortran allocatables/pointers patch. It now supports different kinds of record type mapping, in this case full record type mapping and then explicit member mapping in which there is a special case for certain types when mapped individually to not require any parent map link in the kernel argument structure. To facilitate this required: It also extends the Flang-new OpenMP lowering to support generation of this newly required information, creating the necessary parent <-to-> member map_info links, calculating the member indices and setting if it's a partial map. The OMPDescriptorMapInfoGen pass has 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 constituent members inserted. Otherwise, this patch contains a large amount of tests, both runtime and compilation related, which will hopefully help showcase the cases this patch aims to cover and the IR changes necessary. Patch is 185.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81328.diff 47 Files Affected:
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]
|
| !CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {uniq_name = "_QFmaptype_derived_explicit_single_memberEscalar_arr"} : (!fir.ref<!fir.type<_QFmaptype_derived_explicit_single_memberTscalar_and_array{real:f32,array:!fir.array<10xi32>,int:i32}>>) -> (!fir.ref<!fir.type<_QFmaptype_derived_explicit_single_memberTscalar_and_array{real:f32,array:!fir.array<10xi32>,int:i32}>>, !fir.ref<!fir.type<_QFmaptype_derived_explicit_single_memberTscalar_and_array{real:f32,array:!fir.array<10xi32>,int:i32}>>) | ||
| !CHECK: %[[MEMBER:.*]] = hlfir.designate %[[DECLARE]]#0{"array"} shape %{{.*}} : (!fir.ref<!fir.type<_QFmaptype_derived_explicit_single_memberTscalar_and_array{real:f32,array:!fir.array<10xi32>,int:i32}>>, !fir.shape<1>) -> !fir.ref<!fir.array<10xi32>> | ||
| !CHECK: %[[BOUNDS:.*]] = omp.bounds lower_bound(%{{.*}} : index) upper_bound(%{{.*}} : index) extent(%{{.*}} : index) stride(%{{.*}} : index) start_idx(%{{.*}} : index) | ||
| !CHECK: %[[MEMBER_MAP:.*]] = omp.map_info var_ptr(%[[MEMBER]] : !fir.ref<!fir.array<10xi32>>, !fir.array<10xi32>) map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) -> !fir.ref<!fir.array<10xi32>> {name = "scalar_arr%array"} |
There was a problem hiding this comment.
Hey @razvanlupusoru sorry to bother you, but I had a little question relating to the var_ptr/var_ptr_ptr assignment for the member mappings, currently I've opted to use var_ptr in this PR for the offload pointer, which I am aware is likely incorrect and I am more than happy to fix. However, in this case, if I utilise the base address returned by gatherDataOperandAddrAndBounds for the var_ptr_ptr field which I expect is more correct, what would be the appropriate value to store in the var_ptr field? My FIR/HLFIR knowledge is unfortunately rather lackluster, so I was a little unsure what the correct series of operations to get the desired var_ptr field would be. Again, I am sorry for the bother and thank you very much for your help ahead of time!
There was a problem hiding this comment.
gatherDataOperandAddrAndBounds is giving you the pointer the value you want to map so it's likely to be the one you want to used in var_ptr and not var_ptr_ptr
There was a problem hiding this comment.
Thank you very much @clementval! Then I believe the PR may be correct in this aspect in it's current form, if not then I am more than happy to adjust it. I apologies for the insistent questions about this and I very much appreciate the help you and everyone else provides.
There was a problem hiding this comment.
I understand your question and no need to apologies :-) I guess you want to plug this directly to what the OMPIRBuilder is expecting (2 pointers and a size IIRC).
I'll have a deeper look early next week so I can double check we are on the same page.
There was a problem hiding this comment.
There is no pointer attachment (PTR_AND_OBJ) involved, so var_ptr_ptr should be unused. The pointer of var_ptr is the starting address of array, which is a part of the scalar_arr structured scalar. So this map_info looks correct to me.
But, overall, I am not sure why we need to handle this particular member array differently from other members of the derived type: all the members are mapped as part of the structure same as in
It would make sense to have the member map info if pointer attachment was involved, e.g. if array was a member represented with a descriptor.
There was a problem hiding this comment.
Oh, I was looking at the wrong piece of source code (above). I see what you are doing with the member map infos now. Please ignore the second part of my comment.
There was a problem hiding this comment.
Looks right to me as-is :)
There was a problem hiding this comment.
Thank you everybody for the input, I greatly appreciate it. If you see anything odd on a later inspection after I've fragmented the PR into something more byte sized and easy to consume do please mention it!
I understand your question and no need to apologies :-) I guess you want to plug this directly to what the OMPIRBuilder is expecting (2 pointers and a size IIRC).
Yes, that's exactly what I am trying to do! :-) most of the OpenMP map work (at least lately) has been primarily working out how to reasonably pass data down to the OMPIRBuilder through (Flang -> MLIR -> LLVMIR) to fill the kernel argument structure (offload pointer/base pointer/size/map type being the primary fields) as it expects for the various data types. Whilst trying to keep the dialect open to use with the possible future C/C++ dialect and now I suppose intermixing with the GPU offload dialect as there's some push towards that!
I'll have a deeper look early next week so I can double check we are on the same page.
No rush, take your time! And thank you very much for your help and time.
Will do, thought it might be helpful to have the larger PR up as a larger reference for the overall scope/discussion alongside running the in PR tests correctly (individually the PRs fragmented will not pass the tests, unless there's a way to stack PRs as on phabricator, which there might be now)! I'll do so on Monday. Would you prefer the runtime related tests (the end to end tests that fall into the openmp/libomptarget/test/offloading/fortran/ directory) in a seperate PR or in the Flang lowering PR? |
Fine. No hurry.
Separate. |
Thank you, sounds good! I'll aim for four interlinked PRs then: Sorry for the bother! As an aside, does anyone know of a way to stack dependent patches similar to Phabricator for the purposes of passing the in PR CI tests? Just on the off chance someone reading this may know... |
I have not tried this myself, but here are some notes from an internal conversation with @DavidTruby. |
You can also you tools like SPR to manage your branches for staked PR |
|
Thank you @kiranchandramohan and @clementval for the suggestions, I'll have a little look into both these options and see if I can manage a split of the PRs that will allow at least the "top level" PR to pass the CI (and hopefully be nice and simple to manage). |
|
Opened up the patch stack based on this PR using SPR (thank you for the recommendations everyone), the top level PR can be found here: #81511 so hopefully that will pass CI reasonably. I'll close this PR now as the stack supersedes it! Thank you all for your input and help so far. |
This patch 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:
Current cases of derived type mapping left for future work are:
record types where we explicitly map a member from the internal
record type)
of a derived type
(redundant in Fortran due to former case but still legal as far as I am aware)
not fully tested in this iteration)
record types)
This patch seeks to support this by extending the OpenMPToLLVMIRTranslation phase to more generally support record types, building on the prior groundwork in the Fortran allocatables/pointers patch. It now supports different kinds of record type mapping, in this case full record type mapping and then explicit member mapping in which there is a special case for certain types when mapped individually to not require any parent map link in the kernel argument structure. To facilitate this required:
frontends, now supporting it as a possible flag that can be read and printed
in mlir form. Some minor changes to declare target map type setting was
necessary for this.
of the member in the parent, required for calculating the appropriate size
to offload to the target, alongside the parents offload pointer (always the
first member currently being mapped).
being mapped or just member components, aiding the ability to lower
record types in the different manners that are possible.
location (as well as load/store generation prior to assigning to the kernel
argument structure), as a side affect enter/exit/update/data mapping
should now be more correct and fully support bounds mapping, previously
this would have only worked for target.
It also extends the Flang-new OpenMP lowering to support generation of this newly required information, creating the necessary parent <-to-> member map_info links, calculating the member indices and setting if it's a partial map.
The OMPDescriptorMapInfoGen pass has 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 constituent members inserted.
Otherwise, this patch contains a large amount of tests, both runtime and compilation related, which will hopefully help showcase the cases this patch aims to cover and the IR changes necessary.