https://github.com/mingmingl-llvm updated https://github.com/llvm/llvm-project/pull/162348
>From f68b06f65609078d7e1c788e95d78fab397e4966 Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Tue, 7 Oct 2025 11:59:13 -0700 Subject: [PATCH 1/3] [nfc][StaticDataLayout]Add helper functions to tell the eligibility status of a global variable for section prefix annotation --- .../llvm/Analysis/StaticDataProfileInfo.h | 18 +++++++++ llvm/lib/Analysis/StaticDataProfileInfo.cpp | 37 +++++++++++++++++++ llvm/lib/CodeGen/StaticDataAnnotator.cpp | 15 +------- .../Transforms/Instrumentation/MemProfUse.cpp | 30 ++++----------- 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h index fa21eba1377df..f06e7ceaa74ce 100644 --- a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h +++ b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h @@ -10,6 +10,24 @@ namespace llvm { +namespace memprof { +// Represents the eligibility status of a global variable for section prefix +// annotation. Other than AnnotationOk, each enum value indicates a specific +// reason for ineligibility. +enum class AnnotationKind : uint8_t { + AnnotationOK, + DeclForLinker, + ExplicitSection, + ReservedName, +}; +/// Returns the annotation kind of the global variable \p GV. +AnnotationKind getAnnotationKind(const GlobalVariable &GV); + +/// Returns true if the annotation kind of the global variable \p GV is +/// AnnotationOK. +bool IsAnnotationOK(const GlobalVariable &GV); +} // namespace memprof + /// A class that holds the constants that represent static data and their /// profile information and provides methods to operate on them. class StaticDataProfileInfo { diff --git a/llvm/lib/Analysis/StaticDataProfileInfo.cpp b/llvm/lib/Analysis/StaticDataProfileInfo.cpp index b036b2dde770e..ff4582ca7eeb1 100644 --- a/llvm/lib/Analysis/StaticDataProfileInfo.cpp +++ b/llvm/lib/Analysis/StaticDataProfileInfo.cpp @@ -6,6 +6,43 @@ #include "llvm/ProfileData/InstrProf.h" using namespace llvm; + +namespace llvm { +namespace memprof { +// Returns true iff the global variable has custom section either by +// __attribute__((section("name"))) +// (https://clang.llvm.org/docs/AttributeReference.html#section-declspec-allocate) +// or #pragma clang section directives +// (https://clang.llvm.org/docs/LanguageExtensions.html#specifying-section-names-for-global-objects-pragma-clang-section). +static bool hasExplicitSectionName(const GlobalVariable &GVar) { + if (GVar.hasSection()) + return true; + + auto Attrs = GVar.getAttributes(); + if (Attrs.hasAttribute("bss-section") || Attrs.hasAttribute("data-section") || + Attrs.hasAttribute("relro-section") || + Attrs.hasAttribute("rodata-section")) + return true; + return false; +} + +AnnotationKind getAnnotationKind(const GlobalVariable &GV) { + if (GV.isDeclarationForLinker()) + return AnnotationKind::DeclForLinker; + StringRef Name = GV.getName(); + if (Name.starts_with("llvm.")) + return AnnotationKind::ReservedName; + if (hasExplicitSectionName(GV)) + return AnnotationKind::ExplicitSection; + return AnnotationKind::AnnotationOK; +} + +bool IsAnnotationOK(const GlobalVariable &GV) { + return getAnnotationKind(GV) == AnnotationKind::AnnotationOK; +} +} // namespace memprof +} // namespace llvm + void StaticDataProfileInfo::addConstantProfileCount( const Constant *C, std::optional<uint64_t> Count) { if (!Count) { diff --git a/llvm/lib/CodeGen/StaticDataAnnotator.cpp b/llvm/lib/CodeGen/StaticDataAnnotator.cpp index 53a9ab4dbda02..9a68ee96ab056 100644 --- a/llvm/lib/CodeGen/StaticDataAnnotator.cpp +++ b/llvm/lib/CodeGen/StaticDataAnnotator.cpp @@ -75,22 +75,11 @@ bool StaticDataAnnotator::runOnModule(Module &M) { bool Changed = false; for (auto &GV : M.globals()) { - if (GV.isDeclarationForLinker()) + if (!llvm::memprof::IsAnnotationOK(GV)) continue; - // The implementation below assumes prior passes don't set section prefixes, - // and specifically do 'assign' rather than 'update'. So report error if a - // section prefix is already set. - if (auto maybeSectionPrefix = GV.getSectionPrefix(); - maybeSectionPrefix && !maybeSectionPrefix->empty()) - llvm::report_fatal_error("Global variable " + GV.getName() + - " already has a section prefix " + - *maybeSectionPrefix); - StringRef SectionPrefix = SDPI->getConstantSectionPrefix(&GV, PSI); - if (SectionPrefix.empty()) - continue; - + // setSectionPrefix returns true if the section prefix is changed. Changed |= GV.setSectionPrefix(SectionPrefix); } diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp index d86fcf268ce4f..ca2af1a9534d3 100644 --- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp @@ -17,6 +17,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Analysis/MemoryProfileInfo.h" #include "llvm/Analysis/OptimizationRemarkEmitter.h" +#include "llvm/Analysis/StaticDataProfileInfo.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/Function.h" @@ -775,23 +776,6 @@ PreservedAnalyses MemProfUsePass::run(Module &M, ModuleAnalysisManager &AM) { return PreservedAnalyses::none(); } -// Returns true iff the global variable has custom section either by -// __attribute__((section("name"))) -// (https://clang.llvm.org/docs/AttributeReference.html#section-declspec-allocate) -// or #pragma clang section directives -// (https://clang.llvm.org/docs/LanguageExtensions.html#specifying-section-names-for-global-objects-pragma-clang-section). -static bool hasExplicitSectionName(const GlobalVariable &GVar) { - if (GVar.hasSection()) - return true; - - auto Attrs = GVar.getAttributes(); - if (Attrs.hasAttribute("bss-section") || Attrs.hasAttribute("data-section") || - Attrs.hasAttribute("relro-section") || - Attrs.hasAttribute("rodata-section")) - return true; - return false; -} - bool MemProfUsePass::annotateGlobalVariables( Module &M, const memprof::DataAccessProfData *DataAccessProf) { if (!AnnotateStaticDataSectionPrefix || M.globals().empty()) @@ -817,13 +801,16 @@ bool MemProfUsePass::annotateGlobalVariables( for (GlobalVariable &GVar : M.globals()) { assert(!GVar.getSectionPrefix().has_value() && "GVar shouldn't have section prefix yet"); - if (GVar.isDeclarationForLinker()) - continue; - - if (hasExplicitSectionName(GVar)) { + auto Kind = llvm::memprof::getAnnotationKind(GVar); + switch (Kind) { + case llvm::memprof::AnnotationKind::AnnotationOK: + break; + case llvm::memprof::AnnotationKind::ExplicitSection: ++NumOfMemProfExplicitSectionGlobalVars; LLVM_DEBUG(dbgs() << "Global variable " << GVar.getName() << " has explicit section name. Skip annotating.\n"); + [[fallthrough]]; + default: continue; } @@ -833,7 +820,6 @@ bool MemProfUsePass::annotateGlobalVariables( // TODO: Track string content hash in the profiles and compute it inside the // compiler to categeorize the hotness string literals. if (Name.starts_with(".str")) { - LLVM_DEBUG(dbgs() << "Skip annotating string literal " << Name << "\n"); continue; } >From 331888be493563f8446e17ce670af7ceebffb025 Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Tue, 7 Oct 2025 12:20:49 -0700 Subject: [PATCH 2/3] save irrelevant change for the next PR to make this one focused --- llvm/lib/CodeGen/StaticDataAnnotator.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/llvm/lib/CodeGen/StaticDataAnnotator.cpp b/llvm/lib/CodeGen/StaticDataAnnotator.cpp index 9a68ee96ab056..9b737751c4a98 100644 --- a/llvm/lib/CodeGen/StaticDataAnnotator.cpp +++ b/llvm/lib/CodeGen/StaticDataAnnotator.cpp @@ -78,8 +78,19 @@ bool StaticDataAnnotator::runOnModule(Module &M) { if (!llvm::memprof::IsAnnotationOK(GV)) continue; + // The implementation below assumes prior passes don't set section prefixes, + // and specifically do 'assign' rather than 'update'. So report error if a + // section prefix is already set. + if (auto maybeSectionPrefix = GV.getSectionPrefix(); + maybeSectionPrefix && !maybeSectionPrefix->empty()) + llvm::report_fatal_error("Global variable " + GV.getName() + + " already has a section prefix " + + *maybeSectionPrefix); + StringRef SectionPrefix = SDPI->getConstantSectionPrefix(&GV, PSI); - // setSectionPrefix returns true if the section prefix is changed. + if (SectionPrefix.empty()) + continue; + Changed |= GV.setSectionPrefix(SectionPrefix); } >From dcb9f001398ebffb2b829092f07c6c8cbc33941a Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Tue, 7 Oct 2025 14:38:57 -0700 Subject: [PATCH 3/3] resolve comments --- llvm/lib/Analysis/StaticDataProfileInfo.cpp | 3 +++ llvm/lib/CodeGen/StaticDataSplitter.cpp | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Analysis/StaticDataProfileInfo.cpp b/llvm/lib/Analysis/StaticDataProfileInfo.cpp index ff4582ca7eeb1..1f751ee5e09d9 100644 --- a/llvm/lib/Analysis/StaticDataProfileInfo.cpp +++ b/llvm/lib/Analysis/StaticDataProfileInfo.cpp @@ -29,9 +29,12 @@ static bool hasExplicitSectionName(const GlobalVariable &GVar) { AnnotationKind getAnnotationKind(const GlobalVariable &GV) { if (GV.isDeclarationForLinker()) return AnnotationKind::DeclForLinker; + // Skip 'llvm.'-prefixed global variables conservatively because they are + // often handled specially, StringRef Name = GV.getName(); if (Name.starts_with("llvm.")) return AnnotationKind::ReservedName; + // Respect user-specified custom data sections. if (hasExplicitSectionName(GV)) return AnnotationKind::ExplicitSection; return AnnotationKind::AnnotationOK; diff --git a/llvm/lib/CodeGen/StaticDataSplitter.cpp b/llvm/lib/CodeGen/StaticDataSplitter.cpp index e22dc2507d548..1593a401bcb24 100644 --- a/llvm/lib/CodeGen/StaticDataSplitter.cpp +++ b/llvm/lib/CodeGen/StaticDataSplitter.cpp @@ -130,10 +130,8 @@ StaticDataSplitter::getConstant(const MachineOperand &Op, if (Op.isGlobal()) { // Find global variables with local linkage. const GlobalVariable *GV = getLocalLinkageGlobalVariable(Op.getGlobal()); - // Skip 'llvm.'-prefixed global variables conservatively because they are - // often handled specially, and skip those not in static data - // sections. - if (!GV || GV->getName().starts_with("llvm.") || + // Skip those not eligible for annotation or not in static data sections. + if (!GV || !llvm::memprof::IsAnnotationOK(*GV) || !inStaticDataSection(*GV, TM)) return nullptr; return GV; _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
