[StaticDataLayout] Reconcile data hotness based on PGO counters and data access profiles #163325
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mingmingl-llvm
force-pushed
the
users/mingmingl-llvm/patch
branch
from
1c81e88 to
ed74396
Compare
llvmbot
added
backend:X86
llvm:analysis
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-llvm-analysis Author: Mingming Liu (mingmingl-llvm) ChangesThis PR enhances the This is a follow-up patch of #162388 Full diff: https://github.com/llvm/llvm-project/pull/163325.diff 3 Files Affected:
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,@object
+; IR-NEXT: .section .bss.hot.,"aw"
+; IR: .type data_unknown_hotness,@object
+; IR-NEXT: .section .data,"aw"
+; IR: .type external_relro_array,@object
+; IR-NEXT: .section .data.rel.ro.unlikely.,"aw"
+
+
+; NODAP: .type hot_bss,@object
+; NODAP-NEXT: .section .bss.hot.,"aw"
+; NODAP: .type data_unknown_hotness,@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,@object # @external_relro_array
+; NODAP: .section .data.rel.ro,"aw"
+
+;--- input-with-dap-enabled.ll
+; Internal vars
+@hot_bss = internal global i32 0, !section_prefix !17
+@data_unknown_hotness = internal global i32 1
+; External vars
+@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
+@data_unknown_hotness = internal global i32 1
+; External vars
+@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"}
|
snehasish
approved these changes
Oct 14, 2025
|
|
||
| /// Return the hotness based on section prefix \p SectionPrefix. | ||
| LLVM_ABI StaticDataHotness | ||
| getSectionHotnessUsingDAP(std::optional<StringRef> SectionPrefix) const; |
There was a problem hiding this comment.
I see the DAP suffix used here and also in a few variable names. I wonder how we can make it easier for the reader to know that it's an abbreviation for DataAccessProfile?
There was a problem hiding this comment.
done by extending 'DAP' to DataAccessProfile or DataAccessProf. I'll think about where to mention 'DAP' to LLVM docs so the abbreviation name are widely known (like PGO/LTO, etc).
I'll revise the test name llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll in a follow-up patch.
| } // namespace llvm | ||
|
|
||
| #ifndef NDEBUG | ||
| static StringRef debugPrintSectionPrefix(StringRef Prefix) { |
There was a problem hiding this comment.
This is only used within one function, so maybe move it closer to where it's used and make it a function object?
There was a problem hiding this comment.
done by making this a lambda function inside the caller.
|
|
||
| // Both DAP and PGO counters are available. Use the hotter one. | ||
| auto HotnessFromPGO = getSectionHotnessUsingProfileCount(C, PSI, *Count); | ||
| StringRef Prefix = hotnessToStr(std::max(HotnessFromDAP, HotnessFromPGO)); |
There was a problem hiding this comment.
Using std::max on the enum values is a bit hard to reason about. Could we use a condition here?
There was a problem hiding this comment.
done!
#162388 (comment) makes enum class definition to be signed int. Since we use conditions here, the enum class definition could be signed or unsigned int. I'm open to both.
mingmingl-llvm
commented
Oct 14, 2025
| } // namespace llvm | ||
|
|
||
| #ifndef NDEBUG | ||
| static StringRef debugPrintSectionPrefix(StringRef Prefix) { |
There was a problem hiding this comment.
done by making this a lambda function inside the caller.
|
|
||
| /// Return the hotness based on section prefix \p SectionPrefix. | ||
| LLVM_ABI StaticDataHotness | ||
| getSectionHotnessUsingDAP(std::optional<StringRef> SectionPrefix) const; |
There was a problem hiding this comment.
done by extending 'DAP' to DataAccessProfile or DataAccessProf. I'll think about where to mention 'DAP' to LLVM docs so the abbreviation name are widely known (like PGO/LTO, etc).
I'll revise the test name llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll in a follow-up patch.
|
|
||
| // Both DAP and PGO counters are available. Use the hotter one. | ||
| auto HotnessFromPGO = getSectionHotnessUsingProfileCount(C, PSI, *Count); | ||
| StringRef Prefix = hotnessToStr(std::max(HotnessFromDAP, HotnessFromPGO)); |
There was a problem hiding this comment.
done!
#162388 (comment) makes enum class definition to be signed int. Since we use conditions here, the enum class definition could be signed or unsigned int. I'm open to both.
mingmingl-llvm
added a commit
that referenced
this pull request
Oct 15, 2025
…antSectionPrefix and extract analysis based on PGO-counter to be a helper function (#162388) `StaticDataProfileInfo::getConstantSectionPrefix` is used twice in codegen ([1] and [2]) to emit section prefix for constants. Before this patch, its implementation does analysis using PGO-counters, and PGO-counters are only available on module-internal constants. After this patch, the PGO-counter analysis are extracted to a helper function, and returns enum rather than StringPrefix. This way, the follow up patch #163325 can extend this function to use global variable section prefix and compute a max (the hotter one). [1] https://github.com/llvm/llvm-project/blob/975fba1b499422713e88cd6f374569f3bd38335e/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L3014-L3019 [2] https://github.com/llvm/llvm-project/blob/975fba1b499422713e88cd6f374569f3bd38335e/llvm/lib/CodeGen/StaticDataAnnotator.cpp#L77-L84
mingmingl-llvm
changed the base branch from
main
to
users/mingmingl-llvm/prepatch
mingmingl-llvm
changed the base branch from
users/mingmingl-llvm/prepatch
to
main
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 15, 2025
…o::getConstantSectionPrefix and extract analysis based on PGO-counter to be a helper function (#162388) `StaticDataProfileInfo::getConstantSectionPrefix` is used twice in codegen ([1] and [2]) to emit section prefix for constants. Before this patch, its implementation does analysis using PGO-counters, and PGO-counters are only available on module-internal constants. After this patch, the PGO-counter analysis are extracted to a helper function, and returns enum rather than StringPrefix. This way, the follow up patch llvm/llvm-project#163325 can extend this function to use global variable section prefix and compute a max (the hotter one). [1] https://github.com/llvm/llvm-project/blob/975fba1b499422713e88cd6f374569f3bd38335e/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L3014-L3019 [2] https://github.com/llvm/llvm-project/blob/975fba1b499422713e88cd6f374569f3bd38335e/llvm/lib/CodeGen/StaticDataAnnotator.cpp#L77-L84
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enhances the
StaticDataProfileInfo::getConstantSectionPrefixpass to reconcile data hotness information from both PGO counters and data access profiles. When both profiles are available for a global variable, the pass will now use the "hotter" of the two to determine the variable's section placement.This is a follow-up patch of #162388