[llvm] [llvm-profgen] Filter out ambiguous cold profiles during profile generation (PR #81803)

Wed Feb 14 15:10:28 PST 2024

llvmbot wrote:



@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-pgo

Author: Lei Wang (wlei-llvm)

<details>
<summary>Changes</summary>

For the local pre-defined initialization function("__cxx_global_var_init", "__tls_init" prefix), the profile is generated either from one version or the merged version,  which is ambiguous. We can't attribute the profile for each version accurately, and it could mislead the profile matching, report a lot of false positives, so filter them out from the profile map during the profile generation time. The profiles are all cold functions, it won't have perf impact.

---
Full diff: https://github.com/llvm/llvm-project/pull/81803.diff


6 Files Affected:

- (modified) llvm/include/llvm/ProfileData/SampleProf.h (+4-2) 
- (modified) llvm/lib/Transforms/IPO/SampleProfile.cpp (+1-1) 
- (added) llvm/test/tools/llvm-profgen/Inputs/filter-ambiguous-profile.prof (+16) 
- (added) llvm/test/tools/llvm-profgen/filter-ambiguous-profile.test (+8) 
- (modified) llvm/tools/llvm-profgen/ProfileGenerator.cpp (+35) 
- (modified) llvm/tools/llvm-profgen/ProfileGenerator.h (+4) 


``````````diff

diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index 8ac84d4b933f20..5530abe37fc2a9 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -466,7 +466,7 @@ struct SampleContextFrame {
   LineLocation Location;
 
   SampleContextFrame() : Location(0, 0) {}
-  
+
   SampleContextFrame(FunctionId Func, LineLocation Location)
       : Func(Func), Location(Location) {}
 
@@ -527,7 +527,7 @@ class SampleContext {
       : Func(Name), State(UnknownContext), Attributes(ContextNone) {
         assert(!Name.empty() && "Name is empty");
       }
-  
+
   SampleContext(FunctionId Func)
       : Func(Func), State(UnknownContext), Attributes(ContextNone) {}
 
@@ -975,6 +975,8 @@ class FunctionSamples {
     return CallsiteSamples;
   }
 
+  CallsiteSampleMap &getCallsiteSamples() { return CallsiteSamples; }
+
   /// Return the maximum of sample counts in a function body. When SkipCallSite
   /// is false, which is the default, the return count includes samples in the
   /// inlined functions. When SkipCallSite is true, the return count only
diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 2fd8668d15e200..5d75f52f985161 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1129,7 +1129,7 @@ void SampleProfileLoader::findExternalInlineCandidate(
         CalleeSample->getContext().hasAttribute(ContextShouldBeInlined);
     if (!PreInline && CalleeSample->getHeadSamplesEstimate() < Threshold)
       continue;
-    
+
     Function *Func = SymbolMap.lookup(CalleeSample->getFunction());
     // Add to the import list only when it's defined out of module.
     if (!Func || Func->isDeclaration())
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..86b0bb4749e79f 100644
--- a/llvm/tools/llvm-profgen/ProfileGenerator.cpp
+++ b/llvm/tools/llvm-profgen/ProfileGenerator.cpp
@@ -196,6 +196,39 @@ void ProfileGeneratorBase::showDensitySuggestion(double Density) {
            << "% total samples: " << format("%.1f", Density) << "\n";
 }
 
+// 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"};
+
+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 : 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;
@@ -492,6 +525,7 @@ void ProfileGenerator::postProcessProfiles() {
   computeSummaryAndThreshold(ProfileMap);
   trimColdProfiles(ProfileMap, ColdCountThreshold);
   calculateAndShowDensity(ProfileMap);
+  filterAmbiguousProfile(ProfileMap);
 }
 
 void ProfileGenerator::trimColdProfiles(const SampleProfileMap &Profiles,
@@ -1024,6 +1058,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..3970876c5bcfb7 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);

``````````

</details>


https://github.com/llvm/llvm-project/pull/81803
