[llvm] [llvm-profgen] Filter out ambiguous cold profiles during profile generation (PR #81803)
Lei Wang via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 15 13:09:14 PST 2024
https://github.com/wlei-llvm updated https://github.com/llvm/llvm-project/pull/81803
>From 5ff2685aee340f202a10c6feffc3104599f322ee Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Wed, 14 Feb 2024 14:33:36 -0800
Subject: [PATCH 1/2] [llvm-profgen] Filter out ambiguous profiles during
profile generation
---
.../Inputs/filter-ambiguous-profile.prof | 16 ++++++++++
.../filter-ambiguous-profile.test | 8 +++++
llvm/tools/llvm-profgen/ProfileGenerator.cpp | 29 +++++++++++++++++++
llvm/tools/llvm-profgen/ProfileGenerator.h | 11 +++++++
4 files changed, 64 insertions(+)
create mode 100644 llvm/test/tools/llvm-profgen/Inputs/filter-ambiguous-profile.prof
create mode 100644 llvm/test/tools/llvm-profgen/filter-ambiguous-profile.test
diff --git a/llvm/test/tools/llvm-profgen/Inputs/filter-ambiguous-profile.prof b/llvm/test/tools/llvm-profgen/Inputs/filter-ambiguous-profile.prof
new file mode 100644
index 00000000000000..a63d151e247c75
--- /dev/null
+++ b/llvm/test/tools/llvm-profgen/Inputs/filter-ambiguous-profile.prof
@@ -0,0 +1,16 @@
+foo:12345:1000
+ 1: 1000
+ 4: bar:1000
+ 1: 1000
+ 2: __tls_init.llvm.123:1
+ 1: 1
+ 3: goo:300
+ 1: 300
+ 8: __cxx_global_var_init.4:4
+ 1: 1
+ 2: goo:3
+ 1: 3
+__cxx_global_var_init.1:1:1
+ 1: 1
+__tls_init.llvm.345:1:1
+ 1: 1
diff --git a/llvm/test/tools/llvm-profgen/filter-ambiguous-profile.test b/llvm/test/tools/llvm-profgen/filter-ambiguous-profile.test
new file mode 100644
index 00000000000000..3a264e3b1108bf
--- /dev/null
+++ b/llvm/test/tools/llvm-profgen/filter-ambiguous-profile.test
@@ -0,0 +1,8 @@
+; RUN: llvm-profgen --format=text --llvm-sample-profile=%S/Inputs/filter-ambiguous-profile.prof --binary=%S/Inputs/inline-cs-noprobe.perfbin --csspgo-preinliner=0 --output=%t1 || FileCheck %s --input-file %t1
+
+;CHECK: foo:12345:1000
+;CHECK-NEXT 1: 1000
+;CHECK-NEXT 4: bar:1000
+;CHECK-NEXT 1: 1000
+;CHECK-NEXT 3: goo:300
+;CHECK-NEXT 1: 300
diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
index c4028e6b132871..4bca8d03f904d7 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
@@ -196,6 +196,33 @@ void ProfileGeneratorBase::showDensitySuggestion(double Density) {
<< "% total samples: " << format("%.1f", Density) << "\n";
}
+bool ProfileGeneratorBase::filterAmbiguousProfile(FunctionSamples &FS) {
+ for (const auto &Prefix : FuncPrefixsToFilter) {
+ if (FS.getFuncName().starts_with(Prefix))
+ return true;
+ }
+
+ // Filter the function profiles for the inlinees.
+ for (auto &Callees :
+ const_cast<CallsiteSampleMap &>(FS.getCallsiteSamples())) {
+ auto &CalleesMap = Callees.second;
+ for (auto I = CalleesMap.begin(); I != CalleesMap.end();) {
+ auto Tmp = I++;
+ if (filterAmbiguousProfile(Tmp->second))
+ CalleesMap.erase(Tmp);
+ }
+ }
+ return false;
+}
+
+void ProfileGeneratorBase::filterAmbiguousProfile(SampleProfileMap &Profiles) {
+ for (auto I = ProfileMap.begin(); I != ProfileMap.end();) {
+ auto Tmp = I++;
+ if (filterAmbiguousProfile(Tmp->second))
+ ProfileMap.erase(Tmp);
+ }
+}
+
double ProfileGeneratorBase::calculateDensity(const SampleProfileMap &Profiles,
uint64_t HotCntThreshold) {
double Density = DBL_MAX;
@@ -491,6 +518,7 @@ void ProfileGenerator::generateProfile() {
void ProfileGenerator::postProcessProfiles() {
computeSummaryAndThreshold(ProfileMap);
trimColdProfiles(ProfileMap, ColdCountThreshold);
+ filterAmbiguousProfile(ProfileMap);
calculateAndShowDensity(ProfileMap);
}
@@ -1024,6 +1052,7 @@ void CSProfileGenerator::postProcessProfiles() {
CSConverter.convertCSProfiles();
FunctionSamples::ProfileIsCS = false;
}
+ filterAmbiguousProfile(ProfileMap);
}
void ProfileGeneratorBase::computeSummaryAndThreshold(
diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.h b/llvm/tools/llvm-profgen/ProfileGenerator.h
index 88cf2dc0d49f37..86eefbaea2bec1 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.h
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.h
@@ -108,6 +108,10 @@ class ProfileGeneratorBase {
void updateCallsiteSamples();
+ void filterAmbiguousProfile(SampleProfileMap &Profiles);
+
+ bool filterAmbiguousProfile(FunctionSamples &FS);
+
StringRef getCalleeNameForAddress(uint64_t TargetAddress);
void computeSummaryAndThreshold(SampleProfileMap &ProfileMap);
@@ -128,6 +132,13 @@ class ProfileGeneratorBase {
virtual bool collectFunctionsFromLLVMProfile(
std::unordered_set<const BinaryFunction *> &ProfiledFunctions) = 0;
+ // Those functions are usually cold but could have multiple versions, the
+ // profile are either from one version or the merged version, which is
+ // ambiguous. We can't attribute the profile for each version accurately, so
+ // filter them out from the profile map.
+ static constexpr const char *FuncPrefixsToFilter[] = {"__cxx_global_var_init",
+ "__tls_init"};
+
// Thresholds from profile summary to answer isHotCount/isColdCount queries.
uint64_t HotCountThreshold;
>From aeb41eb980e34e0345f4fc898cbe17805ad288bc Mon Sep 17 00:00:00 2001
From: wlei <wlei at fb.com>
Date: Thu, 15 Feb 2024 13:05:16 -0800
Subject: [PATCH 2/2] add more comments
---
llvm/tools/llvm-profgen/ProfileGenerator.cpp | 29 +++++++++++++++-----
llvm/tools/llvm-profgen/ProfileGenerator.h | 5 +---
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.cpp b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
index 4bca8d03f904d7..5aa44108f96605 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
@@ -202,24 +202,39 @@ bool ProfileGeneratorBase::filterAmbiguousProfile(FunctionSamples &FS) {
return true;
}
- // Filter the function profiles for the inlinees.
+ // Filter the function profiles for the inlinees. It's useful for fuzzy
+ // profile matching which flattens the profile and inlinees' samples are
+ // merged into top-level function.
for (auto &Callees :
const_cast<CallsiteSampleMap &>(FS.getCallsiteSamples())) {
auto &CalleesMap = Callees.second;
for (auto I = CalleesMap.begin(); I != CalleesMap.end();) {
- auto Tmp = I++;
- if (filterAmbiguousProfile(Tmp->second))
- CalleesMap.erase(Tmp);
+ auto FS = I++;
+ if (filterAmbiguousProfile(FS->second))
+ CalleesMap.erase(FS);
}
}
return false;
}
+// For built-in local initialization function such as __cxx_global_var_init,
+// __tls_init prefix function, there could be multiple versions of the functions
+// in the final binary. However, in the profile generation, we call
+// getCanonicalFnName to canonicalize the names which strips the suffixes.
+// Therefore, samples from different functions queries the same profile and the
+// samples are merged. As the functions are essentially different, entries of
+// the merged profile are ambiguous. In sample loader, the IR from one version
+// would be attributed towards a merged entries, which is inaccurate. Especially
+// for fuzzy profile matching, it gets multiple callsites(from different
+// function) but used to match one callsite, which misleads the matching and
+// causes a lot of false positives report. Hence, we want to filter them out
+// from the profile map during the profile generation time. The profiles are all
+// cold functions, it won't have perf impact.
void ProfileGeneratorBase::filterAmbiguousProfile(SampleProfileMap &Profiles) {
for (auto I = ProfileMap.begin(); I != ProfileMap.end();) {
- auto Tmp = I++;
- if (filterAmbiguousProfile(Tmp->second))
- ProfileMap.erase(Tmp);
+ auto FS = I++;
+ if (filterAmbiguousProfile(FS->second))
+ ProfileMap.erase(FS);
}
}
diff --git a/llvm/tools/llvm-profgen/ProfileGenerator.h b/llvm/tools/llvm-profgen/ProfileGenerator.h
index 86eefbaea2bec1..d258fb78bfb113 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.h
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.h
@@ -132,10 +132,7 @@ class ProfileGeneratorBase {
virtual bool collectFunctionsFromLLVMProfile(
std::unordered_set<const BinaryFunction *> &ProfiledFunctions) = 0;
- // Those functions are usually cold but could have multiple versions, the
- // profile are either from one version or the merged version, which is
- // ambiguous. We can't attribute the profile for each version accurately, so
- // filter them out from the profile map.
+ // List of function prefix to filter out.
static constexpr const char *FuncPrefixsToFilter[] = {"__cxx_global_var_init",
"__tls_init"};
More information about the llvm-commits
mailing list