Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions llvm/include/llvm/Analysis/StaticDataProfileInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -44,8 +47,29 @@ 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 hotness based on section prefix \p SectionPrefix.
LLVM_ABI StaticDataHotness
getSectionHotnessUsingDAP(std::optional<StringRef> SectionPrefix) const;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/// 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
Expand All @@ -54,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;
};
Expand Down
107 changes: 91 additions & 16 deletions llvm/lib/Analysis/StaticDataProfileInfo.cpp
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -46,6 +50,12 @@ bool IsAnnotationOK(const GlobalVariable &GV) {
} // namespace memprof
} // namespace llvm

#ifndef NDEBUG
static StringRef debugPrintSectionPrefix(StringRef Prefix) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used within one function, so maybe move it closer to where it's used and make it a function object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done by making this a lambda function inside the caller.

return Prefix.empty() ? "<empty>" : Prefix;
}
#endif

void StaticDataProfileInfo::addConstantProfileCount(
const Constant *C, std::optional<uint64_t> Count) {
if (!Count) {
Expand All @@ -60,6 +70,49 @@ 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;
}

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) {
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);
Expand All @@ -70,27 +123,49 @@ 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 (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));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using std::max on the enum values is a bit hard to reason about. Could we use a condition here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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 "";
// 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) {
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;
}

Expand Down
99 changes: 93 additions & 6 deletions llvm/test/CodeGen/X86/global-variable-partition-with-dap.ll
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"}
Expand All @@ -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"}