Skip to content

[PGO] Fix incorrect count threshold calculation when 0% cutoff #117359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ken-matsui merged 1 commit into from
Feb 18, 2025
+17 −0
Merged

[PGO] Fix incorrect count threshold calculation when 0% cutoff #117359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions llvm/lib/ProfileData/ProfileSummaryBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,18 @@ static const uint32_t DefaultCutoffsData[] = {
const ArrayRef<uint32_t> ProfileSummaryBuilder::DefaultCutoffs =
DefaultCutoffsData;

// An entry for the 0th percentile to correctly calculate hot/cold count
// thresholds when -profile-summary-cutoff-hot/cold is 0. If the hot cutoff is
// 0, no sample counts are treated as hot. If the cold cutoff is 0, all sample
// counts are treated as cold. Assumes there is no UINT64_MAX sample counts.
Comment on lines +80 to +83
Copy link
Contributor

@ellishg ellishg Dec 5, 2024

Choose a reason for hiding this comment

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

Is this a typo? Or am I misunderstanding these flags?

Suggested change
// An entry for the 0th percentile to correctly calculate hot/cold count
// thresholds when -profile-summary-cutoff-hot/cold is 0. If the hot cutoff is
// 0, no sample counts are treated as hot. If the cold cutoff is 0, all sample
// counts are treated as cold. Assumes there is no UINT64_MAX sample counts.
// An entry for the 0th percentile to correctly calculate hot/cold count
// thresholds when -profile-summary-cutoff-hot/cold is 0. If the hot cutoff is
// 0, all sample counts are treated as hot. If the cold cutoff is 0, no sample
// counts are treated as cold. Assumes there is no UINT64_MAX sample counts.

Copy link
Member Author

@ken-matsui ken-matsui Dec 9, 2024

Choose a reason for hiding this comment

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

Actually, it is not a typo.

  • -profile-summary-cutoff-hot: A count is hot if it exceeds the minimum count to reach this percentile of total counts.
  • -profile-summary-cutoff-cold: A count is cold if it is below the minimum count to reach this percentile of total counts.

When cutoff is 0, the minimum count is UINT64_MAX. So, when a count exceeds UINT64_MAX, it is hot, meaning no counts are hot. Similarly, when a count is below UINT64_MAX, it is cold, meaning all counts are cold.

Copy link
Contributor

@ellishg ellishg Feb 13, 2025

Choose a reason for hiding this comment

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

It seems like this can be accomplished with -profile-summary-cutoff-hot=1000000. A function will be hot if its count is larger than 100% of functions, which is impossible. So no functions are hot. Likewise, -profile-summary-cutoff-cold=1000000 should make all functions cold. Why do we need a special case for these? With this patch, how do I specify -profile-summary-cutoff-hot to make all functions hot?

Copy link
Member Author

@ken-matsui ken-matsui Feb 17, 2025

Choose a reason for hiding this comment

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

No, -profile-summary-cutoff-hot=1000000 (unimplemented) doesn't mean that a function will be hot if its count is larger than 100% of functions. Instead, a function will be hot if its count is larger than a minimum count required to become 100% of functions, i.e., always.

With this patch, how do I specify -profile-summary-cutoff-hot to make all functions hot?

This patch isn't resolving the problem you are referring to. That requires a different patch to handle -profile-summary-cutoff-hot=1000000. This patch resolves an issue that 0% cutoff (to make all functions NOT hot) isn't supported.

Copy link
Contributor

@ellishg ellishg Feb 18, 2025

Choose a reason for hiding this comment

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

Thanks for the explanation and the clarification! LGTM

static const ProfileSummaryEntry ZeroCutoffEntry = {0, UINT64_MAX, 0};

const ProfileSummaryEntry &
ProfileSummaryBuilder::getEntryForPercentile(const SummaryEntryVector &DS,
uint64_t Percentile) {
if (Percentile == 0)
return ZeroCutoffEntry;

auto It = partition_point(DS, [=](const ProfileSummaryEntry &Entry) {
return Entry.Cutoff < Percentile;
});
Expand Down
8 changes: 8 additions & 0 deletions llvm/test/Analysis/ProfileSummary/basic.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@
; RUN: opt < %s -disable-output -profile-summary-hot-count=500 -passes=print-profile-summary -S 2>&1 | FileCheck %s -check-prefixes=OVERRIDE-HOT
; RUN: opt < %s -disable-output -profile-summary-cold-count=0 -passes=print-profile-summary -S 2>&1 | FileCheck %s -check-prefixes=OVERRIDE-COLD
; RUN: opt < %s -disable-output -profile-summary-cold-count=200 -profile-summary-hot-count=1000 -passes=print-profile-summary -S 2>&1 | FileCheck %s -check-prefixes=OVERRIDE-BOTH
; RUN: opt < %s -disable-output -profile-summary-cutoff-hot=0 -passes=print-profile-summary -S 2>&1 | FileCheck %s -check-prefixes=HOT-CUTOFF-0
; RUN: opt < %s -disable-output -profile-summary-cutoff-cold=0 -profile-summary-hot-count=18446744073709551615 -passes=print-profile-summary -S 2>&1 | FileCheck %s -check-prefixes=COLD-CUTOFF-0

define void @f1() !prof !20 {
; CHECK-LABEL: f1 :hot
; OVERRIDE-HOT-LABEL: f1
; OVERRIDE-COLD-LABEL: f1 :hot
; OVERRIDE-BOTH-LABEL: f1
; HOT-CUTOFF-0-LABEL: f1{{$}}
; COLD-CUTOFF-0-LABEL: f1 :cold

ret void
}
Expand All @@ -17,6 +21,8 @@ define void @f2() !prof !21 {
; OVERRIDE-HOT-LABEL: f2 :cold
; OVERRIDE-COLD-LABEL: f2
; OVERRIDE-BOTH-LABEL: f2
; HOT-CUTOFF-0-LABEL: f2 :cold
; COLD-CUTOFF-0-LABEL: f2 :cold

ret void
}
Expand All @@ -26,6 +32,8 @@ define void @f3() !prof !22 {
; OVERRIDE-HOT-LABEL: f3
; OVERRIDE-COLD-LABEL: f3
; OVERRIDE-BOTH-LABEL: f3
; HOT-CUTOFF-0-LABEL: f3{{$}}
; COLD-CUTOFF-0-LABEL: f3 :cold

ret void
}
Expand Down
Loading