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 01/10] [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 02/10] 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 03/10] 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; >From 3565bc0ebf5e36225f745e5c0804f9caa9c91403 Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Wed, 8 Oct 2025 15:46:19 -0700 Subject: [PATCH 04/10] add test coverage --- .../Transforms/Instrumentation/MemProfUse.cpp | 30 +++++++++++++++---- .../CodeGen/X86/global-variable-partition.ll | 18 ++++++++--- .../PGOProfile/data-access-profile.ll | 22 ++++++++++++-- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp index ca2af1a9534d3..a4a70077264ad 100644 --- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp @@ -195,6 +195,30 @@ static bool isAllocationWithHotColdVariant(const Function *Callee, } } +static void HandleUnsupportedAnnotationKinds(GlobalVariable &GVar, + AnnotationKind Kind) { + assert(Kind != llvm::memprof::AnnotationKind::AnnotationOK && + "Should not handle AnnotationOK here"); + SmallString<32> Reason; + switch (Kind) { + case llvm::memprof::AnnotationKind::ExplicitSection: + ++NumOfMemProfExplicitSectionGlobalVars; + Reason.append("explicit section name"); + break; + case llvm::memprof::AnnotationKind::DeclForLinker: + Reason.append("linker declaration"); + break; + case llvm::memprof::AnnotationKind::ReservedName: + Reason.append("name starts with `llvm.`"); + break; + default: + llvm_unreachable("Unexpected annotation kind"); + } + LLVM_DEBUG(dbgs() << "Skip annotation for " << GVar.getName() << " due to " + << Reason << ".\n"); + return; +} + struct AllocMatchInfo { uint64_t TotalSize = 0; AllocationType AllocType = AllocationType::None; @@ -805,12 +829,8 @@ bool MemProfUsePass::annotateGlobalVariables( 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: + HandleUnsupportedAnnotationKinds(GVar, Kind); continue; } diff --git a/llvm/test/CodeGen/X86/global-variable-partition.ll b/llvm/test/CodeGen/X86/global-variable-partition.ll index ce06d1712f840..604b4fd5a96ed 100644 --- a/llvm/test/CodeGen/X86/global-variable-partition.ll +++ b/llvm/test/CodeGen/X86/global-variable-partition.ll @@ -106,23 +106,31 @@ target triple = "x86_64-unknown-linux-gnu" ; UNIQ-NEXT: .section .data.unlikely.,"aw",@progbits,unique,8 ; AGG-NEXT: .section .data.unlikely.,"aw",@progbits +;; The `.section` directive is omitted for .data with -unique-section-names=false. +; See MCSectionELF::shouldOmitSectionDirective for the implementation details. + ; For @data_with_unknown_hotness ; SYM: .type .Ldata_with_unknown_hotness,@object # @data_with_unknown_hotness ; SYM: .section .data..Ldata_with_unknown_hotness,"aw",@progbits ; UNIQ: .section .data,"aw",@progbits,unique,9 -; The `.section` directive is omitted for .data with -unique-section-names=false. -; See MCSectionELF::shouldOmitSectionDirective for the implementation details. + ; AGG: .data ; COMMON: .Ldata_with_unknown_hotness: -; For @hot_data_custom_bar_section -; It has an explicit section attribute 'var' and shouldn't have hot or unlikely suffix. +; For variables that are not eligible for section prefix annotation ; COMMON: .type hot_data_custom_bar_section,@object ; SYM-NEXT: .section bar,"aw",@progbits ; SYM: hot_data_custom_bar_section ; UNIQ: .section bar,"aw",@progbits ; AGG: .section bar,"aw",@progbits +; SYM: .section .data.llvm.fake_var,"aw" +; UNIQ: .section .data,"aw" +; AGG: .data + +;; No section for linker declaration +; COMMON-NOT: qux + @.str = private unnamed_addr constant [5 x i8] c"hot\09\00", align 1 @.str.1 = private unnamed_addr constant [10 x i8] c"%d\09%d\09%d\0A\00", align 1 @hot_relro_array = internal constant [2 x ptr] [ptr @bss2, ptr @data3] @@ -137,6 +145,8 @@ target triple = "x86_64-unknown-linux-gnu" @data3 = internal global i32 3 @data_with_unknown_hotness = private global i32 5 @hot_data_custom_bar_section = internal global i32 101 #0 [email protected]_var = internal global i32 123 +@qux = external global i64 define void @cold_func(i32 %0) !prof !15 { %2 = load i32, ptr @cold_bss diff --git a/llvm/test/Transforms/PGOProfile/data-access-profile.ll b/llvm/test/Transforms/PGOProfile/data-access-profile.ll index d9976a9d23755..15c1db739e4f4 100644 --- a/llvm/test/Transforms/PGOProfile/data-access-profile.ll +++ b/llvm/test/Transforms/PGOProfile/data-access-profile.ll @@ -31,8 +31,10 @@ ; LOG: Global variable var2.llvm.125 is annotated as hot ; LOG: Global variable bar is not annotated ; LOG: Global variable foo is annotated as unlikely -; LOG: Global variable var3 has explicit section name. Skip annotating. -; LOG: Global variable var4 has explicit section name. Skip annotating. +; LOG: Skip annotation for var3 due to explicit section name. +; LOG: Skip annotation for var4 due to explicit section name. +; LOG: Skip annotation for llvm.fake_var due to name starts with `llvm.`. +; LOG: Skip annotation for qux due to linker declaration. ;; String literals are not annotated. ; IR: @.str = unnamed_addr constant [5 x i8] c"abcde" @@ -54,6 +56,11 @@ ; IR-NEXT: @var3 = constant [2 x i32] [i32 12345, i32 6789], section "sec1" ; IR-NEXT: @var4 = constant [1 x i64] [i64 98765] #0 +; IR: @llvm.fake_var = global i32 123 +; IR-NOT: !section_prefix +; IR: @qux = external global i64 +; IR-NOT: !section_prefix + ; IR: attributes #0 = { "rodata-section"="sec2" } ; IR: !0 = !{!"section_prefix", !"hot"} @@ -80,6 +87,9 @@ DataAccessProfiles: AccessCount: 145 KnownColdSymbols: - foo + # The llvm.fake_var entry is added to test optimizer won't annotate it with section prefix. + # In real profiles, llvm.* variables may not have entries in the first place. + - llvm.fake_var KnownColdStrHashes: [ 999, 1001 ] ... ;--- memprof-no-dap.yaml @@ -112,11 +122,14 @@ target triple = "x86_64-unknown-linux-gnu" @foo = global i8 2 @var3 = constant [2 x i32][i32 12345, i32 6789], section "sec1" @var4 = constant [1 x i64][i64 98765] #0 [email protected]_var = global i32 123 +@qux = external global i64 define i32 @func() { %a = load i32, ptr @var1 %b = load i32, ptr @var2.llvm.125 - %ret = call i32 (...) @func_taking_arbitrary_param(i32 %a, i32 %b) + %c = load i32, ptr @llvm.fake_var + %ret = call i32 (...) @func_taking_arbitrary_param(i32 %a, i32 %b, i32 %c) ret i32 %ret } @@ -136,5 +149,8 @@ target triple = "x86_64-unknown-linux-gnu" @foo = global i8 2 @var3 = constant [2 x i32][i32 12345, i32 6789], section "sec1" @var4 = constant [1 x i64][i64 98765] #0 [email protected]_var = global i32 123 +@qux = external global i64 + attributes #0 = { "rodata-section"="sec2" } >From 905b80f0c4e8bf8c78dca9dcb0533284849104cf Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Wed, 8 Oct 2025 15:48:50 -0700 Subject: [PATCH 05/10] simplify test --- llvm/test/Transforms/PGOProfile/data-access-profile.ll | 3 --- 1 file changed, 3 deletions(-) diff --git a/llvm/test/Transforms/PGOProfile/data-access-profile.ll b/llvm/test/Transforms/PGOProfile/data-access-profile.ll index 15c1db739e4f4..205184bdd7156 100644 --- a/llvm/test/Transforms/PGOProfile/data-access-profile.ll +++ b/llvm/test/Transforms/PGOProfile/data-access-profile.ll @@ -87,9 +87,6 @@ DataAccessProfiles: AccessCount: 145 KnownColdSymbols: - foo - # The llvm.fake_var entry is added to test optimizer won't annotate it with section prefix. - # In real profiles, llvm.* variables may not have entries in the first place. - - llvm.fake_var KnownColdStrHashes: [ 999, 1001 ] ... ;--- memprof-no-dap.yaml >From 95292a62e7dbd09aecd3bf24b8a6c90cdd1dceda Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Wed, 8 Oct 2025 16:32:16 -0700 Subject: [PATCH 06/10] test coverage for the subsequent patch https://github.com/llvm/llvm-project/pull/162349 --- .../PGOProfile/data-access-profile.ll | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/llvm/test/Transforms/PGOProfile/data-access-profile.ll b/llvm/test/Transforms/PGOProfile/data-access-profile.ll index 205184bdd7156..d13dcf2f08a9e 100644 --- a/llvm/test/Transforms/PGOProfile/data-access-profile.ll +++ b/llvm/test/Transforms/PGOProfile/data-access-profile.ll @@ -16,6 +16,12 @@ ; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' -memprof-annotate-static-data-prefix \ ; RUN: -debug-only=memprof -stats -S input.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,IR,STAT +;; Tests that llc crashes on existing section prefixes. +; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' \ +; RUN: -memprof-annotate-static-data-prefix -S input.ll -o llc-input.ll +; RUN: not llc -mtriple=x86_64-unknown-linux-gnu -partition-static-data-sections=true llc-input.ll -o - 2>&1 | FileCheck %s --check-prefix=ERR +; ERR: Global variable var1 already has a section prefix hot + ;; Run memprof without providing memprof data. Test that IR has module flag ;; `EnableDataAccessProf` as 0. ; RUN: opt -passes='memprof-use<profile-filename=memprof-no-dap.profdata>' -memprof-annotate-static-data-prefix \ @@ -39,10 +45,10 @@ ;; String literals are not annotated. ; IR: @.str = unnamed_addr constant [5 x i8] c"abcde" ; IR-NOT: section_prefix -; IR: @var1 = global i32 123, !section_prefix !0 +; IR: @var1 = global i32 123, !section_prefix ![[NUM1:[0-9]+]] ;; @var.llvm.125 will be canonicalized to @var2 for profile look-up. -; IR-NEXT: @var2.llvm.125 = global i64 0, !section_prefix !0 +; IR-NEXT: @var2.llvm.125 = global i64 0, !section_prefix ![[NUM1]] ;; @bar is not seen in hot symbol or known symbol set, so it won't get a section ;; prefix. Test this by testing that there is no section_prefix between @bar and @@ -51,7 +57,7 @@ ; IR-NOT: !section_prefix ;; @foo is unlikely. -; IR-NEXT: @foo = global i8 2, !section_prefix !1 +; IR-NEXT: @foo = global i8 2, !section_prefix ![[NUM2:[0-9]+]] ; IR-NEXT: @var3 = constant [2 x i32] [i32 12345, i32 6789], section "sec1" ; IR-NEXT: @var4 = constant [1 x i64] [i64 98765] #0 @@ -63,9 +69,10 @@ ; IR: attributes #0 = { "rodata-section"="sec2" } -; IR: !0 = !{!"section_prefix", !"hot"} -; IR-NEXT: !1 = !{!"section_prefix", !"unlikely"} -; IR-NEXT: !2 = !{i32 2, !"EnableDataAccessProf", i32 1} +; IR: ![[NUM1]] = !{!"section_prefix", !"hot"} +; IR-NEXT: ![[NUM2]] = !{!"section_prefix", !"unlikely"} +;; Profile Summary module metadata can come in the middle here. +; IR: !{i32 2, !"EnableDataAccessProf", i32 1} ; FLAG: !{i32 2, !"EnableDataAccessProf", i32 0} ; FLAGLESS-NOT: EnableDataAccessProf @@ -134,6 +141,24 @@ declare i32 @func_taking_arbitrary_param(...) attributes #0 = { "rodata-section"="sec2" } +!llvm.module.flags = !{!1} + +!1 = !{i32 1, !"ProfileSummary", !2} +!2 = !{!3, !4, !5, !6, !7, !8, !9, !10} +!3 = !{!"ProfileFormat", !"InstrProf"} +!4 = !{!"TotalCount", i64 1460183} +!5 = !{!"MaxCount", i64 849024} +!6 = !{!"MaxInternalCount", i64 32769} +!7 = !{!"MaxFunctionCount", i64 849024} +!8 = !{!"NumCounts", i64 23627} +!9 = !{!"NumFunctions", i64 3271} +!10 = !{!"DetailedSummary", !11} +!11 = !{!12, !13} +!12 = !{i32 990000, i64 166, i32 73} +!13 = !{i32 999999, i64 3, i32 1443} +!14 = !{!"function_entry_count", i64 100000} +!15 = !{!"function_entry_count", i64 1} + ;--- funcless-module.ll target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" >From 64be09fca93a4b5ad865a2c3cab42281e127d121 Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Wed, 8 Oct 2025 16:46:28 -0700 Subject: [PATCH 07/10] Revert "test coverage for the subsequent patch https://github.com/llvm/llvm-project/pull/162349"; This reverts commit 95292a62e7dbd09aecd3bf24b8a6c90cdd1dceda. --- .../PGOProfile/data-access-profile.ll | 37 +++---------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/llvm/test/Transforms/PGOProfile/data-access-profile.ll b/llvm/test/Transforms/PGOProfile/data-access-profile.ll index d13dcf2f08a9e..205184bdd7156 100644 --- a/llvm/test/Transforms/PGOProfile/data-access-profile.ll +++ b/llvm/test/Transforms/PGOProfile/data-access-profile.ll @@ -16,12 +16,6 @@ ; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' -memprof-annotate-static-data-prefix \ ; RUN: -debug-only=memprof -stats -S input.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,IR,STAT -;; Tests that llc crashes on existing section prefixes. -; RUN: opt -passes='memprof-use<profile-filename=memprof.profdata>' \ -; RUN: -memprof-annotate-static-data-prefix -S input.ll -o llc-input.ll -; RUN: not llc -mtriple=x86_64-unknown-linux-gnu -partition-static-data-sections=true llc-input.ll -o - 2>&1 | FileCheck %s --check-prefix=ERR -; ERR: Global variable var1 already has a section prefix hot - ;; Run memprof without providing memprof data. Test that IR has module flag ;; `EnableDataAccessProf` as 0. ; RUN: opt -passes='memprof-use<profile-filename=memprof-no-dap.profdata>' -memprof-annotate-static-data-prefix \ @@ -45,10 +39,10 @@ ;; String literals are not annotated. ; IR: @.str = unnamed_addr constant [5 x i8] c"abcde" ; IR-NOT: section_prefix -; IR: @var1 = global i32 123, !section_prefix ![[NUM1:[0-9]+]] +; IR: @var1 = global i32 123, !section_prefix !0 ;; @var.llvm.125 will be canonicalized to @var2 for profile look-up. -; IR-NEXT: @var2.llvm.125 = global i64 0, !section_prefix ![[NUM1]] +; IR-NEXT: @var2.llvm.125 = global i64 0, !section_prefix !0 ;; @bar is not seen in hot symbol or known symbol set, so it won't get a section ;; prefix. Test this by testing that there is no section_prefix between @bar and @@ -57,7 +51,7 @@ ; IR-NOT: !section_prefix ;; @foo is unlikely. -; IR-NEXT: @foo = global i8 2, !section_prefix ![[NUM2:[0-9]+]] +; IR-NEXT: @foo = global i8 2, !section_prefix !1 ; IR-NEXT: @var3 = constant [2 x i32] [i32 12345, i32 6789], section "sec1" ; IR-NEXT: @var4 = constant [1 x i64] [i64 98765] #0 @@ -69,10 +63,9 @@ ; IR: attributes #0 = { "rodata-section"="sec2" } -; IR: ![[NUM1]] = !{!"section_prefix", !"hot"} -; IR-NEXT: ![[NUM2]] = !{!"section_prefix", !"unlikely"} -;; Profile Summary module metadata can come in the middle here. -; IR: !{i32 2, !"EnableDataAccessProf", i32 1} +; IR: !0 = !{!"section_prefix", !"hot"} +; IR-NEXT: !1 = !{!"section_prefix", !"unlikely"} +; IR-NEXT: !2 = !{i32 2, !"EnableDataAccessProf", i32 1} ; FLAG: !{i32 2, !"EnableDataAccessProf", i32 0} ; FLAGLESS-NOT: EnableDataAccessProf @@ -141,24 +134,6 @@ declare i32 @func_taking_arbitrary_param(...) attributes #0 = { "rodata-section"="sec2" } -!llvm.module.flags = !{!1} - -!1 = !{i32 1, !"ProfileSummary", !2} -!2 = !{!3, !4, !5, !6, !7, !8, !9, !10} -!3 = !{!"ProfileFormat", !"InstrProf"} -!4 = !{!"TotalCount", i64 1460183} -!5 = !{!"MaxCount", i64 849024} -!6 = !{!"MaxInternalCount", i64 32769} -!7 = !{!"MaxFunctionCount", i64 849024} -!8 = !{!"NumCounts", i64 23627} -!9 = !{!"NumFunctions", i64 3271} -!10 = !{!"DetailedSummary", !11} -!11 = !{!12, !13} -!12 = !{i32 990000, i64 166, i32 73} -!13 = !{i32 999999, i64 3, i32 1443} -!14 = !{!"function_entry_count", i64 100000} -!15 = !{!"function_entry_count", i64 1} - ;--- funcless-module.ll target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" >From 4a90c77c09ffff9f2c437eed857055621c97bb8e Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Wed, 8 Oct 2025 16:55:12 -0700 Subject: [PATCH 08/10] add a minial test case for subsequent PRs to expand on --- .../X86/global-variable-partition-with-dap.ll | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll diff --git a/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll b/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll new file mode 100644 index 0000000000000..42bbc0ad8f676 --- /dev/null +++ b/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll @@ -0,0 +1,42 @@ +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +;; A minimal test case. llc will crash if global variables already has a section +;; prefix. Subsequent PRs will expand on this test case to test the hotness +;; reconcillation implementation. +; RUN: not llc -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic \ +; RUN: -partition-static-data-sections=true \ +; RUN: -data-sections=true -unique-section-names=false \ +; RUN: %s -o - 2>&1 | FileCheck %s --check-prefix=ERR + +ERR: Global variable hot_bss already has a section prefix hot + +@hot_bss = internal global i32 0, !section_prefix !17 + +define void @hot_func() !prof !14 { + %9 = load i32, ptr @hot_bss + %11 = call i32 (...) @func_taking_arbitrary_param(i32 %9) + ret void +} + +declare i32 @func_taking_arbitrary_param(...) + +!llvm.module.flags = !{!1} + +!1 = !{i32 1, !"ProfileSummary", !2} +!2 = !{!3, !4, !5, !6, !7, !8, !9, !10} +!3 = !{!"ProfileFormat", !"InstrProf"} +!4 = !{!"TotalCount", i64 1460183} +!5 = !{!"MaxCount", i64 849024} +!6 = !{!"MaxInternalCount", i64 32769} +!7 = !{!"MaxFunctionCount", i64 849024} +!8 = !{!"NumCounts", i64 23627} +!9 = !{!"NumFunctions", i64 3271} +!10 = !{!"DetailedSummary", !11} +!11 = !{!12, !13} +!12 = !{i32 990000, i64 166, i32 73} +!13 = !{i32 999999, i64 3, i32 1443} +!14 = !{!"function_entry_count", i64 100000} +!15 = !{!"function_entry_count", i64 1} +!16 = !{!"branch_weights", i32 1, i32 99999} +!17 = !{!"section_prefix", !"hot"} >From f9c6266c55675190dfb1b43c763bbf0cb98bb2e4 Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Wed, 8 Oct 2025 17:15:09 -0700 Subject: [PATCH 09/10] one liner fix in test --- llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll b/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll index 42bbc0ad8f676..7214273ff6d9c 100644 --- a/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll +++ b/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll @@ -9,7 +9,7 @@ target triple = "x86_64-unknown-linux-gnu" ; RUN: -data-sections=true -unique-section-names=false \ ; RUN: %s -o - 2>&1 | FileCheck %s --check-prefix=ERR -ERR: Global variable hot_bss already has a section prefix hot +; ERR: Global variable hot_bss already has a section prefix hot @hot_bss = internal global i32 0, !section_prefix !17 >From 34b47c9852b8f8beef075e3b8dacdbb2110c9d1f Mon Sep 17 00:00:00 2001 From: mingmingl <[email protected]> Date: Fri, 10 Oct 2025 16:07:50 -0700 Subject: [PATCH 10/10] simplify switch-break to if-continue --- llvm/lib/Transforms/Instrumentation/MemProfUse.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp index a4a70077264ad..a6ec6c1207767 100644 --- a/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemProfUse.cpp @@ -826,10 +826,7 @@ bool MemProfUsePass::annotateGlobalVariables( assert(!GVar.getSectionPrefix().has_value() && "GVar shouldn't have section prefix yet"); auto Kind = llvm::memprof::getAnnotationKind(GVar); - switch (Kind) { - case llvm::memprof::AnnotationKind::AnnotationOK: - break; - default: + if (Kind != llvm::memprof::AnnotationKind::AnnotationOK) { HandleUnsupportedAnnotationKinds(GVar, Kind); continue; } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
