Skip to content
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.

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

[BOLT] Drop high discrepancy profiles in matching #95156

Conversation

shawbyoung
Copy link
Contributor

@shawbyoung shawbyoung commented Jun 11, 2024

Summary: Functions with high discrepancy
(measured by matched function blocks)
can be ignored with an added command line
argument for better performance.

Test Plan: Added
stale-matching-min-matched-block.test

Created using spr 1.3.4
@llvmbot llvmbot added PGO Profile Guided Optimizations BOLT llvm:transforms labels Jun 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 11, 2024

@llvm/pr-subscribers-bolt
@llvm/pr-subscribers-pgo

@llvm/pr-subscribers-llvm-transforms

Author: shaw young (shawbyoung)

Changes

Test Plan: tbd


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

2 Files Affected:

  • (modified) bolt/lib/Profile/StaleProfileMatching.cpp (+14-2)
  • (modified) llvm/include/llvm/Transforms/Utils/SampleProfileInference.h (+2-1)
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 5969f4b9bcbc1..41afa6b4bbb19 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -51,6 +51,12 @@ cl::opt<bool>
                       cl::desc("Infer counts from stale profile data."),
                       cl::init(false), cl::Hidden, cl::cat(BoltOptCategory));
 
+cl::opt<unsigned> MatchedProfileThreshold(
+    "matched-profile-threshold",
+    cl::desc("Percentage threshold of matched execution counts at which stale "
+             "profile inference is executed."),
+    cl::init(5), cl::Hidden, cl::cat(BoltOptCategory));
+
 cl::opt<unsigned> StaleMatchingMaxFuncSize(
     "stale-matching-max-func-size",
     cl::desc("The maximum size of a function to consider for inference."),
@@ -451,6 +457,7 @@ void matchWeightsByHashes(BinaryContext &BC,
       if (Matcher.isHighConfidenceMatch(BinHash, YamlHash)) {
         ++BC.Stats.NumMatchedBlocks;
         BC.Stats.MatchedSampleCount += YamlBB.ExecCount;
+        Func.MatchedExecCount += YamlBB.ExecCount;
         LLVM_DEBUG(dbgs() << "  exact match\n");
       } else {
         LLVM_DEBUG(dbgs() << "  loose match\n");
@@ -592,10 +599,15 @@ void preprocessUnreachableBlocks(FlowFunction &Func) {
 /// Decide if stale profile matching can be applied for a given function.
 /// Currently we skip inference for (very) large instances and for instances
 /// having "unexpected" control flow (e.g., having no sink basic blocks).
-bool canApplyInference(const FlowFunction &Func) {
+bool canApplyInference(const FlowFunction &Func,
+                       const yaml::bolt::BinaryFunctionProfile &YamlBF) {
   if (Func.Blocks.size() > opts::StaleMatchingMaxFuncSize)
     return false;
 
+  if (Func.MatchedExecCount / YamlBF.ExecCount >=
+      opts::MatchedProfileThreshold / 100)
+    return false;
+
   bool HasExitBlocks = llvm::any_of(
       Func.Blocks, [&](const FlowBlock &Block) { return Block.isExit(); });
   if (!HasExitBlocks)
@@ -756,7 +768,7 @@ bool YAMLProfileReader::inferStaleProfile(
   preprocessUnreachableBlocks(Func);
 
   // Check if profile inference can be applied for the instance.
-  if (!canApplyInference(Func))
+  if (!canApplyInference(Func, YamlBF))
     return false;
 
   // Apply the profile inference algorithm.
diff --git a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
index 70a80b91405d0..c654715c0ae9f 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -58,7 +58,8 @@ struct FlowFunction {
   std::vector<FlowJump> Jumps;
   /// The index of the entry block.
   uint64_t Entry{0};
-  uint64_t Sink{UINT64_MAX};
+  // Matched execution count for the function.
+  uint64_t MatchedExecCount{0};
 };
 
 /// Various thresholds and options controlling the behavior of the profile

shawbyoung added a commit to shawbyoung/llvm-project that referenced this pull request Jun 11, 2024
bolt/docs/CommandLineArgumentReference.md Outdated Show resolved Hide resolved
bolt/lib/Profile/StaleProfileMatching.cpp Show resolved Hide resolved
bolt/lib/Profile/StaleProfileMatching.cpp Outdated Show resolved Hide resolved
@WenleiHe
Copy link
Member

cc @wlei-llvm

Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Copy link
Member

@WenleiHe WenleiHe left a comment

Choose a reason for hiding this comment

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

lgtm with a nit, thanks.

bolt/lib/Profile/StaleProfileMatching.cpp Outdated Show resolved Hide resolved
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
bolt/lib/Profile/StaleProfileMatching.cpp Outdated Show resolved Hide resolved
bolt/lib/Profile/StaleProfileMatching.cpp Outdated Show resolved Hide resolved
Created using spr 1.3.4
Copy link
Contributor

@aaupov aaupov left a comment

Choose a reason for hiding this comment

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

LGTM with one nit

bolt/lib/Profile/StaleProfileMatching.cpp Outdated Show resolved Hide resolved
@shawbyoung shawbyoung merged commit b7c43ed into users/shawbyoung/spr/main.bolt-drop-high-discrepancy-profiles-in-matching-1 Jun 17, 2024
5 checks passed
@shawbyoung shawbyoung deleted the users/shawbyoung/spr/bolt-drop-high-discrepancy-profiles-in-matching-1 branch June 17, 2024 20:27
shawbyoung added a commit that referenced this pull request Jun 17, 2024
Summary: Functions with high discrepancy 
(measured by matched function blocks) 
can be ignored with an added command line 
argument for better performance.

Test Plan: Added 
stale-matching-min-matched-block.test

---------

Co-authored-by: Amir Ayupov <aaupov@fb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BOLT llvm:transforms PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants