-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[NFC][OpenMP] Move the default declare mapper name suffix to OMPConstants.h #141964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ants.h This patch moves the default declare mapper name suffix ".omp.default.mapper" to the OMPConstants.h file to be used everywhere for lowering.
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: Akash Banerjee (TIFitis) ChangesThis patch moves the default declare mapper name suffix ".omp.default.mapper" to the OMPConstants.h file to be used everywhere for lowering. Full diff: https://github.com/llvm/llvm-project/pull/141964.diff 4 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index ebdda9885d5c2..49a3b64c03a7e 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1148,7 +1148,7 @@ void ClauseProcessor::processMapObjects(
typeSpec = &object.sym()->GetType()->derivedTypeSpec();
if (typeSpec) {
- mapperIdName = typeSpec->name().ToString() + ".omp.default.mapper";
+ mapperIdName = typeSpec->name().ToString() + llvm::omp::OmpDefaultMapperName;
if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
mapperIdName = converter.mangleName(mapperIdName, sym->owner());
}
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index ddb08f74b3841..fa711060c6b90 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -2423,7 +2423,7 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
if (sym.GetType()->category() == semantics::DeclTypeSpec::TypeDerived) {
auto &typeSpec = sym.GetType()->derivedTypeSpec();
std::string mapperIdName =
- typeSpec.name().ToString() + ".omp.default.mapper";
+ typeSpec.name().ToString() + llvm::omp::OmpDefaultMapperName;
if (auto *sym = converter.getCurrentScope().FindSymbol(mapperIdName))
mapperIdName = converter.mangleName(mapperIdName, sym->owner());
if (converter.getModuleOp().lookupSymbol(mapperIdName))
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index c08cd1ab80559..08326fad8c143 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1402,7 +1402,7 @@ static OmpMapperSpecifier ConstructOmpMapperSpecifier(
// This matches the syntax: <type-spec> :: <variable-name>
if (DerivedTypeSpec * derived{std::get_if<DerivedTypeSpec>(&typeSpec.u)}) {
return OmpMapperSpecifier{
- std::get<Name>(derived->t).ToString() + ".omp.default.mapper",
+ std::get<Name>(derived->t).ToString() + llvm::omp::OmpDefaultMapperName,
std::move(typeSpec), std::move(varName)};
}
return OmpMapperSpecifier{std::string("omp.default.mapper"),
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h b/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
index 338b56226f204..6e1bce12af8e4 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
@@ -190,6 +190,9 @@ enum class OMPScheduleType {
LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ ModifierMask)
};
+// Default OpenMP mapper name suffix.
+inline constexpr const char *OmpDefaultMapperName = ".omp.default.mapper";
+
/// Values for bit flags used to specify the mapping type for
/// offloading.
enum class OpenMPOffloadMappingFlags : uint64_t {
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Centralize the default OpenMP mapper name suffix by moving the literal into OMPConstants.h
and updating uses across parser and lowering code.
- Introduce
OmpDefaultMapperName
constant inOMPConstants.h
- Replace hardcoded suffix in parser, lowering, and clause processor with the new constant
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h | Add OmpDefaultMapperName constant for default mapper suffix |
flang/lib/Parser/openmp-parsers.cpp | Use llvm::omp::OmpDefaultMapperName instead of a string literal |
flang/lib/Lower/OpenMP/OpenMP.cpp | Replace hardcoded suffix with the new constant |
flang/lib/Lower/OpenMP/ClauseProcessor.cpp | Update suffix usage to reference the constant |
Comments suppressed due to low confidence (2)
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:194
- [nitpick] LLVM style guidelines recommend prefixing global constants with 'k' and naming them descriptively. Consider renaming to
kOMPDefaultMapperSuffix
orkDefaultOpenMPMapperSuffix
to match conventions and clarify that this is a suffix.
inline constexpr const char *OmpDefaultMapperName = ".omp.default.mapper";
flang/lib/Parser/openmp-parsers.cpp:1408
- This hardcoded literal duplicates the new constant in
OMPConstants.h
. Replace withstd::string(llvm::omp::OmpDefaultMapperName)
to keep a single source of truth and avoid divergence.
return OmpMapperSpecifier{std::string("omp.default.mapper"),
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG. Thanks for the change.
…ants.h (llvm#141964) This patch moves the default declare mapper name suffix ".omp.default.mapper" to the OMPConstants.h file to be used everywhere for lowering.
This patch moves the default declare mapper name suffix ".omp.default.mapper" to the OMPConstants.h file to be used everywhere for lowering.