[Clang][OpenMP][OMPIRBuilder] Move Clang's OpenMP Member/MemberOf flag helpers into the OMPIRBuilder #67844
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s into the OMPIRBuilder This patch seeks to move the following functions to the OMPIRBuilder: - getFlagMemberOffset - getMemberOfFlag - setCorrectMemberOfFlag These small helper functions help set the end bits of the OpenMPOffloadMappingFlags flag that correspond to the reserved segment for OMP_MAP_MEMBER_OF. They will be of use in the future for lowering MLIR types/values that can contian members and can be lowered similarly to a structure or class type within the OpenMPToLLVMIRTranslation step of the OpenMP dialects lowering to LLVM-IR. In particular for Flang which currently uses this flow. Types with descriptors like pointers/allocatables, and likely derived types in certain cases can be lowered as if they were structures with explicitly mapped members.
agozillon
requested review from
skatrak,
jsjodin,
jdoerfert,
raghavendhra,
DominikAdamski,
dpalermo,
shraiysh,
NimishMishra,
TIFitis and
kiranktp
llvmbot
added
clang
agozillon
added
openmp
clang
|
@llvm/pr-subscribers-openmp @llvm/pr-subscribers-clang ChangesThis patch seeks to move the following functions to the OMPIRBuilder:
These small helper functions help set the end bits of the OpenMPOffloadMappingFlags flag that correspond to the reserved segment for OMP_MAP_MEMBER_OF. They will be of use in the future for lowering MLIR types/values that can contian members and can be lowered similarly to a structure or class type within the OpenMPToLLVMIRTranslation step of the OpenMP dialects lowering to LLVM-IR. In particular for Flang which currently uses this flow. Types with descriptors like pointers/allocatables, and likely derived types in certain cases can be lowered as if they were structures with explicitly mapped members. Full diff: https://github.com/llvm/llvm-project/pull/67844.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index b4e482ca5cba6ee..572fa9691b813c0 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7734,30 +7734,6 @@ class MappableExprsHandler {
OpenMPOffloadMappingFlags::OMP_MAP_FROM;
}
- static OpenMPOffloadMappingFlags getMemberOfFlag(unsigned Position) {
- // Rotate by getFlagMemberOffset() bits.
- return static_cast<OpenMPOffloadMappingFlags>(((uint64_t)Position + 1)
- << getFlagMemberOffset());
- }
-
- static void setCorrectMemberOfFlag(OpenMPOffloadMappingFlags &Flags,
- OpenMPOffloadMappingFlags MemberOfFlag) {
- // If the entry is PTR_AND_OBJ but has not been marked with the special
- // placeholder value 0xFFFF in the MEMBER_OF field, then it should not be
- // marked as MEMBER_OF.
- if (static_cast<std::underlying_type_t<OpenMPOffloadMappingFlags>>(
- Flags & OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ) &&
- static_cast<std::underlying_type_t<OpenMPOffloadMappingFlags>>(
- (Flags & OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF) !=
- OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF))
- return;
-
- // Reset the placeholder value to prepare the flag for the assignment of the
- // proper MEMBER_OF value.
- Flags &= ~OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF;
- Flags |= MemberOfFlag;
- }
-
void getPlainLayout(const CXXRecordDecl *RD,
llvm::SmallVectorImpl<const FieldDecl *> &Layout,
bool AsBase) const {
@@ -7825,6 +7801,7 @@ class MappableExprsHandler {
/// the device pointers info array.
void generateAllInfoForClauses(
ArrayRef<const OMPClause *> Clauses, MapCombinedInfoTy &CombinedInfo,
+ llvm::OpenMPIRBuilder &OMPBuilder,
const llvm::DenseSet<CanonicalDeclPtr<const Decl>> &SkipVarSet =
llvm::DenseSet<CanonicalDeclPtr<const Decl>>()) const {
// We have to process the component lists that relate with the same
@@ -8159,7 +8136,7 @@ class MappableExprsHandler {
if (PartialStruct.Base.isValid()) {
CurInfo.NonContigInfo.Dims.push_back(0);
emitCombinedEntry(CombinedInfo, CurInfo.Types, PartialStruct,
- /*IsMapThis*/ !VD, VD);
+ /*IsMapThis*/ !VD, OMPBuilder, VD);
}
// We need to append the results of this capture to what we already
@@ -8226,6 +8203,7 @@ class MappableExprsHandler {
void emitCombinedEntry(MapCombinedInfoTy &CombinedInfo,
MapFlagsArrayTy &CurTypes,
const StructRangeInfoTy &PartialStruct, bool IsMapThis,
+ llvm::OpenMPIRBuilder &OMPBuilder,
const ValueDecl *VD = nullptr,
bool NotTargetParams = true) const {
if (CurTypes.size() == 1 &&
@@ -8313,9 +8291,9 @@ class MappableExprsHandler {
// (except for PTR_AND_OBJ entries which do not have a placeholder value
// 0xFFFF in the MEMBER_OF field).
OpenMPOffloadMappingFlags MemberOfFlag =
- getMemberOfFlag(CombinedInfo.BasePointers.size() - 1);
+ OMPBuilder.getMemberOfFlag(CombinedInfo.BasePointers.size() - 1);
for (auto &M : CurTypes)
- setCorrectMemberOfFlag(M, MemberOfFlag);
+ OMPBuilder.setCorrectMemberOfFlag(M, MemberOfFlag);
}
/// Generate all the base pointers, section pointers, sizes, map types, and
@@ -8324,23 +8302,26 @@ class MappableExprsHandler {
/// pair of the relevant declaration and index where it occurs is appended to
/// the device pointers info array.
void generateAllInfo(
- MapCombinedInfoTy &CombinedInfo,
+ MapCombinedInfoTy &CombinedInfo, llvm::OpenMPIRBuilder &OMPBuilder,
const llvm::DenseSet<CanonicalDeclPtr<const Decl>> &SkipVarSet =
llvm::DenseSet<CanonicalDeclPtr<const Decl>>()) const {
assert(CurDir.is<const OMPExecutableDirective *>() &&
"Expect a executable directive");
const auto *CurExecDir = CurDir.get<const OMPExecutableDirective *>();
- generateAllInfoForClauses(CurExecDir->clauses(), CombinedInfo, SkipVarSet);
+ generateAllInfoForClauses(CurExecDir->clauses(), CombinedInfo, OMPBuilder,
+ SkipVarSet);
}
/// Generate all the base pointers, section pointers, sizes, map types, and
/// mappers for the extracted map clauses of user-defined mapper (all included
/// in \a CombinedInfo).
- void generateAllInfoForMapper(MapCombinedInfoTy &CombinedInfo) const {
+ void generateAllInfoForMapper(MapCombinedInfoTy &CombinedInfo,
+ llvm::OpenMPIRBuilder &OMPBuilder) const {
assert(CurDir.is<const OMPDeclareMapperDecl *>() &&
"Expect a declare mapper directive");
const auto *CurMapperDir = CurDir.get<const OMPDeclareMapperDecl *>();
- generateAllInfoForClauses(CurMapperDir->clauses(), CombinedInfo);
+ generateAllInfoForClauses(CurMapperDir->clauses(), CombinedInfo,
+ OMPBuilder);
}
/// Emit capture info for lambdas for variables captured by reference.
@@ -8422,6 +8403,7 @@ class MappableExprsHandler {
/// Set correct indices for lambdas captures.
void adjustMemberOfForLambdaCaptures(
+ llvm::OpenMPIRBuilder &OMPBuilder,
const llvm::DenseMap<llvm::Value *, llvm::Value *> &LambdaPointers,
MapBaseValuesArrayTy &BasePointers, MapValuesArrayTy &Pointers,
MapFlagsArrayTy &Types) const {
@@ -8446,8 +8428,9 @@ class MappableExprsHandler {
// All other current entries will be MEMBER_OF the combined entry
// (except for PTR_AND_OBJ entries which do not have a placeholder value
// 0xFFFF in the MEMBER_OF field).
- OpenMPOffloadMappingFlags MemberOfFlag = getMemberOfFlag(TgtIdx);
- setCorrectMemberOfFlag(Types[I], MemberOfFlag);
+ OpenMPOffloadMappingFlags MemberOfFlag =
+ OMPBuilder.getMemberOfFlag(TgtIdx);
+ OMPBuilder.setCorrectMemberOfFlag(Types[I], MemberOfFlag);
}
}
@@ -9141,7 +9124,7 @@ void CGOpenMPRuntime::emitUserDefinedMapper(const OMPDeclareMapperDecl *D,
// Get map clause information. Fill up the arrays with all mapped variables.
MappableExprsHandler::MapCombinedInfoTy Info;
MappableExprsHandler MEHandler(*D, MapperCGF);
- MEHandler.generateAllInfoForMapper(Info);
+ MEHandler.generateAllInfoForMapper(Info, OMPBuilder);
// Call the runtime API __tgt_mapper_num_components to get the number of
// pre-existing components.
@@ -9525,7 +9508,8 @@ static void emitTargetCallKernelLaunch(
CombinedInfo.append(PartialStruct.PreliminaryMapData);
MEHandler.emitCombinedEntry(
CombinedInfo, CurInfo.Types, PartialStruct, CI->capturesThis(),
- nullptr, !PartialStruct.PreliminaryMapData.BasePointers.empty());
+ OMPBuilder, nullptr,
+ !PartialStruct.PreliminaryMapData.BasePointers.empty());
}
// We need to append the results of this capture to what we already have.
@@ -9533,11 +9517,11 @@ static void emitTargetCallKernelLaunch(
}
// Adjust MEMBER_OF flags for the lambdas captures.
MEHandler.adjustMemberOfForLambdaCaptures(
- LambdaPointers, CombinedInfo.BasePointers, CombinedInfo.Pointers,
- CombinedInfo.Types);
+ OMPBuilder, LambdaPointers, CombinedInfo.BasePointers,
+ CombinedInfo.Pointers, CombinedInfo.Types);
// Map any list items in a map clause that were not captures because they
// weren't referenced within the construct.
- MEHandler.generateAllInfo(CombinedInfo, MappedVarSet);
+ MEHandler.generateAllInfo(CombinedInfo, OMPBuilder, MappedVarSet);
CGOpenMPRuntime::TargetDataInfo Info;
// Fill up the arrays and create the arguments.
@@ -10272,7 +10256,7 @@ void CGOpenMPRuntime::emitTargetDataCalls(
CGF.Builder.restoreIP(CodeGenIP);
// Get map clause information.
MappableExprsHandler MEHandler(D, CGF);
- MEHandler.generateAllInfo(CombinedInfo);
+ MEHandler.generateAllInfo(CombinedInfo, OMPBuilder);
auto FillInfoMap = [&](MappableExprsHandler::MappingExprInfo &MapExpr) {
return emitMappingInformation(CGF, OMPBuilder, MapExpr);
@@ -10478,7 +10462,7 @@ void CGOpenMPRuntime::emitTargetDataStandAloneCall(
// Get map clause information.
MappableExprsHandler MEHandler(D, CGF);
- MEHandler.generateAllInfo(CombinedInfo);
+ MEHandler.generateAllInfo(CombinedInfo, OMPBuilder);
CGOpenMPRuntime::TargetDataInfo Info;
// Fill up the arrays and create the arguments.
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index 1699ed3aeab7661..75da461cfd8d95e 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -877,6 +877,30 @@ class OpenMPIRBuilder {
std::function<GlobalValue::LinkageTypes()> VariableLinkage,
Type *LlvmPtrTy, Constant *Addr);
+ /// Get the offset of the OMP_MAP_MEMBER_OF field.
+ unsigned getFlagMemberOffset();
+
+ /// Get OMP_MAP_MEMBER_OF flag with extra bits reserved based on
+ /// the position given.
+ /// \param Position - A value indicating the position of the parent
+ /// of the member in the kernel argument structure, often retrieved
+ /// by the parents position in the combined information vectors used
+ /// to generate the structure itself. Multiple children (member's of)
+ /// with the same parent will use the same returned member flag.
+ omp::OpenMPOffloadMappingFlags getMemberOfFlag(unsigned Position);
+
+ /// Given an initial flag set, this function modifies it to contain
+ /// the passed in MemberOfFlag generated from the getMemberOfFlag
+ /// function. The results are dependent on the existing flag bits
+ /// set in the original flag set.
+ /// \param Flags - The original set of flags to be modified with the
+ /// passed in MemberOfFlag.
+ /// \param MemberOfFlag - A modified OMP_MAP_MEMBER_OF flag, adjusted
+ /// slightly based on the getMemberOfFlag which adjusts the flag bits
+ /// based on the members position in its parent.
+ void setCorrectMemberOfFlag(omp::OpenMPOffloadMappingFlags &Flags,
+ omp::OpenMPOffloadMappingFlags MemberOfFlag);
+
private:
/// Modifies the canonical loop to be a statically-scheduled workshare loop.
///
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 9c70d384e55db2b..72e1af55fe63f60 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6126,6 +6126,42 @@ OpenMPIRBuilder::getTargetEntryUniqueInfo(FileIdentifierInfoCallbackTy CallBack,
std::get<1>(FileIDInfo));
}
+unsigned OpenMPIRBuilder::getFlagMemberOffset() {
+ unsigned Offset = 0;
+ for (uint64_t Remain =
+ static_cast<std::underlying_type_t<omp::OpenMPOffloadMappingFlags>>(
+ omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF);
+ !(Remain & 1); Remain = Remain >> 1)
+ Offset++;
+ return Offset;
+}
+
+omp::OpenMPOffloadMappingFlags
+OpenMPIRBuilder::getMemberOfFlag(unsigned Position) {
+ // Rotate by getFlagMemberOffset() bits.
+ return static_cast<omp::OpenMPOffloadMappingFlags>(((uint64_t)Position + 1)
+ << getFlagMemberOffset());
+}
+
+void OpenMPIRBuilder::setCorrectMemberOfFlag(
+ omp::OpenMPOffloadMappingFlags &Flags,
+ omp::OpenMPOffloadMappingFlags MemberOfFlag) {
+ // If the entry is PTR_AND_OBJ but has not been marked with the special
+ // placeholder value 0xFFFF in the MEMBER_OF field, then it should not be
+ // marked as MEMBER_OF.
+ if (static_cast<std::underlying_type_t<omp::OpenMPOffloadMappingFlags>>(
+ Flags & omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ) &&
+ static_cast<std::underlying_type_t<omp::OpenMPOffloadMappingFlags>>(
+ (Flags & omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF) !=
+ omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF))
+ return;
+
+ // Reset the placeholder value to prepare the flag for the assignment of the
+ // proper MEMBER_OF value.
+ Flags &= ~omp::OpenMPOffloadMappingFlags::OMP_MAP_MEMBER_OF;
+ Flags |= MemberOfFlag;
+}
+
Constant *OpenMPIRBuilder::getAddrOfDeclareTargetVar(
OffloadEntriesInfoManager::OMPTargetGlobalVarEntryKind CaptureClause,
OffloadEntriesInfoManager::OMPTargetDeviceClauseKind DeviceClause,
|
|
LGTM. Is it possible to add some tests for this? I think we might be able to add some in OpenMPIRBuilderTest.cpp. |
jdoerfert
approved these changes
Sep 30, 2023
|
Thank you all very much, I'll land this on Tuesday afternoon/evening provided no one has anything else to add in the time between. And I'll see what I can do about adding an OpenMPIRBuilderTest.cpp to the patch. |
I think it’s okay if it’s tested by clang tests (as Johannes mentioned). In case you decide to add tests for this, there is already an OpenMPIRBuilderTest.cpp in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch seeks to move the following functions to the OMPIRBuilder:
These small helper functions help set the end bits of the OpenMPOffloadMappingFlags flag that correspond to the reserved segment for OMP_MAP_MEMBER_OF.
They will be of use in the future for lowering MLIR types/values that can contian members and can be lowered similarly to a structure or class type within the OpenMPToLLVMIRTranslation step of the OpenMP dialects lowering to LLVM-IR. In particular for Flang which currently uses this flow. Types with descriptors like pointers/allocatables, and likely derived types in certain cases can be lowered as if they were structures with explicitly mapped members.