Skip to content

Conversation

@TIFitis
Copy link
Member

@TIFitis TIFitis commented May 29, 2025

This patch moves the default declare mapper name suffix ".omp.default.mapper" to the OMPConstants.h file to be used everywhere for lowering.

…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.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:parser clang:openmp OpenMP related changes to Clang labels May 29, 2025
@llvmbot
Copy link
Member

llvmbot commented May 29, 2025

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Akash Banerjee (TIFitis)

Changes

This 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:

  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+1-1)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+1-1)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+1-1)
  • (modified) llvm/include/llvm/Frontend/OpenMP/OMPConstants.h (+3)
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 {

Copy link
Contributor

Copilot AI left a 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 in OMPConstants.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 or kDefaultOpenMPMapperSuffix 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 with std::string(llvm::omp::OmpDefaultMapperName) to keep a single source of truth and avoid divergence.
return OmpMapperSpecifier{std::string("omp.default.mapper"),

@github-actions
Copy link

github-actions bot commented May 29, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a 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.

@TIFitis TIFitis merged commit 99ae675 into llvm:main May 30, 2025
5 of 9 checks passed
@TIFitis TIFitis deleted the mapper_name branch July 2, 2025 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:openmp OpenMP related changes to Clang flang:fir-hlfir flang:openmp flang:parser flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants