[llvm-branch-commits] [flang] [Flang][OpenMP] Update flang with changes to the OpenMP dialect (PR #92524)
@@ -1086,8 +1086,9 @@ static void genTargetDataClauses( // ordering. // TODO: Perhaps create a user provideable compiler option that will // re-introduce a hard-error rather than a warning in these cases. - promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr(clauseOps, useDeviceTypes, -useDeviceLocs, useDeviceSyms); + promoteNonCPtrUseDevicePtrArgsToUseDeviceAddr( + clauseOps.useDeviceAddrVars, clauseOps.useDevicePtrVars, useDeviceTypes, + useDeviceLocs, useDeviceSyms); ergawy wrote: Not sure why we need that change? Why not pass `clauseOps` and retrieve both members inside `promoteNonCPtrUseDevicePtrArgsToUseDEviceAddr`? https://github.com/llvm/llvm-project/pull/92524 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Update flang with changes to the OpenMP dialect (PR #92524)
https://github.com/ergawy approved this pull request. https://github.com/llvm/llvm-project/pull/92524 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/96266 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -52,12 +52,22 @@ using DeclareTargetCapturePair =
struct OmpMapMemberIndicesData {
// The indices representing the component members placement in its derived
// type parents hierarchy.
- llvm::SmallVector memberPlacementIndices;
+ llvm::SmallVector> memberPlacementIndices;
// Placement of the member in the member vector.
- mlir::omp::MapInfoOp memberMap;
+ llvm::SmallVector memberMap;
};
+llvm::SmallVector
+generateMemberPlacementIndices(const Object &object,
ergawy wrote:
There is already a version of `generateMemberPlacementIndices` that takes a
`SmallVector` param as an output param. And both versions have exactly the same
logic. Is there a reason for duplication?
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -953,6 +954,22 @@ bool ClauseProcessor::processMap(
if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType()))
symAddr = origSymbol;
+ if (object.sym()->owner().IsDerivedType()) {
+omp::ObjectList objectList = gatherObjects(object, semaCtx);
+parentObj = objectList[0];
ergawy wrote:
```suggestion
assert(!objectList.empty());
parentObj = objectList[0];
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -52,12 +52,22 @@ using DeclareTargetCapturePair =
struct OmpMapMemberIndicesData {
ergawy wrote:
It would be nice to extend the doc for this `struct` with some examples where
each example consists of:
- some Fortran variable the user wants to map,
- and how the `OmpMapMemberIndicesData` looks like for that variable.
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -671,4 +672,51 @@ static inline bool isEqual(const Fortran::lower::SomeExpr
*x,
}
} // end namespace Fortran::lower
+// OpenMP utility functions used in locations outside of the
+// OpenMP lowering.
+namespace Fortran::lower::omp {
+
+[[maybe_unused]] static void fillMemberIndices(
ergawy wrote:
To reduce compilation times of files including this one, can you move the
implementations of these 2 utils to a corresponding `.cpp` file?
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -671,4 +672,51 @@ static inline bool isEqual(const Fortran::lower::SomeExpr
*x,
}
} // end namespace Fortran::lower
+// OpenMP utility functions used in locations outside of the
+// OpenMP lowering.
+namespace Fortran::lower::omp {
+
+[[maybe_unused]] static void fillMemberIndices(
+llvm::SmallVector> &memberPlacementData) {
+ size_t largestIndicesSize =
+ std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
+ [](auto a, auto b) { return a.size() < b.size(); })
+ ->size();
+
+ // DenseElementsAttr expects a rectangular shape for the data, so all
+ // index lists have to be of the same length, this emplaces -1 as filler.
ergawy wrote:
Does the `-1` value has a significance here or is it just a random value?
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -30,17 +30,17 @@ subroutine mapType_array
!$omp end target
end subroutine mapType_array
-!CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [3 x i64] [i64
0, i64 24, i64 4]
-!CHECK: @.offload_maptypes{{.*}} = private unnamed_addr constant [3 x i64]
[i64 32, i64 281474976710657, i64 281474976711187]
+!CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [4 x i64] [i64
0, i64 24, i64 8, i64 4]
ergawy wrote:
The test fails on my machine on that line. Does it do the same for you as well?
In general, it would be nice to have comments before each `CHECK` line to
explain these magic numbers; why is 3 or 4 or whatever.
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
https://github.com/ergawy commented: Thanks for the PR @agozillon. I did a first round of review and a few suggestions. I also have a few questions that might be obvious, so please excuse any newbie-like questions here :D. https://github.com/llvm/llvm-project/pull/96266 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -671,4 +672,51 @@ static inline bool isEqual(const Fortran::lower::SomeExpr
*x,
}
} // end namespace Fortran::lower
+// OpenMP utility functions used in locations outside of the
+// OpenMP lowering.
+namespace Fortran::lower::omp {
+
+[[maybe_unused]] static void fillMemberIndices(
+llvm::SmallVector> &memberPlacementData) {
+ size_t largestIndicesSize =
+ std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
ergawy wrote:
```suggestion
llvm::max_element(memberPlacementData,
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+ v1 = (first1 == last1) ? -1 : *first1;
+ v2 = (first2 == last2) ? -1 : *first2;
+
+ if (!(v1 == v2))
+return false;
+}
+return true;
+ };
+
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+ return true;
+ return false;
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, Fortran::lower::AbstractConverter
&converter,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapMemberIndicesData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ Fortran::lower::AddrAndBoundsInfo parentBaseAddr =
+ Fortran::lower::getDataOperandBaseAddr(
+ converter, firOpBuilder, *objectList[0].sym(), clauseLocation);
+ mlir::Value curValue = parentBaseAddr.addr;
+
+ for (size_t i = 0; i < objectList.size(); ++i) {
+mlir::Type unwrappedTy =
+fir::unwrapSequenceType(fir::unwrapPassByRefType(curValue.getType()));
+if (fir::RecordType recordType =
+mlir::dyn_cast_or_null(unwrappedTy)) {
+ mlir::Value idxConst = firOpBuilder.createIntegerConstant(
+ clauseLocation, firOpBuilder.getIndexType(), indices[i]);
+ mlir::Type memberTy = recordType.getTypeList().at(indices[i]).second;
+ curValue = firOpBuilder.create(
+ clauseLocation, firOpBuilder.getRefType(memberTy), curValue,
+ idxConst);
+
+ if ((i != indices.size() - 1) && fir::isTypeWithDescriptor(memberTy)) {
ergawy wrote:
```suggestion
if ((i == indices.size() - 1) || !fir::isTypeWithDescriptor(memberTy)) {
continue;
}
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
ergawy wrote:
I think this works correctly only if rang 1 is shorter than range 2, right? Is
this a pre-condition for `duplicateMemberMapInfo`?
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -953,6 +954,22 @@ bool ClauseProcessor::processMap(
if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType()))
symAddr = origSymbol;
+ if (object.sym()->owner().IsDerivedType()) {
+omp::ObjectList objectList = gatherObjects(object, semaCtx);
+parentObj = objectList[0];
+parentMemberIndices.insert({parentObj.value(), {}});
+if (Fortran::semantics::IsAllocatableOrObjectPointer(
+object.sym()) ||
+memberHasAllocatableParent(object, semaCtx)) {
ergawy wrote:
```suggestion
if (isMemberOrParentAllocatableOrPointer(object.sym())) {
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -189,85 +295,60 @@ generateMemberPlacementIndices(const Object &object,
indices = llvm::SmallVector{llvm::reverse(indices)};
}
-void addChildIndexAndMapToParent(
-const omp::Object &object,
-std::map> &parentMemberIndices,
-mlir::omp::MapInfoOp &mapOp, semantics::SemanticsContext &semaCtx) {
- std::optional dataRef = ExtractDataRef(object.ref());
- assert(dataRef.has_value() &&
- "DataRef could not be extracted during mapping of derived type "
- "cannot proceed");
- const semantics::Symbol *parentSym = &dataRef->GetFirstSymbol();
- assert(parentSym && "Could not find parent symbol during lower of "
- "a component member in OpenMP map clause");
+void addChildIndexAndMapToParent(const omp::Object &object,
+ OmpMapMemberIndicesData &parentMemberIndices,
+ mlir::omp::MapInfoOp &mapOp,
+ semantics::SemanticsContext &semaCtx) {
llvm::SmallVector indices;
generateMemberPlacementIndices(object, indices, semaCtx);
- parentMemberIndices[parentSym].push_back({indices, mapOp});
+ parentMemberIndices.memberPlacementIndices.push_back(indices);
+ parentMemberIndices.memberMap.push_back(mapOp);
}
-static void calculateShapeAndFillIndices(
-llvm::SmallVectorImpl &shape,
-llvm::SmallVectorImpl &memberPlacementData) {
- shape.push_back(memberPlacementData.size());
- size_t largestIndicesSize =
- std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
- [](auto a, auto b) {
- return a.memberPlacementIndices.size() <
-b.memberPlacementIndices.size();
- })
- ->memberPlacementIndices.size();
- shape.push_back(largestIndicesSize);
-
- // DenseElementsAttr expects a rectangular shape for the data, so all
- // index lists have to be of the same length, this emplaces -1 as filler.
- for (auto &v : memberPlacementData) {
-if (v.memberPlacementIndices.size() < largestIndicesSize) {
- auto *prevEnd = v.memberPlacementIndices.end();
- v.memberPlacementIndices.resize(largestIndicesSize);
- std::fill(prevEnd, v.memberPlacementIndices.end(), -1);
-}
+llvm::SmallVector
+generateMemberPlacementIndices(const Object &object,
+ Fortran::semantics::SemanticsContext &semaCtx) {
+ std::list indices;
+ auto compObj = getComponentObject(object, semaCtx);
+ while (compObj) {
+indices.push_front(getComponentPlacementInParent(compObj->sym()));
+compObj =
+getComponentObject(getBaseObject(compObj.value(), semaCtx), semaCtx);
}
+
+ return llvm::SmallVector{std::begin(indices), std::end(indices)};
}
-static mlir::DenseIntElementsAttr createDenseElementsAttrFromIndices(
-llvm::SmallVectorImpl &memberPlacementData,
-fir::FirOpBuilder &builder) {
- llvm::SmallVector shape;
- calculateShapeAndFillIndices(shape, memberPlacementData);
-
- llvm::SmallVector indicesFlattened =
- std::accumulate(memberPlacementData.begin(), memberPlacementData.end(),
- llvm::SmallVector(),
- [](llvm::SmallVector &x, OmpMapMemberIndicesData y)
{
-x.insert(x.end(), y.memberPlacementIndices.begin(),
- y.memberPlacementIndices.end());
-return x;
- });
-
- return mlir::DenseIntElementsAttr::get(
- mlir::VectorType::get(shape,
-mlir::IntegerType::get(builder.getContext(), 32)),
- indicesFlattened);
+bool memberHasAllocatableParent(const Object &object,
+Fortran::semantics::SemanticsContext &semaCtx)
{
ergawy wrote:
```suggestion
bool isMemberOrParentAllocatableOrPointer(const Object &object,
Fortran::semantics::SemanticsContext &semaCtx) {
if (Fortran::semantics::IsAllocatableOrObjectPoint(object.sym()))
return true;
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+ v1 = (first1 == last1) ? -1 : *first1;
+ v2 = (first2 == last2) ? -1 : *first2;
+
+ if (!(v1 == v2))
+return false;
+}
+return true;
+ };
+
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+ return true;
+ return false;
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, Fortran::lower::AbstractConverter
&converter,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapMemberIndicesData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
ergawy wrote:
```suggestion
assert(objectList.size() == indices.size());
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/96266 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/96266 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+ v1 = (first1 == last1) ? -1 : *first1;
+ v2 = (first2 == last2) ? -1 : *first2;
+
+ if (!(v1 == v2))
+return false;
+}
+return true;
+ };
+
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+ return true;
+ return false;
+}
ergawy wrote:
If my above comments make sense, then I think the following should be enough:
```suggestion
bool isDuplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
llvm::SmallVectorImpl &memberIndices) {
for (auto memberData : parentMembers.memberPlacementIndices)
if (std::equal(memberData.begin(), memberData.end(), memberIndices.begin()))
return true;
return false;
}
```
In any case, I think the function name change is suggested to better describe
the purpose and behavior of the function.
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+ v1 = (first1 == last1) ? -1 : *first1;
+ v2 = (first2 == last2) ? -1 : *first2;
+
+ if (!(v1 == v2))
+return false;
+}
+return true;
+ };
+
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+ return true;
+ return false;
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, Fortran::lower::AbstractConverter
&converter,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapMemberIndicesData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ Fortran::lower::AddrAndBoundsInfo parentBaseAddr =
+ Fortran::lower::getDataOperandBaseAddr(
+ converter, firOpBuilder, *objectList[0].sym(), clauseLocation);
+ mlir::Value curValue = parentBaseAddr.addr;
+
+ for (size_t i = 0; i < objectList.size(); ++i) {
+mlir::Type unwrappedTy =
+fir::unwrapSequenceType(fir::unwrapPassByRefType(curValue.getType()));
+if (fir::RecordType recordType =
+mlir::dyn_cast_or_null(unwrappedTy)) {
ergawy wrote:
If `i < objectList.size() - 1`, is this ever going to be `false`? I mean, won't
we have record types all the way until the last object in the list (and maybe
this is one is a record type and maybe not).
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+ v1 = (first1 == last1) ? -1 : *first1;
ergawy wrote:
The condition is never going to be true AFAICT.
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -51,21 +55,66 @@ class OMPMapInfoFinalizationPass
: public fir::impl::OMPMapInfoFinalizationPassBase<
OMPMapInfoFinalizationPass> {
- void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
- fir::FirOpBuilder &builder,
- mlir::Operation *target) {
-mlir::Location loc = op.getLoc();
-mlir::Value descriptor = op.getVarPtr();
+ /// Small helper class tracking a members parent and its
ergawy wrote:
nit: no need to quantify the size of the helper class 😛.
```suggestion
/// Tracks a member's parent and its
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
return op;
}
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = getBaseObject(obj, semaCtx);
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+ // A variation of std:equal that supports non-equal length index lists for
our
+ // specific use-case, if one is larger than the other, we use -1, the default
+ // filler element in place of the smaller vector, this prevents UB from over
+ // indexing and removes the need for us to do any filling of intermediate
+ // index lists we'll discard.
+ auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+ v1 = (first1 == last1) ? -1 : *first1;
+ v2 = (first2 == last2) ? -1 : *first2;
+
+ if (!(v1 == v2))
+return false;
+}
+return true;
+ };
+
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+ return true;
+ return false;
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, Fortran::lower::AbstractConverter
&converter,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapMemberIndicesData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ Fortran::lower::AddrAndBoundsInfo parentBaseAddr =
+ Fortran::lower::getDataOperandBaseAddr(
+ converter, firOpBuilder, *objectList[0].sym(), clauseLocation);
+ mlir::Value curValue = parentBaseAddr.addr;
+
+ for (size_t i = 0; i < objectList.size(); ++i) {
+mlir::Type unwrappedTy =
+fir::unwrapSequenceType(fir::unwrapPassByRefType(curValue.getType()));
+if (fir::RecordType recordType =
+mlir::dyn_cast_or_null(unwrappedTy)) {
+ mlir::Value idxConst = firOpBuilder.createIntegerConstant(
+ clauseLocation, firOpBuilder.getIndexType(), indices[i]);
+ mlir::Type memberTy = recordType.getTypeList().at(indices[i]).second;
+ curValue = firOpBuilder.create(
+ clauseLocation, firOpBuilder.getRefType(memberTy), curValue,
+ idxConst);
+
+ if ((i != indices.size() - 1) && fir::isTypeWithDescriptor(memberTy)) {
+llvm::SmallVector intermIndices = indices;
+std::fill(std::next(intermIndices.begin(), i + 1), intermIndices.end(),
+ -1);
+if (!duplicateMemberMapInfo(parentMemberIndices, intermIndices)) {
+ // TODO: Perhaps generate bounds for these intermediate maps, as it
+ // may be required for cases such as:
+ //dtype(1)%second(3)%array
+ // where second is an allocatable (and dtype may be an allocatable as
+ // well, although in this case I am not sure the fortran syntax would
+ // be legal)
+ mlir::omp::MapInfoOp mapOp = createMapInfoOp(
+ firOpBuilder, clauseLocation, curValue,
+ /*varPtrPtr=*/mlir::Value{}, asFortran,
+ /*bounds=*/llvm::SmallVector{},
+ /*members=*/{},
+ /*membersIndex=*/mlir::DenseIntElementsAttr{},
+ static_cast<
+
std::underlying_type_t>(
+ mapTypeBits),
+ mlir::omp::VariableCaptureKind::ByRef, curValue.getType());
+
+ parentMemberIndices.memberPlacementIndices.push_back(intermIndices);
+ parentMemberIndices.memberMap.push_back(mapOp);
+}
+
+if (i != indices.size() - 1)
ergawy wrote:
We already know this is true from the enclosing `if`.
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commi
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -85,67 +135,227 @@ class OMPMapInfoFinalizationPass
descriptor = alloca;
}
+return descriptor;
+ }
+
+ /// Simple function that will generate a FIR operation accessing
+ /// the descriptors base address (BoxOffsetOp) and then generate a
+ /// MapInfoOp for it, the most important thing to note is that
+ /// we normally move the bounds from the descriptor map onto the
+ /// base address map.
+ mlir::omp::MapInfoOp getBaseAddrMap(mlir::Value descriptor,
+ mlir::OperandRange bounds,
+ int64_t mapType,
+ fir::FirOpBuilder &builder) {
+mlir::Location loc = descriptor.getLoc();
mlir::Value baseAddrAddr = builder.create(
loc, descriptor, fir::BoxFieldAttr::base_addr);
// Member of the descriptor pointing at the allocated data
-mlir::Value baseAddr = builder.create(
+return builder.create(
loc, baseAddrAddr.getType(), descriptor,
mlir::TypeAttr::get(llvm::cast(
fir::unwrapRefType(baseAddrAddr.getType()))
.getElementType()),
-baseAddrAddr, /*members=*/mlir::SmallVector{},
-/*member_index=*/mlir::DenseIntElementsAttr{}, op.getBounds(),
-builder.getIntegerAttr(builder.getIntegerType(64, false),
- op.getMapType().value()),
+baseAddrAddr, mlir::SmallVector{},
+mlir::DenseIntElementsAttr{}, bounds,
+builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(
mlir::omp::VariableCaptureKind::ByRef),
-/*name=*/builder.getStringAttr(""),
-/*partial_map=*/builder.getBoolAttr(false));
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+ }
-// TODO: map the addendum segment of the descriptor, similarly to the
-// above base address/data pointer member.
+ /// This function adjusts the member indices vector to include a new
+ /// base address member, we take the position of the descriptor in
+ /// the member indices list, which is the index data that the base
+ /// addresses index will be based off of, as the base address is
+ /// a member of the descriptor, we must also alter other members
+ /// indices in the list to account for this new addition. This
+ /// requires extending all members with -1's if the addition of
+ /// the new base address has increased the member vector past the
+ /// original size, as we must make sure all member indices are of
+ /// the same length (think rectangle matrix) due to DenseIntElementsAttr
+ /// requiring this. We also need to be aware that we are inserting
+ /// into the middle of a member index vector in some cases (i.e.
+ /// we could be accessing the member of a descriptor type with a
+ /// subsequent map, so we must be sure to adjust any of these cases
+ /// with the addition of the new base address index value).
+ void adjustMemberIndices(
+ llvm::SmallVector> &memberIndices,
+ size_t memberIndex) {
+// Find if the descriptor member we are basing our new base address index
+// off of has a -1 somewhere, indicating an empty index already exists (due
+// to a larger sized member position elsewhere) which allows us to simplify
+// later steps a little
+auto baseAddrIndex = memberIndices[memberIndex];
+auto *iterPos = std::find(baseAddrIndex.begin(), baseAddrIndex.end(), -1);
-if (auto mapClauseOwner =
-llvm::dyn_cast(target)) {
- llvm::SmallVector newMapOps;
- mlir::OperandRange mapOperandsArr = mapClauseOwner.getMapOperands();
+// If we aren't at the end, as we found a -1, we can simply modify the -1
+// to the base addresses index in the descriptor (which will always be the
+// first member in the descriptor, so 0). If not, then we're extending the
+// index list and have to push on a 0 and adjust the position to the new
+// end.
+if (iterPos != baseAddrIndex.end()) {
+ *iterPos = 0;
+} else {
+ baseAddrIndex.push_back(0);
+ iterPos = baseAddrIndex.end();
+}
- for (size_t i = 0; i < mapOperandsArr.size(); ++i) {
-if (mapOperandsArr[i] == op) {
- // Push new implicit maps generated for the descriptor.
- newMapOps.push_back(baseAddr);
+auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
ergawy wrote:
Dejavu! Can you create a shared util of this lambda in `Utils.cpp`. The logic
here is a bit involved and having a shared well-documented util would be nice.
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -85,67 +135,227 @@ class OMPMapInfoFinalizationPass
descriptor = alloca;
}
+return descriptor;
+ }
+
+ /// Simple function that will generate a FIR operation accessing
+ /// the descriptors base address (BoxOffsetOp) and then generate a
+ /// MapInfoOp for it, the most important thing to note is that
+ /// we normally move the bounds from the descriptor map onto the
+ /// base address map.
+ mlir::omp::MapInfoOp getBaseAddrMap(mlir::Value descriptor,
+ mlir::OperandRange bounds,
+ int64_t mapType,
+ fir::FirOpBuilder &builder) {
+mlir::Location loc = descriptor.getLoc();
mlir::Value baseAddrAddr = builder.create(
loc, descriptor, fir::BoxFieldAttr::base_addr);
// Member of the descriptor pointing at the allocated data
-mlir::Value baseAddr = builder.create(
+return builder.create(
loc, baseAddrAddr.getType(), descriptor,
mlir::TypeAttr::get(llvm::cast(
fir::unwrapRefType(baseAddrAddr.getType()))
.getElementType()),
-baseAddrAddr, /*members=*/mlir::SmallVector{},
-/*member_index=*/mlir::DenseIntElementsAttr{}, op.getBounds(),
-builder.getIntegerAttr(builder.getIntegerType(64, false),
- op.getMapType().value()),
+baseAddrAddr, mlir::SmallVector{},
+mlir::DenseIntElementsAttr{}, bounds,
+builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(
mlir::omp::VariableCaptureKind::ByRef),
-/*name=*/builder.getStringAttr(""),
-/*partial_map=*/builder.getBoolAttr(false));
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+ }
-// TODO: map the addendum segment of the descriptor, similarly to the
-// above base address/data pointer member.
+ /// This function adjusts the member indices vector to include a new
+ /// base address member, we take the position of the descriptor in
+ /// the member indices list, which is the index data that the base
+ /// addresses index will be based off of, as the base address is
+ /// a member of the descriptor, we must also alter other members
+ /// indices in the list to account for this new addition. This
+ /// requires extending all members with -1's if the addition of
+ /// the new base address has increased the member vector past the
+ /// original size, as we must make sure all member indices are of
+ /// the same length (think rectangle matrix) due to DenseIntElementsAttr
+ /// requiring this. We also need to be aware that we are inserting
+ /// into the middle of a member index vector in some cases (i.e.
+ /// we could be accessing the member of a descriptor type with a
+ /// subsequent map, so we must be sure to adjust any of these cases
+ /// with the addition of the new base address index value).
+ void adjustMemberIndices(
+ llvm::SmallVector> &memberIndices,
+ size_t memberIndex) {
+// Find if the descriptor member we are basing our new base address index
+// off of has a -1 somewhere, indicating an empty index already exists (due
+// to a larger sized member position elsewhere) which allows us to simplify
+// later steps a little
+auto baseAddrIndex = memberIndices[memberIndex];
+auto *iterPos = std::find(baseAddrIndex.begin(), baseAddrIndex.end(), -1);
-if (auto mapClauseOwner =
-llvm::dyn_cast(target)) {
- llvm::SmallVector newMapOps;
- mlir::OperandRange mapOperandsArr = mapClauseOwner.getMapOperands();
+// If we aren't at the end, as we found a -1, we can simply modify the -1
+// to the base addresses index in the descriptor (which will always be the
+// first member in the descriptor, so 0). If not, then we're extending the
+// index list and have to push on a 0 and adjust the position to the new
+// end.
+if (iterPos != baseAddrIndex.end()) {
+ *iterPos = 0;
+} else {
+ baseAddrIndex.push_back(0);
+ iterPos = baseAddrIndex.end();
+}
- for (size_t i = 0; i < mapOperandsArr.size(); ++i) {
-if (mapOperandsArr[i] == op) {
- // Push new implicit maps generated for the descriptor.
- newMapOps.push_back(baseAddr);
+auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+ int v1, v2;
+ for (; first1 != last1; ++first1, ++first2) {
+v1 = (first1 == last1) ? -1 : *first1;
+v2 = (first2 == last2) ? -1 : *first2;
- // for TargetOp's which have IsolatedFromAbove we must align the
- // new additional map operand with an appropriate BlockArgument,
- // as the printing and later processing currently requires a 1:1
- // mapping of BlockArgs to MapInfoOp's at the same placement i
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -85,67 +135,227 @@ class OMPMapInfoFinalizationPass
descriptor = alloca;
}
+return descriptor;
+ }
+
+ /// Simple function that will generate a FIR operation accessing
+ /// the descriptors base address (BoxOffsetOp) and then generate a
+ /// MapInfoOp for it, the most important thing to note is that
+ /// we normally move the bounds from the descriptor map onto the
+ /// base address map.
+ mlir::omp::MapInfoOp getBaseAddrMap(mlir::Value descriptor,
+ mlir::OperandRange bounds,
+ int64_t mapType,
+ fir::FirOpBuilder &builder) {
+mlir::Location loc = descriptor.getLoc();
mlir::Value baseAddrAddr = builder.create(
loc, descriptor, fir::BoxFieldAttr::base_addr);
// Member of the descriptor pointing at the allocated data
-mlir::Value baseAddr = builder.create(
+return builder.create(
loc, baseAddrAddr.getType(), descriptor,
mlir::TypeAttr::get(llvm::cast(
fir::unwrapRefType(baseAddrAddr.getType()))
.getElementType()),
-baseAddrAddr, /*members=*/mlir::SmallVector{},
-/*member_index=*/mlir::DenseIntElementsAttr{}, op.getBounds(),
-builder.getIntegerAttr(builder.getIntegerType(64, false),
- op.getMapType().value()),
+baseAddrAddr, mlir::SmallVector{},
+mlir::DenseIntElementsAttr{}, bounds,
+builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(
mlir::omp::VariableCaptureKind::ByRef),
-/*name=*/builder.getStringAttr(""),
-/*partial_map=*/builder.getBoolAttr(false));
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+ }
-// TODO: map the addendum segment of the descriptor, similarly to the
-// above base address/data pointer member.
+ /// This function adjusts the member indices vector to include a new
+ /// base address member, we take the position of the descriptor in
+ /// the member indices list, which is the index data that the base
+ /// addresses index will be based off of, as the base address is
+ /// a member of the descriptor, we must also alter other members
+ /// indices in the list to account for this new addition. This
+ /// requires extending all members with -1's if the addition of
+ /// the new base address has increased the member vector past the
+ /// original size, as we must make sure all member indices are of
+ /// the same length (think rectangle matrix) due to DenseIntElementsAttr
+ /// requiring this. We also need to be aware that we are inserting
+ /// into the middle of a member index vector in some cases (i.e.
+ /// we could be accessing the member of a descriptor type with a
+ /// subsequent map, so we must be sure to adjust any of these cases
+ /// with the addition of the new base address index value).
+ void adjustMemberIndices(
+ llvm::SmallVector> &memberIndices,
+ size_t memberIndex) {
+// Find if the descriptor member we are basing our new base address index
+// off of has a -1 somewhere, indicating an empty index already exists (due
+// to a larger sized member position elsewhere) which allows us to simplify
+// later steps a little
+auto baseAddrIndex = memberIndices[memberIndex];
+auto *iterPos = std::find(baseAddrIndex.begin(), baseAddrIndex.end(), -1);
-if (auto mapClauseOwner =
-llvm::dyn_cast(target)) {
- llvm::SmallVector newMapOps;
- mlir::OperandRange mapOperandsArr = mapClauseOwner.getMapOperands();
+// If we aren't at the end, as we found a -1, we can simply modify the -1
+// to the base addresses index in the descriptor (which will always be the
+// first member in the descriptor, so 0). If not, then we're extending the
+// index list and have to push on a 0 and adjust the position to the new
+// end.
+if (iterPos != baseAddrIndex.end()) {
+ *iterPos = 0;
+} else {
+ baseAddrIndex.push_back(0);
+ iterPos = baseAddrIndex.end();
+}
- for (size_t i = 0; i < mapOperandsArr.size(); ++i) {
-if (mapOperandsArr[i] == op) {
- // Push new implicit maps generated for the descriptor.
- newMapOps.push_back(baseAddr);
+auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+ int v1, v2;
+ for (; first1 != last1; ++first1, ++first2) {
+v1 = (first1 == last1) ? -1 : *first1;
+v2 = (first2 == last2) ? -1 : *first2;
- // for TargetOp's which have IsolatedFromAbove we must align the
- // new additional map operand with an appropriate BlockArgument,
- // as the printing and later processing currently requires a 1:1
- // mapping of BlockArgs to MapInfoOp's at the same placement i
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -219,6 +430,33 @@ class OMPMapInfoFinalizationPass mapClauseOwner.getMapOperandsMutable().assign(newMapOps); ergawy wrote: Not part of the PR (sorry for the extra comments), but I have a feeling something is not correct here. Doesn't this line overwrite all map operands which means if we have map clauses from multiple structures/types we will end up with the map ops from the last one and its members, right? Shouldn't we instead append to a shared vector and assign the whole vector after we walk all map info ops (line 5.4)? Apologies if I missed something. https://github.com/llvm/llvm-project/pull/96266 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)
@@ -85,67 +135,227 @@ class OMPMapInfoFinalizationPass
descriptor = alloca;
}
+return descriptor;
+ }
+
+ /// Simple function that will generate a FIR operation accessing
+ /// the descriptors base address (BoxOffsetOp) and then generate a
+ /// MapInfoOp for it, the most important thing to note is that
+ /// we normally move the bounds from the descriptor map onto the
+ /// base address map.
+ mlir::omp::MapInfoOp getBaseAddrMap(mlir::Value descriptor,
+ mlir::OperandRange bounds,
+ int64_t mapType,
+ fir::FirOpBuilder &builder) {
+mlir::Location loc = descriptor.getLoc();
mlir::Value baseAddrAddr = builder.create(
loc, descriptor, fir::BoxFieldAttr::base_addr);
// Member of the descriptor pointing at the allocated data
-mlir::Value baseAddr = builder.create(
+return builder.create(
loc, baseAddrAddr.getType(), descriptor,
mlir::TypeAttr::get(llvm::cast(
fir::unwrapRefType(baseAddrAddr.getType()))
.getElementType()),
-baseAddrAddr, /*members=*/mlir::SmallVector{},
-/*member_index=*/mlir::DenseIntElementsAttr{}, op.getBounds(),
-builder.getIntegerAttr(builder.getIntegerType(64, false),
- op.getMapType().value()),
+baseAddrAddr, mlir::SmallVector{},
+mlir::DenseIntElementsAttr{}, bounds,
+builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(
mlir::omp::VariableCaptureKind::ByRef),
-/*name=*/builder.getStringAttr(""),
-/*partial_map=*/builder.getBoolAttr(false));
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+ }
-// TODO: map the addendum segment of the descriptor, similarly to the
-// above base address/data pointer member.
+ /// This function adjusts the member indices vector to include a new
+ /// base address member, we take the position of the descriptor in
+ /// the member indices list, which is the index data that the base
+ /// addresses index will be based off of, as the base address is
+ /// a member of the descriptor, we must also alter other members
+ /// indices in the list to account for this new addition. This
+ /// requires extending all members with -1's if the addition of
+ /// the new base address has increased the member vector past the
+ /// original size, as we must make sure all member indices are of
+ /// the same length (think rectangle matrix) due to DenseIntElementsAttr
+ /// requiring this. We also need to be aware that we are inserting
+ /// into the middle of a member index vector in some cases (i.e.
+ /// we could be accessing the member of a descriptor type with a
+ /// subsequent map, so we must be sure to adjust any of these cases
+ /// with the addition of the new base address index value).
+ void adjustMemberIndices(
+ llvm::SmallVector> &memberIndices,
+ size_t memberIndex) {
+// Find if the descriptor member we are basing our new base address index
+// off of has a -1 somewhere, indicating an empty index already exists (due
+// to a larger sized member position elsewhere) which allows us to simplify
+// later steps a little
+auto baseAddrIndex = memberIndices[memberIndex];
ergawy wrote:
Pronounce types when possible for readability.
```suggestion
llvm::SmallVector baseAddrIndex = memberIndices[memberIndex];
```
https://github.com/llvm/llvm-project/pull/96266
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
https://github.com/ergawy edited 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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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{
ergawy wrote:
Any reason not to use: `if (const auto *derived = )` instead?
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -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(
-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)) {
ergawy wrote:
This branch is dead now, right? I will never execute AFAICT.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+for (auto t : derived->componentNames()) {
ergawy wrote:
I think this logic looks like a good candidate to be a method inside
`DerivedTypeDetails`, WDYT?
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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{
ergawy wrote:
The [LLVM style
guide](https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code)
suggests to use early exits when possibe. Can we invert the condition and exit
with `-1` in the `if` and then execute the main logic after we close the `if`?
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+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(op)) {
ergawy wrote:
Can we use `auto declareTargetOp =
SymbolTable::lookupNearestSymbolFrom(converter.getModuleOp(),
converter.mangleName(*symbol));` instead?
It will collapse all the 3 lines and, I think, properly encapsulate what we are
doing here.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
https://github.com/ergawy commented: Partially reviewed, will continue later. Thanks Andrew, I am learning quite a bit from this PR. 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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -72,6 +74,29 @@ getOmpObjectSymbol(const Fortran::parser::OmpObject
&ompObject) {
return sym;
}
+static Fortran::semantics::Symbol *
+getOmpObjectSymbol(const Fortran::parser::OmpObject &ompObject) {
ergawy wrote:
Instead of having this and the above functions, can we have one function with a
`bool getParentObjWhenApplicable` argument?
I am suggesting this because almost all the logic is repeated with the
exception of the `StructureComponent` case, right?
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+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(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 &memberParentSyms,
+llvm::SmallVector &memberMaps,
ergawy wrote:
Is there a reason to use the more general `mlir::Value` rather than
`omp::MapInfoOp`? I think all elements of `memberMaps` are always instances of
`MapInfoOp`, right?
It can be argued that we don't need full access to the op's data but my
suggestion is to provide more "documentation" in the code by using as much
specific types as possible.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+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(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 &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *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(
+ mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector 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(
ergawy wrote:
Same question here, is it valid to not be a `MapInfoOp` or a violation of what
should be expected by the function all the time?
Apologies if I missed something.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1906,8 +2036,22 @@ bool ClauseProcessor::processMap(
for (const Fortran::parser::OmpObject &ompObject :
std::get(mapClause->v.t).v) {
+ llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits =
mapTypeBits;
+ checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits,
+ getOmpObjectSymbol(ompObject));
+
llvm::SmallVector 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(),
ergawy wrote:
Maybe a dummy question, but in which cases would
`getOmpObjectSymbol(ompObject)->owner().symbol()` and
`getOmpObjParentSymbol(ompObject)` would return different symbols? Can you give
me an example for such a case?
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+ OMPMapInfoFinalizationPass> {
+
+ void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+ fir::FirOpBuilder &builder,
+ mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+ if (auto addrOp = mlir::dyn_cast_if_present(
+ op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+ }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+ mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+ builder.setInsertionPointToStart(builder.getAllocaBlock());
+ auto alloca = builder.create(loc, descriptor.getType());
+ builder.restoreInsertionPoint(insPt);
+ builder.create(loc, descriptor, alloca);
+ descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1859,7 +1983,13 @@ bool ClauseProcessor::processMap(
llvm::SmallVectorImpl *mapSymbols)
const {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- return findRepeatableClause(
+
+ llvm::SmallVector memberMaps;
+ llvm::SmallVector memberPlacementIndices;
+ llvm::SmallVector memberParentSyms,
+ mapSyms;
ergawy wrote:
Do we need `mapSyms`? I think it is only populated and then conditionally
assigned to `mapSymbols` if `mapSymbols != nullptr`. We can directly use
`mapSymbols` I think.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+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(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 &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *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(
+ mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+ }
ergawy wrote:
What happens if the parent is not a `mapOp`, is it a valid case? Should we
`assert` to ensure that no pre-condtions are broken?
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+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(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 &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *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(
+ mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector 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(
+ 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,
ergawy wrote:
I think we can provide more precise location info.
```suggestion
origSymbol.getLoc(), origSymbol,
```
Might help with troubleshooting issues, I think.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+ OMPMapInfoFinalizationPass> {
+
+ void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+ fir::FirOpBuilder &builder,
+ mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+ if (auto addrOp = mlir::dyn_cast_if_present(
+ op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+ }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+ mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+ builder.setInsertionPointToStart(builder.getAllocaBlock());
+ auto alloca = builder.create(loc, descriptor.getType());
+ builder.restoreInsertionPoint(insPt);
+ builder.create(loc, descriptor, alloca);
+ descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
llvm::cast(retTy).getElementType());
mlir::omp::MapInfoOp op = builder.create(
- loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+ loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(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()}) {
+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(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 &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *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;
ergawy wrote:
```suggestion
parentIdx = i;
break;
```
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -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(
-designator)) {
+if (auto *structComp = Fortran::parser::Unwrap<
+Fortran::parser::StructureComponent>(designator)) {
+ sym = GetFirstName(structComp->base).symbol;
ergawy wrote:
`const Name &GetFirstName(const StructureComponent &x)` does this for you.
```suggestion
sym = GetFirstName(structComp).symbol;
```
Just to not repeat the logic if it is already provided somewhere else.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+ OMPMapInfoFinalizationPass> {
+
+ void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+ fir::FirOpBuilder &builder,
+ mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+ if (auto addrOp = mlir::dyn_cast_if_present(
+ op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+ }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+ mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+ builder.setInsertionPointToStart(builder.getAllocaBlock());
+ auto alloca = builder.create(loc, descriptor.getType());
+ builder.restoreInsertionPoint(insPt);
+ builder.create(loc, descriptor, alloca);
+ descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
https://github.com/ergawy edited 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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+ OMPMapInfoFinalizationPass> {
+
+ void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+ fir::FirOpBuilder &builder,
+ mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+ if (auto addrOp = mlir::dyn_cast_if_present(
+ op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+ }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+ mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+ builder.setInsertionPointToStart(builder.getAllocaBlock());
+ auto alloca = builder.create(loc, descriptor.getType());
+ builder.restoreInsertionPoint(insPt);
+ builder.create(loc, descriptor, alloca);
+ descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
https://github.com/ergawy commented:
I reviewed the entire PR and commented on all what I thought worth mentioning.
But I will leave the approval to people working on this longer than I am.
Hopefully my review makes other reviewers go over the PR faster.
One comment about the lit tests in the PR in general: I see in quite a few
places, there are `CHECK` lines that use the SSA names directly (e.g. `%20`). I
think this is fragile and it would be better to either capture the name of the
SSA value if we care about it and want to check it later (i.e. using
`%[[name:.*]]`) or capture and discard the name of the value in a generic way
(i.e. using `%{{.*}}`).
Thanks Andrew for answering my questions.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1859,7 +1983,13 @@ bool ClauseProcessor::processMap(
llvm::SmallVectorImpl *mapSymbols)
const {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- return findRepeatableClause(
+
+ llvm::SmallVector memberMaps;
+ llvm::SmallVector memberPlacementIndices;
+ llvm::SmallVector memberParentSyms,
+ mapSyms;
ergawy wrote:
Right, makes sense to keep it then since it is always passed to
`insertChildMapInfoIntoParent`.
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
[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)
@@ -1906,8 +2036,22 @@ bool ClauseProcessor::processMap(
for (const Fortran::parser::OmpObject &ompObject :
std::get(mapClause->v.t).v) {
+ llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits =
mapTypeBits;
+ checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits,
+ getOmpObjectSymbol(ompObject));
+
llvm::SmallVector 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(),
ergawy wrote:
💡 Aha, I see now. Thanks for clarifying this!
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
[llvm-branch-commits] [OpenMP][MLIR] Extend explicit derived type member mapping support for OpenMP dialects lowering to LLVM-IR (PR #81510)
@@ -1783,6 +1783,98 @@ void collectMapDataFromMapOperands(MapInfoData &mapData,
}
}
+static int getMapDataMemberIdx(MapInfoData &mapData,
+ mlir::omp::MapInfoOp memberOp) {
+ int memberDataIdx = -1;
+ for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
+if (mapData.MapClause[i] == memberOp)
+ memberDataIdx = i;
+ }
+ return memberDataIdx;
+}
+
+static mlir::omp::MapInfoOp
+getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
+ // Only 1 member has been mapped, we can return it.
+ if (mapInfo.getMembersIndex()->size() == 1)
+if (auto mapOp = mlir::dyn_cast(
+mapInfo.getMembers()[0].getDefiningOp()))
+ return mapOp;
+
+ int64_t curPos =
+ mapInfo.getMembersIndex()->begin()->cast().getInt();
+
+ int64_t idx = 1, curIdx = 0, memberPlacement = 0;
+ for (const auto *iter = std::next(mapInfo.getMembersIndex()->begin());
+ iter != mapInfo.getMembersIndex()->end(); iter++) {
+memberPlacement = iter->cast().getInt();
+if (first) {
+ if (memberPlacement < curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+} else {
+ if (memberPlacement > curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+}
+idx++;
+ }
+
+ if (auto mapOp = mlir::dyn_cast(
+ mapInfo.getMembers()[curIdx].getDefiningOp()))
+return mapOp;
+
+ return {};
ergawy wrote:
Apologies for repeating this, but I think it is better to `assert` if some
assumption in the code is violated rather than returning an empty value.
https://github.com/llvm/llvm-project/pull/81510
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [OpenMP][MLIR] Extend explicit derived type member mapping support for OpenMP dialects lowering to LLVM-IR (PR #81510)
@@ -1783,6 +1783,98 @@ void collectMapDataFromMapOperands(MapInfoData &mapData,
}
}
+static int getMapDataMemberIdx(MapInfoData &mapData,
+ mlir::omp::MapInfoOp memberOp) {
+ int memberDataIdx = -1;
+ for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
+if (mapData.MapClause[i] == memberOp)
+ memberDataIdx = i;
+ }
+ return memberDataIdx;
+}
+
+static mlir::omp::MapInfoOp
+getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
+ // Only 1 member has been mapped, we can return it.
+ if (mapInfo.getMembersIndex()->size() == 1)
+if (auto mapOp = mlir::dyn_cast(
+mapInfo.getMembers()[0].getDefiningOp()))
+ return mapOp;
+
+ int64_t curPos =
+ mapInfo.getMembersIndex()->begin()->cast().getInt();
+
+ int64_t idx = 1, curIdx = 0, memberPlacement = 0;
+ for (const auto *iter = std::next(mapInfo.getMembersIndex()->begin());
+ iter != mapInfo.getMembersIndex()->end(); iter++) {
+memberPlacement = iter->cast().getInt();
+if (first) {
+ if (memberPlacement < curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+} else {
+ if (memberPlacement > curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+}
+idx++;
+ }
+
+ if (auto mapOp = mlir::dyn_cast(
+ mapInfo.getMembers()[curIdx].getDefiningOp()))
+return mapOp;
+
+ return {};
+}
+
+std::vector
+calculateBoundsOffset(LLVM::ModuleTranslation &moduleTranslation,
+ llvm::IRBuilderBase &builder, bool isArrayTy,
+ mlir::OperandRange bounds) {
+ std::vector idx;
+ llvm::Value *offsetAddress = nullptr;
+ if (!bounds.empty()) {
ergawy wrote:
```suggestion
if (bounds.empty())
return idx;
```
https://github.com/llvm/llvm-project/pull/81510
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [OpenMP][MLIR] Extend explicit derived type member mapping support for OpenMP dialects lowering to LLVM-IR (PR #81510)
@@ -1783,6 +1783,98 @@ void collectMapDataFromMapOperands(MapInfoData &mapData,
}
}
+static int getMapDataMemberIdx(MapInfoData &mapData,
+ mlir::omp::MapInfoOp memberOp) {
+ int memberDataIdx = -1;
+ for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
+if (mapData.MapClause[i] == memberOp)
+ memberDataIdx = i;
+ }
+ return memberDataIdx;
+}
ergawy wrote:
`std::find_if(...)`, just like the other PR 😛.
Also it seems like all the uses of this function assume that `-1` will not be
returned (there has to be an element matching the search key). So, I would
suggest:
```suggestion
static int getMapDataMemberIdx(MapInfoData &mapData,
mlir::omp::MapInfoOp memberOp) {
auto res = llvm::find(mapData.MapClause, memberOp);
assert(res != mapData.MapClause.end());
return std::distance(mapData.MapClause.begin(), res);
}
```
https://github.com/llvm/llvm-project/pull/81510
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [OpenMP][MLIR] Extend explicit derived type member mapping support for OpenMP dialects lowering to LLVM-IR (PR #81510)
@@ -1783,6 +1783,98 @@ void collectMapDataFromMapOperands(MapInfoData &mapData,
}
}
+static int getMapDataMemberIdx(MapInfoData &mapData,
+ mlir::omp::MapInfoOp memberOp) {
+ int memberDataIdx = -1;
+ for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
+if (mapData.MapClause[i] == memberOp)
+ memberDataIdx = i;
+ }
+ return memberDataIdx;
+}
+
+static mlir::omp::MapInfoOp
+getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
+ // Only 1 member has been mapped, we can return it.
+ if (mapInfo.getMembersIndex()->size() == 1)
+if (auto mapOp = mlir::dyn_cast(
+mapInfo.getMembers()[0].getDefiningOp()))
+ return mapOp;
+
+ int64_t curPos =
+ mapInfo.getMembersIndex()->begin()->cast().getInt();
+
+ int64_t idx = 1, curIdx = 0, memberPlacement = 0;
+ for (const auto *iter = std::next(mapInfo.getMembersIndex()->begin());
+ iter != mapInfo.getMembersIndex()->end(); iter++) {
+memberPlacement = iter->cast().getInt();
+if (first) {
+ if (memberPlacement < curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+} else {
+ if (memberPlacement > curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+}
+idx++;
+ }
ergawy wrote:
Can we instead sort `getMembersIndex()` somehow and fetch either the head or
the tail based on `first`? Not sure how easy it is but my feeling is that it
should be possible and would be easier to understand.
Or guarantee that it is always sorted on op creation/update. But maybe this is
not worth it.
https://github.com/llvm/llvm-project/pull/81510
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [OpenMP][MLIR] Extend explicit derived type member mapping support for OpenMP dialects lowering to LLVM-IR (PR #81510)
@@ -1783,6 +1783,98 @@ void collectMapDataFromMapOperands(MapInfoData &mapData,
}
}
+static int getMapDataMemberIdx(MapInfoData &mapData,
+ mlir::omp::MapInfoOp memberOp) {
+ int memberDataIdx = -1;
+ for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
+if (mapData.MapClause[i] == memberOp)
+ memberDataIdx = i;
+ }
+ return memberDataIdx;
+}
+
+static mlir::omp::MapInfoOp
+getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
+ // Only 1 member has been mapped, we can return it.
+ if (mapInfo.getMembersIndex()->size() == 1)
+if (auto mapOp = mlir::dyn_cast(
+mapInfo.getMembers()[0].getDefiningOp()))
+ return mapOp;
+
+ int64_t curPos =
+ mapInfo.getMembersIndex()->begin()->cast().getInt();
+
+ int64_t idx = 1, curIdx = 0, memberPlacement = 0;
+ for (const auto *iter = std::next(mapInfo.getMembersIndex()->begin());
+ iter != mapInfo.getMembersIndex()->end(); iter++) {
+memberPlacement = iter->cast().getInt();
+if (first) {
+ if (memberPlacement < curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+} else {
+ if (memberPlacement > curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+}
+idx++;
+ }
+
+ if (auto mapOp = mlir::dyn_cast(
+ mapInfo.getMembers()[curIdx].getDefiningOp()))
+return mapOp;
+
+ return {};
+}
+
+std::vector
+calculateBoundsOffset(LLVM::ModuleTranslation &moduleTranslation,
+ llvm::IRBuilderBase &builder, bool isArrayTy,
+ mlir::OperandRange bounds) {
+ std::vector idx;
+ llvm::Value *offsetAddress = nullptr;
+ if (!bounds.empty()) {
+idx.push_back(builder.getInt64(0));
+if (isArrayTy) {
+ for (int i = bounds.size() - 1; i >= 0; --i) {
+if (auto boundOp = mlir::dyn_cast_if_present(
+bounds[i].getDefiningOp())) {
+
idx.push_back(moduleTranslation.lookupValue(boundOp.getLowerBound()));
+}
+ }
+} else {
+ std::vector dimensionIndexSizeOffset{builder.getInt64(1)};
+ for (size_t i = 1; i < bounds.size(); ++i) {
+if (auto boundOp = mlir::dyn_cast_if_present(
+bounds[i].getDefiningOp())) {
+ dimensionIndexSizeOffset.push_back(builder.CreateMul(
+ moduleTranslation.lookupValue(boundOp.getExtent()),
+ dimensionIndexSizeOffset[i - 1]));
+}
+ }
+
+ for (int i = bounds.size() - 1; i >= 0; --i) {
+if (auto boundOp = mlir::dyn_cast_if_present(
+bounds[i].getDefiningOp())) {
+ if (!offsetAddress)
+offsetAddress = builder.CreateMul(
+moduleTranslation.lookupValue(boundOp.getLowerBound()),
+dimensionIndexSizeOffset[i]);
+ else
+offsetAddress = builder.CreateAdd(
+offsetAddress, builder.CreateMul(moduleTranslation.lookupValue(
+ boundOp.getLowerBound()),
+ dimensionIndexSizeOffset[i]));
ergawy wrote:
I think you can get rid of `offsetAddress` this way:
```suggestion
if (idx.empty())
idx.emplace_back(builder.CreateMul(
moduleTranslation.lookupValue(boundOp.getLowerBound()),
dimensionIndexSizeOffset[i]));
else
offsetAddress.back() = builder.CreateAdd(
offsetAddress.back(),
builder.CreateMul(moduleTranslation.lookupValue(
boundOp.getLowerBound()),
dimensionIndexSizeOffset[i]));
```
https://github.com/llvm/llvm-project/pull/81510
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [OpenMP][MLIR] Extend explicit derived type member mapping support for OpenMP dialects lowering to LLVM-IR (PR #81510)
@@ -1783,6 +1783,98 @@ void collectMapDataFromMapOperands(MapInfoData &mapData,
}
}
+static int getMapDataMemberIdx(MapInfoData &mapData,
+ mlir::omp::MapInfoOp memberOp) {
+ int memberDataIdx = -1;
+ for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
+if (mapData.MapClause[i] == memberOp)
+ memberDataIdx = i;
+ }
+ return memberDataIdx;
+}
+
+static mlir::omp::MapInfoOp
+getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
+ // Only 1 member has been mapped, we can return it.
+ if (mapInfo.getMembersIndex()->size() == 1)
+if (auto mapOp = mlir::dyn_cast(
+mapInfo.getMembers()[0].getDefiningOp()))
+ return mapOp;
+
+ int64_t curPos =
+ mapInfo.getMembersIndex()->begin()->cast().getInt();
+
+ int64_t idx = 1, curIdx = 0, memberPlacement = 0;
+ for (const auto *iter = std::next(mapInfo.getMembersIndex()->begin());
+ iter != mapInfo.getMembersIndex()->end(); iter++) {
+memberPlacement = iter->cast().getInt();
+if (first) {
+ if (memberPlacement < curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+} else {
+ if (memberPlacement > curPos) {
+curIdx = idx;
+curPos = memberPlacement;
+ }
+}
+idx++;
+ }
+
+ if (auto mapOp = mlir::dyn_cast(
+ mapInfo.getMembers()[curIdx].getDefiningOp()))
+return mapOp;
+
+ return {};
+}
+
+std::vector
+calculateBoundsOffset(LLVM::ModuleTranslation &moduleTranslation,
ergawy wrote:
There is quite some complicated logic in this function, can you add some
documentation on the function to provide a high-level description? And in the
function itself, can you add a few more comments explaining things like why you
are iterating backwards in some cases?
https://github.com/llvm/llvm-project/pull/81510
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Don't pass clauses to op-generating functions anymore (PR #90108)
https://github.com/ergawy approved this pull request. https://github.com/llvm/llvm-project/pull/90108 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -49,38 +51,95 @@ using DeclareTargetCapturePair =
// and index data when lowering OpenMP map clauses. Keeps track of the
// placement of the component in the derived type hierarchy it rests within,
// alongside the generated mlir::omp::MapInfoOp for the mapped component.
-struct OmpMapMemberIndicesData {
+//
+// As an example of what the contents of this data structure may be like,
+// when provided the following derived type and map of that type:
+//
+// type :: bottom_layer
+// real(8) :: i2
+// real(4) :: array_i2(10)
+// real(4) :: array_j2(10)
+// end type bottom_layer
+//
+// type :: top_layer
+// real(4) :: i
+// integer(4) :: array_i(10)
+// real(4) :: j
+// type(bottom_layer) :: nested
+// integer, allocatable :: array_j(:)
+// integer(4) :: k
+// end type top_layer
+//
+// type(top_layer) :: top_dtype
+//
+// map(tofrom: top_dtype%nested%i2, top_dtype%k, top_dtype%nested%array_i2)
+//
+// We would end up with an OmpMapParentAndMemberData populated like below:
+//
+// memberPlacementIndices:
+// Vector 1: 3, 0
+// Vector 2: 5
+// Vector 3: 3, 1
+//
+// memberMap:
+// Entry 1: omp.map.info for "top_dtype%nested%i2"
+// Entry 2: omp.map.info for "top_dtype%k"
+// Entry 3: omp.map.info for "top_dtype%nested%array_i2"
+//
+// And this OmpMapParentAndMemberData would be accessed via the parent
+// symbol for top_dtype. Other parent derived type instances that have
+// members mapped would have there own OmpMapParentAndMemberData entry
+// accessed via their own symbol.
+struct OmpMapParentAndMemberData {
ergawy wrote:
Could this be something like this:
```suggestion
struct OmpMapParentAndMemberData {
struct MemberMappingData {
llvm::SmallVector placementIndices;
mlir::omp::MapInfo memberMap;
};
llvm::SmallVector allMembersData;
};
```
The reason I think this is better is that it ties all the mapping data for a
single member in one structure which makes easier to understand what
`OmpMapParentAndMemberData` encapsulates.
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -145,11 +146,174 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(mapCaptureType),
builder.getStringAttr(name), builder.getBoolAttr(partialMap));
-
return op;
}
-static int
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = obj;
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool isDuplicateMemberMapInfo(OmpMapParentAndMemberData &parentMembers,
ergawy wrote:
Can this be a method in `OmpMapParentAndMemberData`?
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -183,99 +347,69 @@ getComponentObject(std::optional object,
return getComponentObject(baseObj.value(), semaCtx);
}
-static void
-generateMemberPlacementIndices(const Object &object,
- llvm::SmallVectorImpl &indices,
- semantics::SemanticsContext &semaCtx) {
+void generateMemberPlacementIndices(const Object &object,
+llvm::SmallVectorImpl &indices,
+semantics::SemanticsContext &semaCtx) {
+ indices.clear();
auto compObj = getComponentObject(object, semaCtx);
+
while (compObj) {
-indices.push_back(getComponentPlacementInParent(compObj->sym()));
+int64_t index = getComponentPlacementInParent(compObj->sym());
+assert(index >= 0);
+indices.push_back(index);
compObj =
getComponentObject(getBaseObject(compObj.value(), semaCtx), semaCtx);
}
- indices = llvm::SmallVector{llvm::reverse(indices)};
+ indices = llvm::SmallVector{llvm::reverse(indices)};
}
-void addChildIndexAndMapToParent(
-const omp::Object &object,
-std::map> &parentMemberIndices,
-mlir::omp::MapInfoOp &mapOp, semantics::SemanticsContext &semaCtx) {
- std::optional dataRef = ExtractDataRef(object.ref());
- assert(dataRef.has_value() &&
- "DataRef could not be extracted during mapping of derived type "
- "cannot proceed");
- const semantics::Symbol *parentSym = &dataRef->GetFirstSymbol();
- assert(parentSym && "Could not find parent symbol during lower of "
- "a component member in OpenMP map clause");
- llvm::SmallVector indices;
+void addChildIndexAndMapToParent(const omp::Object &object,
+ OmpMapParentAndMemberData
&parentMemberIndices,
+ mlir::omp::MapInfoOp &mapOp,
+ semantics::SemanticsContext &semaCtx) {
+ llvm::SmallVector indices;
generateMemberPlacementIndices(object, indices, semaCtx);
- parentMemberIndices[parentSym].push_back({indices, mapOp});
+ parentMemberIndices.memberPlacementIndices.push_back(indices);
+ parentMemberIndices.memberMap.push_back(mapOp);
}
-static void calculateShapeAndFillIndices(
-llvm::SmallVectorImpl &shape,
-llvm::SmallVectorImpl &memberPlacementData) {
- shape.push_back(memberPlacementData.size());
- size_t largestIndicesSize =
- std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
- [](auto a, auto b) {
- return a.memberPlacementIndices.size() <
-b.memberPlacementIndices.size();
- })
- ->memberPlacementIndices.size();
- shape.push_back(largestIndicesSize);
-
- // DenseElementsAttr expects a rectangular shape for the data, so all
- // index lists have to be of the same length, this emplaces -1 as filler.
- for (auto &v : memberPlacementData) {
-if (v.memberPlacementIndices.size() < largestIndicesSize) {
- auto *prevEnd = v.memberPlacementIndices.end();
- v.memberPlacementIndices.resize(largestIndicesSize);
- std::fill(prevEnd, v.memberPlacementIndices.end(), -1);
-}
+bool isMemberOrParentAllocatableOrPointer(
+const Object &object, semantics::SemanticsContext &semaCtx) {
+ if (semantics::IsAllocatableOrObjectPointer(object.sym()))
+return true;
+
+ auto compObj = getBaseObject(object, semaCtx);
+ while (compObj) {
+if (compObj.has_value() &&
+semantics::IsAllocatableOrObjectPointer(compObj.value().sym()))
ergawy wrote:
```suggestion
if (semantics::IsAllocatableOrObjectPointer(compObj.value().sym()))
```
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
https://github.com/ergawy commented: Reviewed `Utils.h/.cpp`, submitting the current state of the review to prevent double-reviews when possible. https://github.com/llvm/llvm-project/pull/92 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -145,11 +146,174 @@ createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc, builder.getIntegerAttr(builder.getIntegerType(64, false), mapType), builder.getAttr(mapCaptureType), builder.getStringAttr(name), builder.getBoolAttr(partialMap)); - return op; } -static int +omp::ObjectList gatherObjects(omp::Object obj, ergawy wrote: ```suggestion omp::ObjectList gatherParentObjectsOf(omp::Object derivedTypeMember, ``` https://github.com/llvm/llvm-project/pull/92 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -145,11 +146,174 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(mapCaptureType),
builder.getStringAttr(name), builder.getBoolAttr(partialMap));
-
return op;
}
-static int
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = obj;
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool isDuplicateMemberMapInfo(OmpMapParentAndMemberData &parentMembers,
+ llvm::SmallVectorImpl &memberIndices) {
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (std::equal(memberIndices.begin(), memberIndices.end(),
+ memberData.begin()))
+ return true;
+ return false;
+}
+
+static void generateArrayIndices(lower::AbstractConverter &converter,
ergawy wrote:
Please add docs with an example.
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -49,38 +51,95 @@ using DeclareTargetCapturePair =
// and index data when lowering OpenMP map clauses. Keeps track of the
// placement of the component in the derived type hierarchy it rests within,
// alongside the generated mlir::omp::MapInfoOp for the mapped component.
-struct OmpMapMemberIndicesData {
+//
+// As an example of what the contents of this data structure may be like,
+// when provided the following derived type and map of that type:
+//
+// type :: bottom_layer
+// real(8) :: i2
+// real(4) :: array_i2(10)
+// real(4) :: array_j2(10)
+// end type bottom_layer
+//
+// type :: top_layer
+// real(4) :: i
+// integer(4) :: array_i(10)
+// real(4) :: j
+// type(bottom_layer) :: nested
+// integer, allocatable :: array_j(:)
+// integer(4) :: k
+// end type top_layer
+//
+// type(top_layer) :: top_dtype
+//
+// map(tofrom: top_dtype%nested%i2, top_dtype%k, top_dtype%nested%array_i2)
+//
+// We would end up with an OmpMapParentAndMemberData populated like below:
+//
+// memberPlacementIndices:
+// Vector 1: 3, 0
+// Vector 2: 5
+// Vector 3: 3, 1
+//
+// memberMap:
+// Entry 1: omp.map.info for "top_dtype%nested%i2"
+// Entry 2: omp.map.info for "top_dtype%k"
+// Entry 3: omp.map.info for "top_dtype%nested%array_i2"
+//
+// And this OmpMapParentAndMemberData would be accessed via the parent
+// symbol for top_dtype. Other parent derived type instances that have
+// members mapped would have there own OmpMapParentAndMemberData entry
+// accessed via their own symbol.
+struct OmpMapParentAndMemberData {
// The indices representing the component members placement in its derived
// type parents hierarchy.
- llvm::SmallVector memberPlacementIndices;
+ llvm::SmallVector> memberPlacementIndices;
// Placement of the member in the member vector.
- mlir::omp::MapInfoOp memberMap;
+ llvm::SmallVector memberMap;
};
-mlir::omp::MapInfoOp
-createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
-mlir::Value baseAddr, mlir::Value varPtrPtr, std::string name,
-mlir::ArrayRef bounds,
-mlir::ArrayRef members,
-mlir::DenseIntElementsAttr membersIndex, uint64_t mapType,
-mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type
retTy,
-bool partialMap = false);
-
-void addChildIndexAndMapToParent(
-const omp::Object &object,
-std::map> &parentMemberIndices,
-mlir::omp::MapInfoOp &mapOp, semantics::SemanticsContext &semaCtx);
+mlir::omp::MapInfoOp createMapInfoOp(
+fir::FirOpBuilder &builder, mlir::Location loc, mlir::Value baseAddr,
+mlir::Value varPtrPtr, std::string name, mlir::ArrayRef
bounds,
+mlir::ArrayRef members, mlir::ArrayAttr membersIndex,
+uint64_t mapType, mlir::omp::VariableCaptureKind mapCaptureType,
+mlir::Type retTy, bool partialMap = false);
+
+void addChildIndexAndMapToParent(const omp::Object &object,
ergawy wrote:
Can we have this as a member method inside `OmpMapParentAndMemberData`?
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -183,99 +347,69 @@ getComponentObject(std::optional object,
return getComponentObject(baseObj.value(), semaCtx);
}
-static void
-generateMemberPlacementIndices(const Object &object,
- llvm::SmallVectorImpl &indices,
- semantics::SemanticsContext &semaCtx) {
+void generateMemberPlacementIndices(const Object &object,
+llvm::SmallVectorImpl &indices,
+semantics::SemanticsContext &semaCtx) {
+ indices.clear();
ergawy wrote:
I think it would be better to assert the list is emtpy here.
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -0,0 +1,161 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+!CHECK: %[[ALLOCA:.*]] = fir.alloca
!fir.type<_QFtest_derived_type_allocatable_map_operand_and_block_additionTone_layer{i:f32,scalar:!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>
{bindc_name = "one_l", uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"}
+!CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
+!CHECK: %[[BOUNDS:.*]] = omp.map.bounds lower_bound({{.*}})
upper_bound({{.*}}) extent({{.*}}) stride({{.*}}) start_idx({{.*}})
{stride_in_bytes = true}
+!CHECK: %[[MEMBER_INDEX:.*]] = arith.constant 4 : index
+!CHECK: %[[MEMBER_COORD:.*]] = fir.coordinate_of %[[DECLARE]]#0,
%[[MEMBER_INDEX]] :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
index) -> !fir.ref>>>
ergawy wrote:
@agozillon Here is what I meant, nothing fancy, just making sure the test it
more readable by reusing short names for complicated types.
```suggestion
!CHECK: %[[ALLOCA:.*]] = fir.alloca
!fir.type<[[ONE_LAYER_TY:_QFtest_derived_type_allocatable_map_operand_and_block_additionTone_layer{i:f32,scalar:!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}]]>
{bindc_name = "one_l", uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"}
!CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"} :
(!fir.ref>) ->
(!fir.ref>, !fir.ref>)
!CHECK: %[[BOUNDS:.*]] = omp.map.bounds lower_bound({{.*}}) upper_bound({{.*}})
extent({{.*}}) stride({{.*}}) start_idx({{.*}}) {stride_in_bytes = true}
!CHECK: %[[MEMBER_INDEX:.*]] = arith.constant 4 : index
!CHECK: %[[MEMBER_COORD:.*]] = fir.coordinate_of %[[DECLARE]]#0,
%[[MEMBER_INDEX]] : (!fir.ref>, index) ->
!fir.ref>>>
```
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -183,99 +347,69 @@ getComponentObject(std::optional object,
return getComponentObject(baseObj.value(), semaCtx);
}
-static void
-generateMemberPlacementIndices(const Object &object,
- llvm::SmallVectorImpl &indices,
- semantics::SemanticsContext &semaCtx) {
+void generateMemberPlacementIndices(const Object &object,
+llvm::SmallVectorImpl &indices,
+semantics::SemanticsContext &semaCtx) {
+ indices.clear();
auto compObj = getComponentObject(object, semaCtx);
+
while (compObj) {
-indices.push_back(getComponentPlacementInParent(compObj->sym()));
+int64_t index = getComponentPlacementInParent(compObj->sym());
+assert(index >= 0);
+indices.push_back(index);
compObj =
getComponentObject(getBaseObject(compObj.value(), semaCtx), semaCtx);
}
- indices = llvm::SmallVector{llvm::reverse(indices)};
+ indices = llvm::SmallVector{llvm::reverse(indices)};
}
-void addChildIndexAndMapToParent(
-const omp::Object &object,
-std::map> &parentMemberIndices,
-mlir::omp::MapInfoOp &mapOp, semantics::SemanticsContext &semaCtx) {
- std::optional dataRef = ExtractDataRef(object.ref());
- assert(dataRef.has_value() &&
- "DataRef could not be extracted during mapping of derived type "
- "cannot proceed");
- const semantics::Symbol *parentSym = &dataRef->GetFirstSymbol();
- assert(parentSym && "Could not find parent symbol during lower of "
- "a component member in OpenMP map clause");
- llvm::SmallVector indices;
+void addChildIndexAndMapToParent(const omp::Object &object,
+ OmpMapParentAndMemberData
&parentMemberIndices,
+ mlir::omp::MapInfoOp &mapOp,
+ semantics::SemanticsContext &semaCtx) {
+ llvm::SmallVector indices;
generateMemberPlacementIndices(object, indices, semaCtx);
- parentMemberIndices[parentSym].push_back({indices, mapOp});
+ parentMemberIndices.memberPlacementIndices.push_back(indices);
+ parentMemberIndices.memberMap.push_back(mapOp);
}
-static void calculateShapeAndFillIndices(
-llvm::SmallVectorImpl &shape,
-llvm::SmallVectorImpl &memberPlacementData) {
- shape.push_back(memberPlacementData.size());
- size_t largestIndicesSize =
- std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
- [](auto a, auto b) {
- return a.memberPlacementIndices.size() <
-b.memberPlacementIndices.size();
- })
- ->memberPlacementIndices.size();
- shape.push_back(largestIndicesSize);
-
- // DenseElementsAttr expects a rectangular shape for the data, so all
- // index lists have to be of the same length, this emplaces -1 as filler.
- for (auto &v : memberPlacementData) {
-if (v.memberPlacementIndices.size() < largestIndicesSize) {
- auto *prevEnd = v.memberPlacementIndices.end();
- v.memberPlacementIndices.resize(largestIndicesSize);
- std::fill(prevEnd, v.memberPlacementIndices.end(), -1);
-}
+bool isMemberOrParentAllocatableOrPointer(
+const Object &object, semantics::SemanticsContext &semaCtx) {
+ if (semantics::IsAllocatableOrObjectPointer(object.sym()))
+return true;
+
+ auto compObj = getBaseObject(object, semaCtx);
+ while (compObj) {
+if (compObj.has_value() &&
+semantics::IsAllocatableOrObjectPointer(compObj.value().sym()))
+ return true;
+compObj = getBaseObject(compObj.value(), semaCtx);
}
-}
-static mlir::DenseIntElementsAttr createDenseElementsAttrFromIndices(
-llvm::SmallVectorImpl &memberPlacementData,
-fir::FirOpBuilder &builder) {
- llvm::SmallVector shape;
- calculateShapeAndFillIndices(shape, memberPlacementData);
-
- llvm::SmallVector indicesFlattened =
- std::accumulate(memberPlacementData.begin(), memberPlacementData.end(),
- llvm::SmallVector(),
- [](llvm::SmallVector &x, OmpMapMemberIndicesData y)
{
-x.insert(x.end(), y.memberPlacementIndices.begin(),
- y.memberPlacementIndices.end());
-return x;
- });
-
- return mlir::DenseIntElementsAttr::get(
- mlir::VectorType::get(shape,
-mlir::IntegerType::get(builder.getContext(), 32)),
- indicesFlattened);
+ return false;
}
void insertChildMapInfoIntoParent(
-lower::AbstractConverter &converter,
-std::map> &parentMemberIndices,
+lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx,
+lower::StatementContext &stmtCtx,
+std::map &parentMemberIndices,
llvm::SmallVectorImpl &mapOperands,
-llvm::SmallVectorImpl &mapSyms,
llvm::SmallVectorImpl *mapSymTypes,
-ll
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -183,99 +347,69 @@ getComponentObject(std::optional object,
return getComponentObject(baseObj.value(), semaCtx);
}
-static void
-generateMemberPlacementIndices(const Object &object,
- llvm::SmallVectorImpl &indices,
- semantics::SemanticsContext &semaCtx) {
+void generateMemberPlacementIndices(const Object &object,
+llvm::SmallVectorImpl &indices,
ergawy wrote:
Just curious why the change from `int` to `int64_t` in particular?
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/92 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -145,11 +146,174 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(mapCaptureType),
builder.getStringAttr(name), builder.getBoolAttr(partialMap));
-
return op;
}
-static int
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = obj;
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool isDuplicateMemberMapInfo(OmpMapParentAndMemberData &parentMembers,
+ llvm::SmallVectorImpl &memberIndices) {
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (std::equal(memberIndices.begin(), memberIndices.end(),
+ memberData.begin()))
+ return true;
+ return false;
+}
+
+static void generateArrayIndices(lower::AbstractConverter &converter,
+ fir::FirOpBuilder &firOpBuilder,
+ lower::StatementContext &stmtCtx,
+ mlir::Location clauseLocation,
+ llvm::SmallVectorImpl &indices,
+ omp::Object object) {
+ if (auto maybeRef = evaluate::ExtractDataRef(*object.ref())) {
+evaluate::DataRef ref = *maybeRef;
+if (auto *arr = std::get_if(&ref.u)) {
+ for (auto v : arr->subscript()) {
+if (std::holds_alternative(v.u)) {
+ llvm_unreachable("Triplet indexing in map clause is unsupported");
+} else {
+ auto expr =
+ std::get(v.u);
+ mlir::Value subscript = fir::getBase(
+ converter.genExprValue(toEvExpr(expr.value()), stmtCtx));
+ mlir::Value one = firOpBuilder.createIntegerConstant(
+ clauseLocation, firOpBuilder.getIndexType(), 1);
+ subscript = firOpBuilder.createConvert(
+ clauseLocation, firOpBuilder.getIndexType(), subscript);
+ indices.push_back(firOpBuilder.create(
+ clauseLocation, subscript, one));
+}
+ }
+}
+ }
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
ergawy wrote:
It is difficult for me to understand what this function does. Can you please
document the paramemters (the ones specific to the function, no need to
document `converter` and other usual parameters.
Can you also add some pseudo-MLIR guiding examples to the comments to help
clarify the behavior of the function?
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -145,11 +146,174 @@ createMapInfoOp(fir::FirOpBuilder &builder,
mlir::Location loc,
builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(mapCaptureType),
builder.getStringAttr(name), builder.getBoolAttr(partialMap));
-
return op;
}
-static int
+omp::ObjectList gatherObjects(omp::Object obj,
+ semantics::SemanticsContext &semaCtx) {
+ omp::ObjectList objList;
+ std::optional baseObj = obj;
+ while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+ }
+ return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool isDuplicateMemberMapInfo(OmpMapParentAndMemberData &parentMembers,
+ llvm::SmallVectorImpl &memberIndices) {
+ for (auto memberData : parentMembers.memberPlacementIndices)
+if (std::equal(memberIndices.begin(), memberIndices.end(),
+ memberData.begin()))
+ return true;
+ return false;
+}
+
+static void generateArrayIndices(lower::AbstractConverter &converter,
+ fir::FirOpBuilder &firOpBuilder,
+ lower::StatementContext &stmtCtx,
+ mlir::Location clauseLocation,
+ llvm::SmallVectorImpl &indices,
+ omp::Object object) {
+ if (auto maybeRef = evaluate::ExtractDataRef(*object.ref())) {
+evaluate::DataRef ref = *maybeRef;
+if (auto *arr = std::get_if(&ref.u)) {
+ for (auto v : arr->subscript()) {
+if (std::holds_alternative(v.u)) {
+ llvm_unreachable("Triplet indexing in map clause is unsupported");
+} else {
+ auto expr =
+ std::get(v.u);
+ mlir::Value subscript = fir::getBase(
+ converter.genExprValue(toEvExpr(expr.value()), stmtCtx));
+ mlir::Value one = firOpBuilder.createIntegerConstant(
+ clauseLocation, firOpBuilder.getIndexType(), 1);
+ subscript = firOpBuilder.createConvert(
+ clauseLocation, firOpBuilder.getIndexType(), subscript);
+ indices.push_back(firOpBuilder.create(
+ clauseLocation, subscript, one));
+}
+ }
+}
+ }
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, lower::AbstractConverter &converter,
+semantics::SemanticsContext &semaCtx, lower::StatementContext &stmtCtx,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapParentAndMemberData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+
+ auto arrayExprWithSubscript = [](omp::Object obj) {
+if (auto maybeRef = evaluate::ExtractDataRef(*obj.ref())) {
+ evaluate::DataRef ref = *maybeRef;
+ if (auto *arr = std::get_if(&ref.u))
+return !arr->subscript().empty();
+}
+return false;
+ };
+
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ lower::AddrAndBoundsInfo parentBaseAddr = lower::getDataOperandBaseAddr(
+ converter, firOpBuilder, *objectList[0].sym(), clauseLocation);
+ mlir::Value curValue = parentBaseAddr.addr;
+
+ // Iterate over all objects in the objectList, this should consist of all
+ // record types between the parent and the member being mapped (including
+ // the parent). The object list may also contain array objects as well,
+ // this can occur when specifying bounds or a specific element access
+ // within a member map, we skip these.
+ size_t currentIndex = 0;
+ for (size_t i = 0; i < objectList.size(); ++i) {
ergawy wrote:
We can use a range-based loop here.
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -82,104 +132,188 @@ class MapInfoFinalizationPass
// perform an alloca and then store to it and retrieve the data from the
new
// alloca.
if (mlir::isa(descriptor.getType())) {
- // If we have already created a local allocation for this BoxType,
- // we must be sure to re-use it so that we end up with the same
- // allocations being utilised for the same descriptor across all map
uses,
- // this prevents runtime issues such as not appropriately releasing or
- // deleting all mapped data.
- auto find = localBoxAllocas.find(descriptor.getAsOpaquePointer());
- if (find != localBoxAllocas.end()) {
-builder.create(loc, descriptor, find->second);
-descriptor = find->second;
- } else {
-mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
-mlir::Block *allocaBlock = builder.getAllocaBlock();
-assert(allocaBlock && "No alloca block found for this top level op");
-builder.setInsertionPointToStart(allocaBlock);
-auto alloca = builder.create(loc, descriptor.getType());
-builder.restoreInsertionPoint(insPt);
-builder.create(loc, descriptor, alloca);
-localBoxAllocas[descriptor.getAsOpaquePointer()] = alloca;
-descriptor = alloca;
- }
+ mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+ mlir::Block *allocaBlock = builder.getAllocaBlock();
+ mlir::Location loc = boxMap->getLoc();
+ assert(allocaBlock && "No alloca block found for this top level op");
+ builder.setInsertionPointToStart(allocaBlock);
+ auto alloca = builder.create(loc, descriptor.getType());
+ builder.restoreInsertionPoint(insPt);
+ builder.create(loc, descriptor, alloca);
+ descriptor = alloca;
}
+return descriptor;
+ }
+
+ /// Function that generates a FIR operation accessing the descriptor's
+ /// base address (BoxOffsetOp) and a MapInfoOp for it. The most
+ /// important thing to note is that we normally move the bounds from
+ /// the descriptor map onto the base address map.
+ mlir::omp::MapInfoOp getBaseAddrMap(mlir::Value descriptor,
+ mlir::OperandRange bounds,
+ int64_t mapType,
+ fir::FirOpBuilder &builder) {
+mlir::Location loc = descriptor.getLoc();
mlir::Value baseAddrAddr = builder.create(
loc, descriptor, fir::BoxFieldAttr::base_addr);
// Member of the descriptor pointing at the allocated data
-mlir::Value baseAddr = builder.create(
+return builder.create(
loc, baseAddrAddr.getType(), descriptor,
mlir::TypeAttr::get(llvm::cast(
fir::unwrapRefType(baseAddrAddr.getType()))
.getElementType()),
baseAddrAddr, /*members=*/mlir::SmallVector{},
-/*member_index=*/mlir::DenseIntElementsAttr{}, op.getBounds(),
-builder.getIntegerAttr(builder.getIntegerType(64, false),
- op.getMapType().value()),
+/*membersIndex=*/mlir::ArrayAttr{}, bounds,
+builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
builder.getAttr(
mlir::omp::VariableCaptureKind::ByRef),
/*name=*/builder.getStringAttr(""),
/*partial_map=*/builder.getBoolAttr(false));
+ }
-// TODO: map the addendum segment of the descriptor, similarly to the
-// above base address/data pointer member.
+ /// This function adjusts the member indices vector to include a new
+ /// base address member. We take the position of the descriptor in
+ /// the member indices list, which is the index data that the base
+ /// addresses index will be based off of, as the base address is
+ /// a member of the descriptor. We must also alter other member's
ergawy wrote:
I think the comment was cut-off.
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -58,21 +67,62 @@ class MapInfoFinalizationPass
/*corresponding local alloca=*/fir::AllocaOp>
localBoxAllocas;
- void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
- fir::FirOpBuilder &builder,
- mlir::Operation *target) {
-mlir::Location loc = op.getLoc();
-mlir::Value descriptor = op.getVarPtr();
+ /// getMemberUserList gathers all users of a particular MapInfoOp that are
+ /// other MapInfoOp's and places them into the mapMemberUsers list, which
+ /// records the map that the current argument MapInfoOp "op" is part of
+ /// alongside the placement of "op" in the recorded users members list. The
+ /// intent of the generated list is to find all MapInfoOp's that may be
+ /// considered parents of the passed in "op" and in which it shows up in the
+ /// member list, alongside collecting the placement information of "op" in
its
+ /// parents member list.
+ void
+ getMemberUserList(mlir::omp::MapInfoOp op,
+llvm::SmallVectorImpl &mapMemberUsers)
{
+for (auto *users : op->getUsers())
ergawy wrote:
nit
```suggestion
for (auto *user : op->getUsers())
```
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -58,21 +67,62 @@ class MapInfoFinalizationPass
/*corresponding local alloca=*/fir::AllocaOp>
localBoxAllocas;
- void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
- fir::FirOpBuilder &builder,
- mlir::Operation *target) {
-mlir::Location loc = op.getLoc();
-mlir::Value descriptor = op.getVarPtr();
+ /// getMemberUserList gathers all users of a particular MapInfoOp that are
+ /// other MapInfoOp's and places them into the mapMemberUsers list, which
+ /// records the map that the current argument MapInfoOp "op" is part of
+ /// alongside the placement of "op" in the recorded users members list. The
+ /// intent of the generated list is to find all MapInfoOp's that may be
+ /// considered parents of the passed in "op" and in which it shows up in the
+ /// member list, alongside collecting the placement information of "op" in
its
+ /// parents member list.
+ void
+ getMemberUserList(mlir::omp::MapInfoOp op,
+llvm::SmallVectorImpl &mapMemberUsers)
{
ergawy wrote:
In what scenario will `mapMemberUsers` end up with more than one element?
As a test (just to learn more about the changes), I modified this function as
follows and none of the tests failed:
```diff
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -81,8 +81,10 @@ class MapInfoFinalizationPass
for (auto *users : op->getUsers())
if (auto map = mlir::dyn_cast_if_present(users))
for (auto [i, mapMember] : llvm::enumerate(map.getMembers()))
- if (mapMember.getDefiningOp() == op)
+ if (mapMember.getDefiningOp() == op) {
mapMemberUsers.push_back({map, i});
+break;
+ }
}
llvm::SmallVector
```
Also, `mapMemberUsers[0]` is the only element used below (in
`genDescriptorMemberMaps`).
https://github.com/llvm/llvm-project/pull/92
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
https://github.com/ergawy commented: First round, just reading through some tests to make sure I understand the effect of the changes. Will continue later. Thanks Andrew! https://github.com/llvm/llvm-project/pull/92 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/92 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -0,0 +1,161 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+!CHECK: %[[ALLOCA:.*]] = fir.alloca
!fir.type<_QFtest_derived_type_allocatable_map_operand_and_block_additionTone_layer{i:f32,scalar:!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>
{bindc_name = "one_l", uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"}
+!CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
+!CHECK: %[[BOUNDS:.*]] = omp.map.bounds lower_bound({{.*}})
upper_bound({{.*}}) extent({{.*}}) stride({{.*}}) start_idx({{.*}})
{stride_in_bytes = true}
+!CHECK: %[[MEMBER_INDEX:.*]] = arith.constant 4 : index
+!CHECK: %[[MEMBER_COORD:.*]] = fir.coordinate_of %[[DECLARE]]#0,
%[[MEMBER_INDEX]] :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
index) -> !fir.ref>>>
+!CHECK: %[[MEMBER_BASE_ADDR:.*]] = fir.box_offset %[[MEMBER_COORD]] base_addr
: (!fir.ref>>>) ->
!fir.llvm_ptr>>
+!CHECK: %[[MAP_MEMBER_BASE_ADDR:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]]
: !fir.ref>>>, !fir.array)
var_ptr_ptr(%[[MEMBER_BASE_ADDR]] : !fir.llvm_ptr>>)
map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) ->
!fir.llvm_ptr>> {name = ""}
+!CHECK: %[[MAP_MEMBER_DESCRIPTOR:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]]
: !fir.ref>>>,
!fir.box>>) map_clauses(to) capture(ByRef) ->
!fir.ref>>> {name = "one_l%array_j"}
+!CHECK: %[[MAP_PARENT:.*]] = omp.map.info var_ptr(%[[DECLARE]]#1 :
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.type<_QFtest_derived_type_allocatable_map_operand_and_block_additionTone_layer{i:f32,scalar:!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>)
map_clauses(tofrom) capture(ByRef) members(%[[MAP_MEMBER_DESCRIPTOR]],
%[[MAP_MEMBER_BASE_ADDR]] : [4], [4,0] :
!fir.ref>>>,
!fir.llvm_ptr>>) ->
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>
{name = "one_l", partial_map = true}
+!CHECK: omp.target map_entries(%[[MAP_MEMBER_DESCRIPTOR]] -> %[[ARG0:.*]],
%[[MAP_MEMBER_BASE_ADDR]] -> %[[ARG1:.*]], %[[MAP_PARENT]] -> %[[ARG2:.*]] :
!fir.ref>>>,
!fir.llvm_ptr>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
{
+!CHECK:%{{.*}}:2 = hlfir.declare %[[ARG2]] {uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
+subroutine test_derived_type_allocatable_map_operand_and_block_addition()
+type :: one_layer
+real(4) :: i
+integer, allocatable :: scalar
+integer(4) :: array_i(10)
+real(4) :: j
+integer, allocatable :: array_j(:)
+integer(4) :: k
+end type one_layer
+
+type(one_layer) :: one_l
+
+allocate(one_l%array_j(10))
+
+!$omp target map(tofrom: one_l%array_j)
+one_l%array_j(1) = 10
+!$omp end target
+end subroutine
+
+!CHECK: %[[ALLOCA:.*]] = fir.alloca
!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>>
{bindc_name = "one_l", uniq_name =
"_QFtest_allocatable_derived_type_map_operand_and_block_additionEone_l"}
+!CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {fortran_attrs =
#fir.var_attrs, uniq_name =
"_QFtest_allocatable_derived_type_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32})
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32},
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32})
+!CHECK: %[[BOUNDS:.*]] = omp.map.bounds lower_bound({{.*}})
upper_bound({{.*}}) extent({{.*}}) stride({{.*}}) start_idx({{.*}})
{stride_in_bytes = true}
+!CHECK: %[[LOAD_DTYPE:.*]] = fir.load %[[DECLARE]]#0 :
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}
+!CHECK: %[[MEMBER_INDEX:.*]] = arith.constant 4 : index
+!CHECK: %[[MEMBER_COORD:.*]] = fir.coordinate_of %[[LOAD_DTYPE]],
%[[MEMBER_INDEX]] :
(!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>>,
index) -> !fir.ref>>>
+!CHECK: %[[MEMBER_BASE_ADDR:.*]] = fir.box_offset %[[MEMBER_COORD]] base_addr
: (!fir.ref>>>) ->
!fir.llvm_ptr>>
+!CHECK: %[[MAP_MEMBER_BASE_ADDR:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]]
: !fir.ref>>>, !fir.array)
var_ptr_ptr(%[[MEMBER_BASE_ADDR]] : !fir.llvm_ptr>>)
map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) ->
!fir.llvm_ptr>> {name = ""}
+!CHECK: %[[MAP_MEMBER_DESC:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]] :
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -0,0 +1,161 @@ +!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s ergawy wrote: In the whole file, can you capture the names of longs types in FileCheck variables and use the captured type names to improve readability and reduce line lengths a bit? https://github.com/llvm/llvm-project/pull/92 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
@@ -0,0 +1,161 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+!CHECK: %[[ALLOCA:.*]] = fir.alloca
!fir.type<_QFtest_derived_type_allocatable_map_operand_and_block_additionTone_layer{i:f32,scalar:!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>
{bindc_name = "one_l", uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"}
+!CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
+!CHECK: %[[BOUNDS:.*]] = omp.map.bounds lower_bound({{.*}})
upper_bound({{.*}}) extent({{.*}}) stride({{.*}}) start_idx({{.*}})
{stride_in_bytes = true}
+!CHECK: %[[MEMBER_INDEX:.*]] = arith.constant 4 : index
+!CHECK: %[[MEMBER_COORD:.*]] = fir.coordinate_of %[[DECLARE]]#0,
%[[MEMBER_INDEX]] :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
index) -> !fir.ref>>>
+!CHECK: %[[MEMBER_BASE_ADDR:.*]] = fir.box_offset %[[MEMBER_COORD]] base_addr
: (!fir.ref>>>) ->
!fir.llvm_ptr>>
+!CHECK: %[[MAP_MEMBER_BASE_ADDR:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]]
: !fir.ref>>>, !fir.array)
var_ptr_ptr(%[[MEMBER_BASE_ADDR]] : !fir.llvm_ptr>>)
map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) ->
!fir.llvm_ptr>> {name = ""}
+!CHECK: %[[MAP_MEMBER_DESCRIPTOR:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]]
: !fir.ref>>>,
!fir.box>>) map_clauses(to) capture(ByRef) ->
!fir.ref>>> {name = "one_l%array_j"}
+!CHECK: %[[MAP_PARENT:.*]] = omp.map.info var_ptr(%[[DECLARE]]#1 :
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.type<_QFtest_derived_type_allocatable_map_operand_and_block_additionTone_layer{i:f32,scalar:!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>)
map_clauses(tofrom) capture(ByRef) members(%[[MAP_MEMBER_DESCRIPTOR]],
%[[MAP_MEMBER_BASE_ADDR]] : [4], [4,0] :
!fir.ref>>>,
!fir.llvm_ptr>>) ->
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>
{name = "one_l", partial_map = true}
+!CHECK: omp.target map_entries(%[[MAP_MEMBER_DESCRIPTOR]] -> %[[ARG0:.*]],
%[[MAP_MEMBER_BASE_ADDR]] -> %[[ARG1:.*]], %[[MAP_PARENT]] -> %[[ARG2:.*]] :
!fir.ref>>>,
!fir.llvm_ptr>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
{
+!CHECK:%{{.*}}:2 = hlfir.declare %[[ARG2]] {uniq_name =
"_QFtest_derived_type_allocatable_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>,
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>)
+subroutine test_derived_type_allocatable_map_operand_and_block_addition()
+type :: one_layer
+real(4) :: i
+integer, allocatable :: scalar
+integer(4) :: array_i(10)
+real(4) :: j
+integer, allocatable :: array_j(:)
+integer(4) :: k
+end type one_layer
+
+type(one_layer) :: one_l
+
+allocate(one_l%array_j(10))
+
+!$omp target map(tofrom: one_l%array_j)
+one_l%array_j(1) = 10
+!$omp end target
+end subroutine
+
+!CHECK: %[[ALLOCA:.*]] = fir.alloca
!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>>
{bindc_name = "one_l", uniq_name =
"_QFtest_allocatable_derived_type_map_operand_and_block_additionEone_l"}
+!CHECK: %[[DECLARE:.*]]:2 = hlfir.declare %[[ALLOCA]] {fortran_attrs =
#fir.var_attrs, uniq_name =
"_QFtest_allocatable_derived_type_map_operand_and_block_additionEone_l"} :
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32})
->
(!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32},
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32})
+!CHECK: %[[BOUNDS:.*]] = omp.map.bounds lower_bound({{.*}})
upper_bound({{.*}}) extent({{.*}}) stride({{.*}}) start_idx({{.*}})
{stride_in_bytes = true}
+!CHECK: %[[LOAD_DTYPE:.*]] = fir.load %[[DECLARE]]#0 :
!fir.ref>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}
+!CHECK: %[[MEMBER_INDEX:.*]] = arith.constant 4 : index
+!CHECK: %[[MEMBER_COORD:.*]] = fir.coordinate_of %[[LOAD_DTYPE]],
%[[MEMBER_INDEX]] :
(!fir.box>,array_i:!fir.array<10xi32>,j:f32,array_j:!fir.box>>,k:i32}>>>,
index) -> !fir.ref>>>
+!CHECK: %[[MEMBER_BASE_ADDR:.*]] = fir.box_offset %[[MEMBER_COORD]] base_addr
: (!fir.ref>>>) ->
!fir.llvm_ptr>>
+!CHECK: %[[MAP_MEMBER_BASE_ADDR:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]]
: !fir.ref>>>, !fir.array)
var_ptr_ptr(%[[MEMBER_BASE_ADDR]] : !fir.llvm_ptr>>)
map_clauses(tofrom) capture(ByRef) bounds(%[[BOUNDS]]) ->
!fir.llvm_ptr>> {name = ""}
+!CHECK: %[[MAP_MEMBER_DESC:.*]] = omp.map.info var_ptr(%[[MEMBER_COORD]] :
[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #113557)
https://github.com/ergawy approved this pull request. Thanks Andrew. Compared the new PR against my comments in the old one and seems ok to me. However, the PR is hge and I cannot claim I got every little detail of what is going on, therefore, it would be better to wait for other approvals. https://github.com/llvm/llvm-project/pull/113557 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Add version checks for clauses (PR #110015)
https://github.com/ergawy approved this pull request. Nice! LGTM! https://github.com/llvm/llvm-project/pull/110015 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [MLIR][OpenMP] Add conversion support from FIR to LLVM Dialect for OMP DeclareMapper (PR #121005)
https://github.com/ergawy commented: Thanks Akash! Just a small comment! https://github.com/llvm/llvm-project/pull/121005 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [MLIR][OpenMP] Add conversion support from FIR to LLVM Dialect for OMP DeclareMapper (PR #121005)
@@ -186,6 +186,32 @@ struct MapInfoOpConversion : public
ConvertOpToLLVMPattern {
}
};
+struct DeclMapperOpConversion
ergawy wrote:
I think we can resue `MultiRegionOpConversion` and specialize `forwardOpAttrs`
for `DeclareMapperOp`, right?
https://github.com/llvm/llvm-project/pull/121005
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP custom mappers in map clause (PR #121001)
@@ -0,0 +1,23 @@
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %s -o - | FileCheck
%s
+program p
+ integer, parameter :: n = 256
+ type t1
+ integer :: x(256)
+ end type t1
+
+ !$omp declare mapper(xx : t1 :: nn) map(to: nn, nn%x)
+ !$omp declare mapper(t1 :: nn) map(from: nn)
+
+ !CHECK-LABEL: omp.declare_mapper @_QQFt1.default :
!fir.type<_QFTt1{x:!fir.array<256xi32>}>
+ !CHECK-LABEL: omp.declare_mapper @_QQFxx :
!fir.type<_QFTt1{x:!fir.array<256xi32>}>
+
+ type(t1) :: a, b
+ !CHECK: %[[MAP_A:.*]] = omp.map.info var_ptr(%{{.*}} : {{.*}}, {{.*}})
mapper(@_QQFxx) map_clauses(tofrom) capture(ByRef) -> {{.*}} {name = "a"}
+ !CHECK: %[[MAP_B:.*]] = omp.map.info var_ptr(%{{.*}} : {{.*}}, {{.*}})
mapper(@_QQFt1.default) map_clauses(tofrom) capture(ByRef) -> {{.*}} {name =
"b"}
+ !CHECK: omp.target map_entries(%[[MAP_A]] -> %{{.*}}, %[[MAP_B]] ->
%{{.*}}, %{{.*}} -> %{{.*}}, %{{.*}} -> %{{.*}} : {{.*}}, {{.*}}, {{.*}},
{{.*}}) {
+ !$omp target map(mapper(xx) : a) map(mapper(default) : b)
ergawy wrote:
nit: can we split this into 2: one for the `xx` mapper and one for the
`default` mapper. Just to highlight the fact that the 2 are treated differently
by code-gen.
https://github.com/llvm/llvm-project/pull/121001
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP custom mappers in map clause (PR #121001)
@@ -970,6 +972,20 @@ void ClauseProcessor::processMapObjects(
}
}
+if (!mapperIdName.empty()) {
+ if (mapperIdName == "default") {
+auto &typeSpec = object.sym()->owner().IsDerivedType()
+ ? *object.sym()->owner().derivedTypeSpec()
+ : object.sym()->GetType()->derivedTypeSpec();
ergawy wrote:
Is this safe to do without making sure that `object.sym()`'s type is derived?
https://github.com/llvm/llvm-project/pull/121001
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [MLIR][OpenMP] Add Lowering support for OpenMP custom mappers in map clause (PR #121001)
@@ -1057,13 +1075,17 @@ bool ClauseProcessor::processMap(
"Support for iterator modifiers is not implemented yet");
}
if (mappers) {
- TODO(currentLocation,
- "Support for mapper modifiers is not implemented yet");
+ assert(mappers->size() == 1 && "more than one mapper");
+ mapperIdName = mappers->front().v.id().symbol->name().ToString();
+ if (mapperIdName != "default")
ergawy wrote:
I think it would be better to do this processing of the `mapperIdName` inside
`processMapObjects` since we do further processing of the name in that function
anyway ([line
975](https://github.com/llvm/llvm-project/pull/121001/files#diff-55798c5090a8f8499f773b3fb46fb98a7aabe0f58ec60ff295e107acb36c7707R975)
in `ClauseProcessor.cpp`).
https://github.com/llvm/llvm-project/pull/121001
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [MLIR][OpenMP] Add OMP Mapper field to MapInfoOp (PR #120994)
https://github.com/ergawy approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/120994 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [mlir] [MLIR][OpenMP] Add conversion support from FIR to LLVM Dialect for OMP DeclareMapper (PR #121005)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/121005 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/124746 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
@@ -3421,6 +3441,85 @@ static void genMapInfos(llvm::IRBuilderBase &builder,
}
}
+static llvm::Expected
+emitUserDefinedMapper(Operation *declMapperOp, llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation);
+
+static llvm::Expected
+getOrCreateUserDefinedMapperFunc(Operation *declMapperOp,
+ llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation) {
+ llvm::DenseMap userDefMapperMap;
+ auto iter = userDefMapperMap.find(declMapperOp);
ergawy wrote:
Yes, that's my point, you get a new instance of `userDefMapperMap` with every
invocation of `getOrCreateUserDefinedMapperFunc` and you directly search it for
the user defined maper of `declMapperOp`. So nothing is actually cache. Did I
miss something?
https://github.com/llvm/llvm-project/pull/124746
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/124746 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
@@ -3421,6 +3441,85 @@ static void genMapInfos(llvm::IRBuilderBase &builder,
}
}
+static llvm::Expected
+emitUserDefinedMapper(Operation *declMapperOp, llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation);
+
+static llvm::Expected
+getOrCreateUserDefinedMapperFunc(Operation *declMapperOp,
+ llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation) {
+ llvm::DenseMap userDefMapperMap;
+ auto iter = userDefMapperMap.find(declMapperOp);
+ if (iter != userDefMapperMap.end())
+return iter->second;
+ llvm::Expected mapperFunc =
+ emitUserDefinedMapper(declMapperOp, builder, moduleTranslation);
+ if (!mapperFunc)
+return mapperFunc.takeError();
+ userDefMapperMap.try_emplace(declMapperOp, *mapperFunc);
+ return userDefMapperMap.lookup(declMapperOp);
+}
+
+static llvm::Expected
+emitUserDefinedMapper(Operation *op, llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation) {
+ auto declMapperOp = cast(op);
+ auto declMapperInfoOp =
+ *declMapperOp.getOps().begin();
+ DataLayout dl = DataLayout(declMapperOp->getParentOfType());
+ llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+ llvm::Type *varType =
+ moduleTranslation.convertType(declMapperOp.getVarType());
+ std::string mapperName = ompBuilder->createPlatformSpecificName(
+ {"omp_mapper", declMapperOp.getSymName()});
+ SmallVector mapVars = declMapperInfoOp.getMapVars();
+
+ using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+ // Fill up the arrays with all the mapped variables.
+ MapInfosTy combinedInfo;
+ auto genMapInfoCB =
+ [&](InsertPointTy codeGenIP, llvm::Value *ptrPHI,
+ llvm::Value *unused2) -> llvm::OpenMPIRBuilder::MapInfosOrErrorTy {
+builder.restoreIP(codeGenIP);
+moduleTranslation.mapValue(declMapperOp.getRegion().getArgument(0),
ptrPHI);
+moduleTranslation.mapBlock(&declMapperOp.getRegion().front(),
+ builder.GetInsertBlock());
+if (failed(moduleTranslation.convertBlock(declMapperOp.getRegion().front(),
+ /*ignoreArguments=*/true,
+ builder)))
+ return llvm::make_error();
+MapInfoData mapData;
+collectMapDataFromMapOperands(mapData, mapVars, moduleTranslation, dl,
+ builder);
+genMapInfos(builder, moduleTranslation, dl, combinedInfo, mapData);
+
+// Drop the mapping that is no longer necessary so that the same region can
+// be processed multiple times.
+moduleTranslation.forgetMapping(declMapperOp.getRegion());
+return combinedInfo;
+ };
+
+ auto customMapperCB = [&](unsigned i, llvm::Function **mapperFunc) {
+if (combinedInfo.Mappers[i]) {
+ // Call the corresponding mapper function.
+ llvm::Expected newFn =
getOrCreateUserDefinedMapperFunc(
ergawy wrote:
> A declare mapper may refer to another mapper in it's mapping scheme
Can you please add a test that activates this recursion to better understand
how it works? I believe the 2 tests added in the PR don't achieve that. I am
just a bit uncomfortable doing this without being sure how it works.
In general, instead of the recursion, we can try to come up with a worklist
algorithm that builds up the list of mapper functions that need to be generated.
https://github.com/llvm/llvm-project/pull/124746
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/124746 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
@@ -2709,13 +2709,23 @@ getRefPtrIfDeclareTarget(mlir::Value value,
}
namespace {
+// Append customMappers information to existing MapInfosTy
+struct MapInfosTy : llvm::OpenMPIRBuilder::MapInfosTy {
+ SmallVector Mappers;
ergawy wrote:
I understand we cannot merge all of this into
`llvm::OpenMPIRBuilder::MapInfosTy`. I am talking about the new struct
`MapInfosTy` that sets in between `llvm::OpenMPIRBuilder::MapInfosTy` and
`MapInfoData`. I think we can make this new struct and `MapInfoData` one struct
and use it in this file. I gave it a try and should be fine.
https://github.com/llvm/llvm-project/pull/124746
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [mlir] [MLIR][OpenMP] Add LLVM translation support for OpenMP UserDefinedMappers (PR #124746)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/124746 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)
https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/125307 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)
@@ -1730,6 +1730,126 @@ buildDependData(std::optional dependKinds,
OperandRange dependVars,
}
}
+static bool privatizerReadsSourceVariable(omp::PrivateClauseOp &priv) {
+ if (priv.getDataSharingType() == omp::DataSharingClauseType::FirstPrivate)
+return true;
+
+ Region &initRegion = priv.getInitRegion();
+ if (initRegion.empty())
+return false;
+
+ BlockArgument sourceVariable = priv.getInitMoldArg();
+ if (!sourceVariable)
+return false;
+ return !sourceVariable.use_empty();
+}
+
+namespace {
+/// TaskContextStructManager takes care of creating and freeing a structure
+/// containing information needed by the task body to execute.
+class TaskContextStructManager {
+public:
+ TaskContextStructManager(llvm::IRBuilderBase &builder,
+ LLVM::ModuleTranslation &moduleTranslation,
+ MutableArrayRef privateDecls)
+ : builder{builder}, moduleTranslation{moduleTranslation},
+privateDecls{privateDecls} {}
+
+ /// Creates a heap allocated struct containing space for each private
+ /// variable. Returns nullptr if there are is no struct needed. Invariant:
ergawy wrote:
I think the comment needs to be updated, this method does not return a value.
https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)
https://github.com/ergawy commented: Thanks Tom! I have a few comments. Not much of an expert when it comes to tasks so I might have missed something worth noting. https://github.com/llvm/llvm-project/pull/125307 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
