[llvm-branch-commits] Using matched block counts to measure discrepancy (PR #95486)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: shaw young (shawbyoung)

<details>
<summary>Changes</summary>



Test Plan: tbd


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


2 Files Affected:

- (modified) bolt/lib/Profile/StaleProfileMatching.cpp (+29-8) 
- (modified) llvm/include/llvm/Transforms/Utils/SampleProfileInference.h (-2) 


``````````diff
diff --git a/bolt/lib/Profile/StaleProfileMatching.cpp b/bolt/lib/Profile/StaleProfileMatching.cpp
index 6588cf2c0ce66..cbd98f4d4769f 100644
--- a/bolt/lib/Profile/StaleProfileMatching.cpp
+++ b/bolt/lib/Profile/StaleProfileMatching.cpp
@@ -53,9 +53,9 @@ cl::opt<bool>
 
 cl::opt<unsigned> MatchedProfileThreshold(
     "matched-profile-threshold",
-    cl::desc("Percentage threshold of matched execution counts at which stale "
+    cl::desc("Percentage threshold of matched basic blocks at which stale "
              "profile inference is executed."),
-    cl::init(5), cl::Hidden, cl::cat(BoltOptCategory));
+    cl::init(50), cl::Hidden, cl::cat(BoltOptCategory));
 
 cl::opt<unsigned> StaleMatchingMaxFuncSize(
     "stale-matching-max-func-size",
@@ -186,6 +186,17 @@ struct BlendedBlockHash {
   uint8_t SuccHash{0};
 };
 
+/// A data object containing function matching information.
+struct FunctionMatchingData {
+public:
+  /// The number of blocks matched exactly.
+  uint64_t MatchedExactBlocks{0};
+  /// The number of blocks matched loosely.
+  uint64_t MatchedLooseBlocks{0};
+  /// The number of execution counts matched.
+  uint64_t MatchedExecCounts{0};
+};
+
 /// The object is used to identify and match basic blocks in a BinaryFunction
 /// given their hashes computed on a binary built from several revisions behind
 /// release.
@@ -400,7 +411,8 @@ createFlowFunction(const BinaryFunction::BasicBlockOrderType &BlockOrder) {
 void matchWeightsByHashes(BinaryContext &BC,
                           const BinaryFunction::BasicBlockOrderType &BlockOrder,
                           const yaml::bolt::BinaryFunctionProfile &YamlBF,
-                          FlowFunction &Func) {
+                          FlowFunction &Func,
+                          FunctionMatchingData &FuncMatchingData) {
   assert(Func.Blocks.size() == BlockOrder.size() + 1);
 
   std::vector<FlowBlock *> Blocks;
@@ -440,9 +452,11 @@ void matchWeightsByHashes(BinaryContext &BC,
       if (Matcher.isHighConfidenceMatch(BinHash, YamlHash)) {
         ++BC.Stats.NumMatchedBlocks;
         BC.Stats.MatchedSampleCount += YamlBB.ExecCount;
-        Func.MatchedExecCount += YamlBB.ExecCount;
+        FuncMatchingData.MatchedExecCounts += YamlBB.ExecCount;
+        FuncMatchingData.MatchedExactBlocks += 1;
         LLVM_DEBUG(dbgs() << "  exact match\n");
       } else {
+        FuncMatchingData.MatchedLooseBlocks += 1;
         LLVM_DEBUG(dbgs() << "  loose match\n");
       }
       if (YamlBB.NumInstructions == BB->size())
@@ -582,11 +596,14 @@ 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, const yaml::bolt::BinaryFunctionProfile &YamlBF) {
+bool canApplyInference(const FlowFunction &Func,
+                       const yaml::bolt::BinaryFunctionProfile &YamlBF,
+                       const FunctionMatchingData &FuncMatchingData) {
   if (Func.Blocks.size() > opts::StaleMatchingMaxFuncSize)
     return false;
 
-  if (Func.MatchedExecCount / YamlBF.ExecCount >= opts::MatchedProfileThreshold)
+  if ((double)FuncMatchingData.MatchedExactBlocks / YamlBF.Blocks.size() >=
+      opts::MatchedProfileThreshold / 100.0)
     return false;
 
   bool HasExitBlocks = llvm::any_of(
@@ -735,18 +752,22 @@ bool YAMLProfileReader::inferStaleProfile(
   const BinaryFunction::BasicBlockOrderType BlockOrder(
       BF.getLayout().block_begin(), BF.getLayout().block_end());
 
+  // Create a containter for function matching data.
+  FunctionMatchingData FuncMatchingData;
+
   // Create a wrapper flow function to use with the profile inference algorithm.
   FlowFunction Func = createFlowFunction(BlockOrder);
 
   // Match as many block/jump counts from the stale profile as possible
-  matchWeightsByHashes(BF.getBinaryContext(), BlockOrder, YamlBF, Func);
+  matchWeightsByHashes(BF.getBinaryContext(), BlockOrder, YamlBF, Func,
+                       FuncMatchingData);
 
   // Adjust the flow function by marking unreachable blocks Unlikely so that
   // they don't get any counts assigned.
   preprocessUnreachableBlocks(Func);
 
   // Check if profile inference can be applied for the instance.
-  if (!canApplyInference(Func, YamlBF))
+  if (!canApplyInference(Func, YamlBF, FuncMatchingData))
     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 e7971ca1cb428..b4ea1ad840f9d 100644
--- a/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
+++ b/llvm/include/llvm/Transforms/Utils/SampleProfileInference.h
@@ -57,8 +57,6 @@ struct FlowFunction {
   std::vector<FlowJump> Jumps;
   /// The index of the entry block.
   uint64_t Entry{0};
-  // Matched execution count for the function.
-  uint64_t MatchedExecCount{0};
 };
 
 /// Various thresholds and options controlling the behavior of the profile

``````````

</details>


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