[llvm] [StaticDataLayout] Reconcile data hotness based on PGO counters and data access profiles (PR #163325)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 14 21:30:35 PDT 2025
https://github.com/mingmingl-llvm updated https://github.com/llvm/llvm-project/pull/163325
>From 3d70a00454023842e0806ddfeb0542369748636e Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 13 Oct 2025 16:44:07 -0700
Subject: [PATCH 1/4] squash commits
---
.../llvm/Analysis/StaticDataProfileInfo.h | 21 +++++++-
llvm/lib/Analysis/StaticDataProfileInfo.cpp | 48 +++++++++++++------
2 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
index f06e7ceaa74ce..70199a904f320 100644
--- a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
+++ b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
@@ -32,8 +32,11 @@ bool IsAnnotationOK(const GlobalVariable &GV);
/// profile information and provides methods to operate on them.
class StaticDataProfileInfo {
public:
- /// Accummulate the profile count of a constant that will be lowered to static
- /// data sections.
+ /// A constant is tracked only if the following conditions are met.
+ /// 1) It has local (i.e., private or internal) linkage.
+ // 2) Its data kind is one of {.rodata, .data, .bss, .data.rel.ro}.
+ // 3) It's eligible for section prefix annotation. See `AnnotationKind`
+ // above for ineligible reasons.
DenseMap<const Constant *, uint64_t> ConstantProfileCounts;
/// Keeps track of the constants that are seen at least once without profile
@@ -44,6 +47,20 @@ class StaticDataProfileInfo {
LLVM_ABI std::optional<uint64_t>
getConstantProfileCount(const Constant *C) const;
+ enum class StaticDataHotness : uint8_t {
+ Cold = 0,
+ LukewarmOrUnknown = 1,
+ Hot = 2,
+ };
+
+ /// Return the hotness of the constant \p C based on its profile count \p
+ /// Count.
+ LLVM_ABI StaticDataHotness getSectionHotnessUsingProfileCount(
+ const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const;
+
+ /// Return the string representation of the hotness enum \p Hotness.
+ LLVM_ABI StringRef hotnessToStr(StaticDataHotness Hotness) const;
+
public:
StaticDataProfileInfo() = default;
diff --git a/llvm/lib/Analysis/StaticDataProfileInfo.cpp b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
index 1f751ee5e09d9..27f8d216454aa 100644
--- a/llvm/lib/Analysis/StaticDataProfileInfo.cpp
+++ b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
@@ -60,6 +60,37 @@ void StaticDataProfileInfo::addConstantProfileCount(
OriginalCount = getInstrMaxCountValue();
}
+StaticDataProfileInfo::StaticDataHotness
+StaticDataProfileInfo::getSectionHotnessUsingProfileCount(
+ const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const {
+ // The accummulated counter shows the constant is hot. Return 'hot' whether
+ // this variable is seen by unprofiled functions or not.
+ if (PSI->isHotCount(Count))
+ return StaticDataHotness::Hot;
+ // The constant is not hot, and seen by unprofiled functions. We don't want to
+ // assign it to unlikely sections, even if the counter says 'cold'. So return
+ // an empty prefix before checking whether the counter is cold.
+ if (ConstantWithoutCounts.count(C))
+ return StaticDataHotness::LukewarmOrUnknown;
+ // The accummulated counter shows the constant is cold. Return 'unlikely'.
+ if (PSI->isColdCount(Count))
+ return StaticDataHotness::Cold;
+
+ return StaticDataHotness::LukewarmOrUnknown;
+}
+
+StringRef StaticDataProfileInfo::hotnessToStr(
+ StaticDataProfileInfo::StaticDataHotness Hotness) const {
+ switch (Hotness) {
+ case StaticDataProfileInfo::StaticDataHotness::Cold:
+ return "unlikely";
+ case StaticDataProfileInfo::StaticDataHotness::Hot:
+ return "hot";
+ default:
+ return "";
+ }
+}
+
std::optional<uint64_t>
StaticDataProfileInfo::getConstantProfileCount(const Constant *C) const {
auto I = ConstantProfileCounts.find(C);
@@ -70,23 +101,10 @@ StaticDataProfileInfo::getConstantProfileCount(const Constant *C) const {
StringRef StaticDataProfileInfo::getConstantSectionPrefix(
const Constant *C, const ProfileSummaryInfo *PSI) const {
- auto Count = getConstantProfileCount(C);
+ std::optional<uint64_t> Count = getConstantProfileCount(C);
if (!Count)
return "";
- // The accummulated counter shows the constant is hot. Return 'hot' whether
- // this variable is seen by unprofiled functions or not.
- if (PSI->isHotCount(*Count))
- return "hot";
- // The constant is not hot, and seen by unprofiled functions. We don't want to
- // assign it to unlikely sections, even if the counter says 'cold'. So return
- // an empty prefix before checking whether the counter is cold.
- if (ConstantWithoutCounts.count(C))
- return "";
- // The accummulated counter shows the constant is cold. Return 'unlikely'.
- if (PSI->isColdCount(*Count))
- return "unlikely";
- // The counter says lukewarm. Return an empty prefix.
- return "";
+ return hotnessToStr(getSectionHotnessUsingProfileCount(C, PSI, *Count));
}
bool StaticDataProfileInfoWrapperPass::doInitialization(Module &M) {
>From ed7439604e857aaf2a7732b7b04f1ceb575036de Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Mon, 13 Oct 2025 21:22:20 -0700
Subject: [PATCH 2/4] reconcile hotness
---
.../llvm/Analysis/StaticDataProfileInfo.h | 21 ++--
llvm/lib/Analysis/StaticDataProfileInfo.cpp | 59 ++++++++++-
.../X86/global-variable-partition-with-dap.ll | 99 +++++++++++++++++--
3 files changed, 163 insertions(+), 16 deletions(-)
diff --git a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
index 70199a904f320..4a97e3370e451 100644
--- a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
+++ b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
@@ -58,11 +58,18 @@ class StaticDataProfileInfo {
LLVM_ABI StaticDataHotness getSectionHotnessUsingProfileCount(
const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const;
+ /// Return the hotness based on section prefix \p SectionPrefix.
+ LLVM_ABI StaticDataHotness
+ getSectionHotnessUsingDAP(std::optional<StringRef> SectionPrefix) const;
+
/// Return the string representation of the hotness enum \p Hotness.
LLVM_ABI StringRef hotnessToStr(StaticDataHotness Hotness) const;
+ bool EnableDataAccessProf = false;
+
public:
- StaticDataProfileInfo() = default;
+ StaticDataProfileInfo(bool EnableDataAccessProf)
+ : EnableDataAccessProf(EnableDataAccessProf) {}
/// If \p Count is not nullopt, add it to the profile count of the constant \p
/// C in a saturating way, and clamp the count to \p getInstrMaxCountValue if
@@ -71,14 +78,10 @@ class StaticDataProfileInfo {
LLVM_ABI void addConstantProfileCount(const Constant *C,
std::optional<uint64_t> Count);
- /// Return a section prefix for the constant \p C based on its profile count.
- /// - If a constant doesn't have a counter, return an empty string.
- /// - Otherwise,
- /// - If it has a hot count, return "hot".
- /// - If it is seen by unprofiled function, return an empty string.
- /// - If it has a cold count, return "unlikely".
- /// - Otherwise (e.g. it's used by lukewarm functions), return an empty
- /// string.
+ /// Given a constant \p C, returns a section prefix.
+ /// If \p C is a global variable, the section prefix is the bigger one
+ /// between its existing section prefix and its use profile count. Otherwise,
+ /// the section prefix is based on its use profile count.
LLVM_ABI StringRef getConstantSectionPrefix(
const Constant *C, const ProfileSummaryInfo *PSI) const;
};
diff --git a/llvm/lib/Analysis/StaticDataProfileInfo.cpp b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
index 27f8d216454aa..61e1e6779a2b0 100644
--- a/llvm/lib/Analysis/StaticDataProfileInfo.cpp
+++ b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
@@ -1,10 +1,14 @@
#include "llvm/Analysis/StaticDataProfileInfo.h"
#include "llvm/Analysis/ProfileSummaryInfo.h"
#include "llvm/IR/Constant.h"
+#include "llvm/IR/Constants.h"
#include "llvm/IR/GlobalVariable.h"
+#include "llvm/IR/Module.h"
#include "llvm/InitializePasses.h"
#include "llvm/ProfileData/InstrProf.h"
+#define DEBUG_TYPE "static-data-profile-info"
+
using namespace llvm;
namespace llvm {
@@ -46,6 +50,12 @@ bool IsAnnotationOK(const GlobalVariable &GV) {
} // namespace memprof
} // namespace llvm
+#ifndef NDEBUG
+static StringRef debugPrintSectionPrefix(StringRef Prefix) {
+ return Prefix.empty() ? "<empty>" : Prefix;
+}
+#endif
+
void StaticDataProfileInfo::addConstantProfileCount(
const Constant *C, std::optional<uint64_t> Count) {
if (!Count) {
@@ -79,6 +89,18 @@ StaticDataProfileInfo::getSectionHotnessUsingProfileCount(
return StaticDataHotness::LukewarmOrUnknown;
}
+StaticDataProfileInfo::StaticDataHotness
+StaticDataProfileInfo::getSectionHotnessUsingDAP(
+ std::optional<StringRef> MaybeSectionPrefix) const {
+ if (!MaybeSectionPrefix)
+ return StaticDataProfileInfo::StaticDataHotness::LukewarmOrUnknown;
+ StringRef Prefix = *MaybeSectionPrefix;
+ assert((Prefix == "hot" || Prefix == "unlikely") &&
+ "Expect section_prefix to be one of hot or unlikely");
+ return Prefix == "hot" ? StaticDataProfileInfo::StaticDataHotness::Hot
+ : StaticDataProfileInfo::StaticDataHotness::Cold;
+}
+
StringRef StaticDataProfileInfo::hotnessToStr(
StaticDataProfileInfo::StaticDataHotness Hotness) const {
switch (Hotness) {
@@ -102,13 +124,48 @@ StaticDataProfileInfo::getConstantProfileCount(const Constant *C) const {
StringRef StaticDataProfileInfo::getConstantSectionPrefix(
const Constant *C, const ProfileSummaryInfo *PSI) const {
std::optional<uint64_t> Count = getConstantProfileCount(C);
+
+ if (EnableDataAccessProf) {
+ // Module flag `HasDataAccessProf` is 1 -> empty section prefix means
+ // unknown hotness except for string literals.
+ if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(C);
+ GV && llvm::memprof::IsAnnotationOK(*GV) &&
+ !GV->getName().starts_with(".str")) {
+ auto HotnessFromDAP = getSectionHotnessUsingDAP(GV->getSectionPrefix());
+
+ if (!Count) {
+ StringRef Prefix = hotnessToStr(HotnessFromDAP);
+ LLVM_DEBUG(dbgs() << GV->getName() << "has section prefix "
+ << debugPrintSectionPrefix(Prefix)
+ << ", solely from data access profiles\n");
+ return Prefix;
+ }
+
+ // Both DAP and PGO counters are available. Use the hotter one.
+ auto HotnessFromPGO = getSectionHotnessUsingProfileCount(C, PSI, *Count);
+ StringRef Prefix = hotnessToStr(std::max(HotnessFromDAP, HotnessFromPGO));
+ LLVM_DEBUG(dbgs() << GV->getName() << " has section prefix "
+ << debugPrintSectionPrefix(Prefix)
+ << ", the max from DAP as "
+ << debugPrintSectionPrefix(hotnessToStr(HotnessFromDAP))
+ << " and PGO counters as "
+ << debugPrintSectionPrefix(hotnessToStr(HotnessFromPGO))
+ << "\n");
+ return Prefix;
+ }
+ }
if (!Count)
return "";
+
return hotnessToStr(getSectionHotnessUsingProfileCount(C, PSI, *Count));
}
bool StaticDataProfileInfoWrapperPass::doInitialization(Module &M) {
- Info.reset(new StaticDataProfileInfo());
+ bool EnableDataAccessProf = false;
+ if (auto *MD = mdconst::extract_or_null<ConstantInt>(
+ M.getModuleFlag("EnableDataAccessProf")))
+ EnableDataAccessProf = MD->getZExtValue();
+ Info.reset(new StaticDataProfileInfo(EnableDataAccessProf));
return false;
}
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 f3950b75a969f..2d3cbaf345f3e 100644
--- a/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll
+++ b/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll
@@ -1,17 +1,102 @@
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. Subsequent PRs will expand on this test case
-;; (e.g., with more functions, variables and profiles) and test the hotness
-;; reconcillation implementation.
+;; Requires asserts for -debug-only.
+; REQUIRES: asserts
+
+; RUN: rm -rf %t && split-file %s %t && cd %t
+
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic \
+; RUN: -partition-static-data-sections=true \
+; RUN: -debug-only=static-data-profile-info \
+; RUN: -data-sections=true -unique-section-names=false \
+; RUN: input-with-dap-enabled.ll -o - 2>&1 | FileCheck %s --check-prefixes=LOG,IR
+
; RUN: llc -mtriple=x86_64-unknown-linux-gnu -relocation-model=pic \
; RUN: -partition-static-data-sections=true \
+; RUN: -debug-only=static-data-profile-info \
; RUN: -data-sections=true -unique-section-names=false \
-; RUN: %s -o - 2>&1 | FileCheck %s --check-prefix=IR
+; RUN: input-without-dap.ll -o - 2>&1 | FileCheck %s --check-prefixes=NODAP
+
+; LOG: hot_bss has section prefix hot, the max from DAP as hot and PGO counters as hot
+; LOG: data_unknown_hotness has section prefix <empty>, the max from DAP as <empty> and PGO counters as unlikely
+; LOG: external_relro_arrayhas section prefix unlikely, solely from data access profiles
+
+; IR: .type hot_bss, at object
+; IR-NEXT: .section .bss.hot.,"aw"
+; IR: .type data_unknown_hotness, at object
+; IR-NEXT: .section .data,"aw"
+; IR: .type external_relro_array, at object
+; IR-NEXT: .section .data.rel.ro.unlikely.,"aw"
+
+
+; NODAP: .type hot_bss, at object
+; NODAP-NEXT: .section .bss.hot.,"aw"
+; NODAP: .type data_unknown_hotness, at object
+; NODAP-NEXT: .section .data.unlikely.,"aw"
+;; Global variable section prefix metadata is not used when
+;; module flag `EnableDataAccessProf` is 0, and @external_relro_array has
+;; external linkage, so analysis based on PGO counters doesn't apply.
+; NODAP: .type external_relro_array, at object # @external_relro_array
+; NODAP: .section .data.rel.ro,"aw"
+
+;--- input-with-dap-enabled.ll
+; Internal vars
+ at hot_bss = internal global i32 0, !section_prefix !17
+ at data_unknown_hotness = internal global i32 1
+; External vars
+ at external_relro_array = constant [2 x ptr] [ptr @hot_bss, ptr @data_unknown_hotness], !section_prefix !18
+
+define void @cold_func() !prof !15 {
+ %9 = load i32, ptr @data_unknown_hotness
+ %11 = call i32 (...) @func_taking_arbitrary_param(i32 %9)
+ ret void
+}
+
+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(...)
-; IR: .section .bss.hot.,"aw"
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 2, !"EnableDataAccessProf", i32 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"}
+!18 = !{!"section_prefix", !"unlikely"}
+
+;--- input-without-dap.ll
+; Same as `input-with-dap-enabled.ll` above except that module flag
+; `EnableDataAccessProf` has value 0.
+; Internal vars
@hot_bss = internal global i32 0, !section_prefix !17
+ at data_unknown_hotness = internal global i32 1
+; External vars
+ at external_relro_array = constant [2 x ptr] [ptr @hot_bss, ptr @data_unknown_hotness], !section_prefix !18
+
+define void @cold_func() !prof !15 {
+ %9 = load i32, ptr @data_unknown_hotness
+ %11 = call i32 (...) @func_taking_arbitrary_param(i32 %9)
+ ret void
+}
define void @hot_func() !prof !14 {
%9 = load i32, ptr @hot_bss
@@ -21,8 +106,9 @@ define void @hot_func() !prof !14 {
declare i32 @func_taking_arbitrary_param(...)
-!llvm.module.flags = !{!1}
+!llvm.module.flags = !{!0, !1}
+!0 = !{i32 2, !"EnableDataAccessProf", i32 0}
!1 = !{i32 1, !"ProfileSummary", !2}
!2 = !{!3, !4, !5, !6, !7, !8, !9, !10}
!3 = !{!"ProfileFormat", !"InstrProf"}
@@ -40,3 +126,4 @@ declare i32 @func_taking_arbitrary_param(...)
!15 = !{!"function_entry_count", i64 1}
!16 = !{!"branch_weights", i32 1, i32 99999}
!17 = !{!"section_prefix", !"hot"}
+!18 = !{!"section_prefix", !"unlikely"}
>From 433f716ed45cefbd17834fc85e70d4e1645d3fd1 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 14 Oct 2025 16:10:52 -0700
Subject: [PATCH 3/4] resolve comments
---
.../llvm/Analysis/StaticDataProfileInfo.h | 12 +++++++-----
llvm/lib/Analysis/StaticDataProfileInfo.cpp | 19 +++++++++----------
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
index 70199a904f320..bb7f3be2e8d90 100644
--- a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
+++ b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
@@ -47,15 +47,17 @@ class StaticDataProfileInfo {
LLVM_ABI std::optional<uint64_t>
getConstantProfileCount(const Constant *C) const;
- enum class StaticDataHotness : uint8_t {
- Cold = 0,
- LukewarmOrUnknown = 1,
- Hot = 2,
+ /// Use signed enums for enum value comparison, and make 'LukewarmOrUnknown'
+ /// as 0 so any accidentally uninitialized value will default to unknown.
+ enum class StaticDataHotness : int8_t {
+ Cold = -1,
+ LukewarmOrUnknown = 0,
+ Hot = 1,
};
/// Return the hotness of the constant \p C based on its profile count \p
/// Count.
- LLVM_ABI StaticDataHotness getSectionHotnessUsingProfileCount(
+ LLVM_ABI StaticDataHotness getConstantHotnessUsingProfileCount(
const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const;
/// Return the string representation of the hotness enum \p Hotness.
diff --git a/llvm/lib/Analysis/StaticDataProfileInfo.cpp b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
index 27f8d216454aa..e7f0b2c15d604 100644
--- a/llvm/lib/Analysis/StaticDataProfileInfo.cpp
+++ b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
@@ -61,30 +61,29 @@ void StaticDataProfileInfo::addConstantProfileCount(
}
StaticDataProfileInfo::StaticDataHotness
-StaticDataProfileInfo::getSectionHotnessUsingProfileCount(
+StaticDataProfileInfo::getConstantHotnessUsingProfileCount(
const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const {
- // The accummulated counter shows the constant is hot. Return 'hot' whether
- // this variable is seen by unprofiled functions or not.
+ // The accummulated counter shows the constant is hot. Return enum 'hot'
+ // whether this variable is seen by unprofiled functions or not.
if (PSI->isHotCount(Count))
return StaticDataHotness::Hot;
// The constant is not hot, and seen by unprofiled functions. We don't want to
// assign it to unlikely sections, even if the counter says 'cold'. So return
- // an empty prefix before checking whether the counter is cold.
+ // enum 'LukewarmOrUnknown'.
if (ConstantWithoutCounts.count(C))
return StaticDataHotness::LukewarmOrUnknown;
- // The accummulated counter shows the constant is cold. Return 'unlikely'.
+ // The accummulated counter shows the constant is cold so return enum 'cold'.
if (PSI->isColdCount(Count))
return StaticDataHotness::Cold;
return StaticDataHotness::LukewarmOrUnknown;
}
-StringRef StaticDataProfileInfo::hotnessToStr(
- StaticDataProfileInfo::StaticDataHotness Hotness) const {
+StringRef StaticDataProfileInfo::hotnessToStr(StaticDataHotness Hotness) const {
switch (Hotness) {
- case StaticDataProfileInfo::StaticDataHotness::Cold:
+ case StaticDataHotness::Cold:
return "unlikely";
- case StaticDataProfileInfo::StaticDataHotness::Hot:
+ case StaticDataHotness::Hot:
return "hot";
default:
return "";
@@ -104,7 +103,7 @@ StringRef StaticDataProfileInfo::getConstantSectionPrefix(
std::optional<uint64_t> Count = getConstantProfileCount(C);
if (!Count)
return "";
- return hotnessToStr(getSectionHotnessUsingProfileCount(C, PSI, *Count));
+ return hotnessToStr(getConstantHotnessUsingProfileCount(C, PSI, *Count));
}
bool StaticDataProfileInfoWrapperPass::doInitialization(Module &M) {
>From 5031e1becc8343ecd0c49384d6034a26d336cb52 Mon Sep 17 00:00:00 2001
From: mingmingl <mingmingl at google.com>
Date: Tue, 14 Oct 2025 16:35:30 -0700
Subject: [PATCH 4/4] resolve review comments
---
.../llvm/Analysis/StaticDataProfileInfo.h | 4 +-
llvm/lib/Analysis/StaticDataProfileInfo.cpp | 53 ++++++++++++-------
.../X86/global-variable-partition-with-dap.ll | 6 +--
3 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
index 4a97e3370e451..dfdab47428352 100644
--- a/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
+++ b/llvm/include/llvm/Analysis/StaticDataProfileInfo.h
@@ -59,8 +59,8 @@ class StaticDataProfileInfo {
const Constant *C, const ProfileSummaryInfo *PSI, uint64_t Count) const;
/// Return the hotness based on section prefix \p SectionPrefix.
- LLVM_ABI StaticDataHotness
- getSectionHotnessUsingDAP(std::optional<StringRef> SectionPrefix) const;
+ LLVM_ABI StaticDataHotness getSectionHotnessUsingDataAccessProfile(
+ std::optional<StringRef> SectionPrefix) const;
/// Return the string representation of the hotness enum \p Hotness.
LLVM_ABI StringRef hotnessToStr(StaticDataHotness Hotness) const;
diff --git a/llvm/lib/Analysis/StaticDataProfileInfo.cpp b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
index 61e1e6779a2b0..3ece3913df66f 100644
--- a/llvm/lib/Analysis/StaticDataProfileInfo.cpp
+++ b/llvm/lib/Analysis/StaticDataProfileInfo.cpp
@@ -50,12 +50,6 @@ bool IsAnnotationOK(const GlobalVariable &GV) {
} // namespace memprof
} // namespace llvm
-#ifndef NDEBUG
-static StringRef debugPrintSectionPrefix(StringRef Prefix) {
- return Prefix.empty() ? "<empty>" : Prefix;
-}
-#endif
-
void StaticDataProfileInfo::addConstantProfileCount(
const Constant *C, std::optional<uint64_t> Count) {
if (!Count) {
@@ -90,7 +84,7 @@ StaticDataProfileInfo::getSectionHotnessUsingProfileCount(
}
StaticDataProfileInfo::StaticDataHotness
-StaticDataProfileInfo::getSectionHotnessUsingDAP(
+StaticDataProfileInfo::getSectionHotnessUsingDataAccessProfile(
std::optional<StringRef> MaybeSectionPrefix) const {
if (!MaybeSectionPrefix)
return StaticDataProfileInfo::StaticDataHotness::LukewarmOrUnknown;
@@ -125,32 +119,51 @@ StringRef StaticDataProfileInfo::getConstantSectionPrefix(
const Constant *C, const ProfileSummaryInfo *PSI) const {
std::optional<uint64_t> Count = getConstantProfileCount(C);
+#ifndef NDEBUG
+ auto DbgPrintPrefix = [](StringRef Prefix) {
+ return Prefix.empty() ? "<empty>" : Prefix;
+ };
+#endif
+
if (EnableDataAccessProf) {
// Module flag `HasDataAccessProf` is 1 -> empty section prefix means
// unknown hotness except for string literals.
if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(C);
GV && llvm::memprof::IsAnnotationOK(*GV) &&
!GV->getName().starts_with(".str")) {
- auto HotnessFromDAP = getSectionHotnessUsingDAP(GV->getSectionPrefix());
+ auto HotnessFromDataAccessProf =
+ getSectionHotnessUsingDataAccessProfile(GV->getSectionPrefix());
if (!Count) {
- StringRef Prefix = hotnessToStr(HotnessFromDAP);
- LLVM_DEBUG(dbgs() << GV->getName() << "has section prefix "
- << debugPrintSectionPrefix(Prefix)
+ StringRef Prefix = hotnessToStr(HotnessFromDataAccessProf);
+ LLVM_DEBUG(dbgs() << GV->getName() << " has section prefix "
+ << DbgPrintPrefix(Prefix)
<< ", solely from data access profiles\n");
return Prefix;
}
- // Both DAP and PGO counters are available. Use the hotter one.
+ // Both data access profiles and PGO counters are available. Use the
+ // hotter one.
auto HotnessFromPGO = getSectionHotnessUsingProfileCount(C, PSI, *Count);
- StringRef Prefix = hotnessToStr(std::max(HotnessFromDAP, HotnessFromPGO));
- LLVM_DEBUG(dbgs() << GV->getName() << " has section prefix "
- << debugPrintSectionPrefix(Prefix)
- << ", the max from DAP as "
- << debugPrintSectionPrefix(hotnessToStr(HotnessFromDAP))
- << " and PGO counters as "
- << debugPrintSectionPrefix(hotnessToStr(HotnessFromPGO))
- << "\n");
+ StaticDataHotness GlobalVarHotness = StaticDataHotness::LukewarmOrUnknown;
+ if (HotnessFromDataAccessProf == StaticDataHotness::Hot ||
+ HotnessFromPGO == StaticDataHotness::Hot) {
+ GlobalVarHotness = StaticDataHotness::Hot;
+ } else if (HotnessFromDataAccessProf ==
+ StaticDataHotness::LukewarmOrUnknown ||
+ HotnessFromPGO == StaticDataHotness::LukewarmOrUnknown) {
+ GlobalVarHotness = StaticDataHotness::LukewarmOrUnknown;
+ } else {
+ GlobalVarHotness = StaticDataHotness::Cold;
+ }
+ StringRef Prefix = hotnessToStr(GlobalVarHotness);
+ LLVM_DEBUG(
+ dbgs() << GV->getName() << " has section prefix "
+ << DbgPrintPrefix(Prefix)
+ << ", the max from data access profiles as "
+ << DbgPrintPrefix(hotnessToStr(HotnessFromDataAccessProf))
+ << " and PGO counters as "
+ << DbgPrintPrefix(hotnessToStr(HotnessFromPGO)) << "\n");
return Prefix;
}
}
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 2d3cbaf345f3e..edf05e74f1f50 100644
--- a/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll
+++ b/llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll
@@ -18,9 +18,9 @@ target triple = "x86_64-unknown-linux-gnu"
; RUN: -data-sections=true -unique-section-names=false \
; RUN: input-without-dap.ll -o - 2>&1 | FileCheck %s --check-prefixes=NODAP
-; LOG: hot_bss has section prefix hot, the max from DAP as hot and PGO counters as hot
-; LOG: data_unknown_hotness has section prefix <empty>, the max from DAP as <empty> and PGO counters as unlikely
-; LOG: external_relro_arrayhas section prefix unlikely, solely from data access profiles
+; LOG: hot_bss has section prefix hot, the max from data access profiles as hot and PGO counters as hot
+; LOG: data_unknown_hotness has section prefix <empty>, the max from data access profiles as <empty> and PGO counters as unlikely
+; LOG: external_relro_array has section prefix unlikely, solely from data access profiles
; IR: .type hot_bss, at object
; IR-NEXT: .section .bss.hot.,"aw"
More information about the llvm-commits
mailing list