[llvm] [BOLT] Profile quality stats -- CFG discontinuity (PR #109683)

via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 8 13:42:02 PDT 2024


https://github.com/ShatianWang updated https://github.com/llvm/llvm-project/pull/109683

>From f7d8c3e1f72ffad0087089f98aa304e0858e664d Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Fri, 6 Sep 2024 14:39:25 -0700
Subject: [PATCH 1/7] [BOLT] Profile quality stats -- CFG discontinuity

---
 bolt/include/bolt/Passes/ContinuityStats.h    |  40 +++
 bolt/lib/Passes/CMakeLists.txt                |   1 +
 bolt/lib/Passes/ContinuityStats.cpp           | 270 ++++++++++++++++++
 bolt/lib/Rewrite/BinaryPassManager.cpp        |   9 +
 .../test/X86/cfg-discontinuity-reporting.test |   5 +
 5 files changed, 325 insertions(+)
 create mode 100644 bolt/include/bolt/Passes/ContinuityStats.h
 create mode 100644 bolt/lib/Passes/ContinuityStats.cpp
 create mode 100644 bolt/test/X86/cfg-discontinuity-reporting.test

diff --git a/bolt/include/bolt/Passes/ContinuityStats.h b/bolt/include/bolt/Passes/ContinuityStats.h
new file mode 100644
index 00000000000000..b9edc09cbf55cc
--- /dev/null
+++ b/bolt/include/bolt/Passes/ContinuityStats.h
@@ -0,0 +1,40 @@
+//===- bolt/Passes/ContinuityStats.h - function cfg continuity analysis ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Conduct function CFG continuity analysis.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef BOLT_PASSES_CONTINUITYSTATS_H
+#define BOLT_PASSES_CONTINUITYSTATS_H
+
+#include <vector>
+#include "bolt/Passes/BinaryPasses.h"
+
+namespace llvm {
+
+class raw_ostream;
+
+namespace bolt {
+class BinaryContext;
+
+/// Compute and report to the user the function CFG continuity quality
+class PrintContinuityStats : public BinaryFunctionPass {
+public:
+  explicit PrintContinuityStats(const cl::opt<bool> &PrintPass)
+      : BinaryFunctionPass(PrintPass) {}
+
+  const char *getName() const override { return "continuity-stats"; }
+  bool shouldPrint(const BinaryFunction &) const override { return false; }
+  Error runOnFunctions(BinaryContext &BC) override;
+};
+
+} // namespace bolt
+} // namespace llvm
+
+#endif // BOLT_PASSES_CONTINUITYSTATS_H
diff --git a/bolt/lib/Passes/CMakeLists.txt b/bolt/lib/Passes/CMakeLists.txt
index 407d8b03f73977..1c1273b3d2420d 100644
--- a/bolt/lib/Passes/CMakeLists.txt
+++ b/bolt/lib/Passes/CMakeLists.txt
@@ -26,6 +26,7 @@ add_llvm_library(LLVMBOLTPasses
   PatchEntries.cpp
   PettisAndHansen.cpp
   PLTCall.cpp
+  ContinuityStats.cpp
   RegAnalysis.cpp
   RegReAssign.cpp
   ReorderAlgorithm.cpp
diff --git a/bolt/lib/Passes/ContinuityStats.cpp b/bolt/lib/Passes/ContinuityStats.cpp
new file mode 100644
index 00000000000000..d2ce97949c1c12
--- /dev/null
+++ b/bolt/lib/Passes/ContinuityStats.cpp
@@ -0,0 +1,270 @@
+//===- bolt/Passes/ContinuityStats.cpp - function cfg continuity analysis ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Conduct function CFG continuity analysis.
+//
+//===----------------------------------------------------------------------===//
+
+#include "bolt/Passes/ContinuityStats.h"
+#include "bolt/Core/BinaryBasicBlock.h"
+#include "bolt/Core/BinaryFunction.h"
+#include "bolt/Utils/CommandLineOpts.h"
+#include "llvm/Support/CommandLine.h"
+#include <queue>
+#include <unordered_map>
+#include <unordered_set>
+
+#define DEBUG_TYPE "bolt-opts"
+
+using namespace llvm;
+using namespace bolt;
+
+namespace opts {
+extern cl::opt<unsigned> Verbosity;
+cl::opt<unsigned> NumTopFunctions(
+    "num-top-functions",
+    cl::desc(
+        "number of hottest functions to print aggregated CFG discontinuity stats of."),
+    cl::init(1000), cl::ZeroOrMore, cl::Hidden, cl::cat(BoltOptCategory));
+cl::opt<bool> PrintBucketedStats(
+    "print-bucketed-stats",
+    cl::desc("print CFG discontinuity stats for the top functions divided into buckets "
+             "based on their execution counts."),
+    cl::Hidden, cl::cat(BoltCategory));
+cl::opt<unsigned>
+    NumFunctionsPerBucket("num-functions-per-bucket",
+                          cl::desc("maximum number of functions per bucket."),
+                          cl::init(500), cl::ZeroOrMore, cl::Hidden,
+                          cl::cat(BoltOptCategory));
+cl::opt<unsigned>
+    MinNumFunctions("min-num-functions",
+                          cl::desc("minimum number of hot functions in the binary to "
+                          "trigger profile CFG continuity check."),
+                          cl::init(5), cl::ZeroOrMore, cl::Hidden,
+                          cl::cat(BoltOptCategory));
+} // namespace opts
+
+namespace {
+using FunctionListType = std::vector<const BinaryFunction *>;
+using function_iterator = FunctionListType::iterator;
+
+template <typename T>
+void printDistribution(raw_ostream &OS, std::vector<T> &values,
+                       bool Fraction = false) {
+  if (values.empty())
+    return;
+  // Sort values from largest to smallest and print the MAX, TOP 1%, 5%, 10%,
+  // 20%, 50%, 80%, MIN. If Fraction is true, then values are printed as
+  // fractions instead of integers.
+  std::sort(values.begin(), values.end());
+
+  auto printLine = [&](std::string Text, double Percent) {
+    int Rank = int(values.size() * (1.0 - Percent / 100));
+    if (Percent == 0)
+      Rank = values.size() - 1;
+    if (Fraction)
+      OS << "  " << Text << std::string(9 - Text.length(), ' ') << ": "
+         << format("%.2lf%%", values[Rank] * 100) << "\n";
+    else
+      OS << "  " << Text << std::string(9 - Text.length(), ' ') << ": "
+         << values[Rank] << "\n";
+  };
+
+  printLine("MAX", 0);
+  int percentages[] = {1, 5, 10, 20, 50, 80};
+  for (size_t i = 0; i < sizeof(percentages) / sizeof(percentages[0]); ++i) {
+    printLine("TOP " + std::to_string(percentages[i]) + "%", percentages[i]);
+  }
+  printLine("MIN", 100);
+}
+
+void printCFGContinuityStats(raw_ostream &OS,
+                               iterator_range<function_iterator> &Functions, 
+                               bool Verbose=false) {
+  // Given a perfect profile, every positive-execution-count BB should be
+  // connected to an entry of the function through a positive-execution-count
+  // directed path in the control flow graph.
+  std::vector<size_t> NumUnreachables;
+  std::vector<size_t> SumECUnreachables;
+  std::vector<double> FractionECUnreachables;
+
+  for (auto it = Functions.begin(); it != Functions.end(); ++it) {
+    const BinaryFunction *Function = *it;
+    if (Function->size() <= 1)
+      continue;
+
+    // Compute the sum of all BB execution counts (ECs).
+    size_t NumPosECBBs = 0;
+    size_t SumAllBBEC = 0;
+    for (const BinaryBasicBlock &BB : *Function) {
+      size_t BBEC = BB.getKnownExecutionCount();
+      NumPosECBBs += BBEC > 0 ? 1 : 0;
+      SumAllBBEC += BBEC;
+    }
+
+    // Perform BFS on subgraph of CFG induced by positive weight edges.
+    // Compute the number of BBs reachable from the entry(s) of the function and
+    // the sum of their execution counts (ECs).
+    std::unordered_map<unsigned, const BinaryBasicBlock *> IndexToBB;
+    std::unordered_set<unsigned> Visited;
+    std::queue<unsigned> Queue;
+    for (const BinaryBasicBlock &BB : *Function) {
+      // Make sure BB.getIndex() is not already in IndexToBB.
+      assert(IndexToBB.find(BB.getIndex()) == IndexToBB.end());
+      IndexToBB[BB.getIndex()] = &BB;
+      if (BB.isEntryPoint() && BB.getKnownExecutionCount() > 0) {
+        Queue.push(BB.getIndex());
+        Visited.insert(BB.getIndex());
+      }
+    }
+    while (!Queue.empty()) {
+      unsigned BBIndex = Queue.front();
+      const BinaryBasicBlock *BB = IndexToBB[BBIndex];
+      Queue.pop();
+      auto SuccBIIter = BB->branch_info_begin();
+      for (BinaryBasicBlock *Succ : BB->successors()) {
+        uint64_t Count = SuccBIIter->Count;
+        if (Count == BinaryBasicBlock::COUNT_NO_PROFILE || Count == 0) {
+          ++SuccBIIter;
+          continue;
+        }
+        if (!Visited.insert(Succ->getIndex()).second) {
+          ++SuccBIIter;
+          continue;
+        }
+        Queue.push(Succ->getIndex());
+        ++SuccBIIter;
+      }
+    }
+
+    size_t NumReachableBBs = Visited.size();
+
+    // Loop through Visited, and sum the corresponding BBs' execution counts (ECs).
+    size_t SumReachableBBEC = 0;
+    for (unsigned BBIndex : Visited) {
+      const BinaryBasicBlock *BB = IndexToBB[BBIndex];
+      SumReachableBBEC += BB->getKnownExecutionCount();
+    }
+
+    size_t NumPosECBBsUnreachableFromEntry = NumPosECBBs - NumReachableBBs;
+    size_t SumUnreachableBBEC = SumAllBBEC - SumReachableBBEC;
+    double FractionECUnreachable = (double)SumUnreachableBBEC / SumAllBBEC;
+
+    if (opts::Verbosity >= 2 && FractionECUnreachable > 0.1 &&
+        SumUnreachableBBEC > 50) {
+      OS << "Non-trivial CFG discontinuity observed in function "
+         << Function->getPrintName() << "\n";
+      LLVM_DEBUG(Function->dump());
+    }
+
+    NumUnreachables.push_back(NumPosECBBsUnreachableFromEntry);
+    SumECUnreachables.push_back(SumUnreachableBBEC);
+    FractionECUnreachables.push_back(FractionECUnreachable);
+  }
+
+  if (!Verbose) {
+    if (FractionECUnreachables.empty()) {
+      OS << "no functions have more than 1 basic block and hence no CFG discontinuity.\n";
+      return;
+    }
+    std::sort(FractionECUnreachables.begin(), FractionECUnreachables.end());
+    int Rank = int(FractionECUnreachables.size() * 0.95);
+    OS << format("the TOP 5%% function CFG discontinuity is %.2lf%%", FractionECUnreachables[Rank] * 100) << "\n";
+    return;
+  }
+  
+  OS << format("Focus on %zu (%.2lf%%) of considered functions that have at "
+               "least 2 basic blocks\n",
+               SumECUnreachables.size(),
+               100.0 * (double)SumECUnreachables.size() /
+                   (std::distance(Functions.begin(), Functions.end())));
+
+  if (!NumUnreachables.empty()) {
+    OS << "- Distribution of NUM(unreachable POS BBs) among all focal "
+          "functions\n";
+    printDistribution(OS, NumUnreachables);
+  }
+  if (!SumECUnreachables.empty()) {
+    OS << "- Distribution of SUM(unreachable POS BBs) among all focal "
+          "functions\n";
+    printDistribution(OS, SumECUnreachables);
+  }
+  if (!FractionECUnreachables.empty()) {
+    OS << "- Distribution of [(SUM(unreachable POS BBs) / SUM(all "
+          "POS BBs))] among all focal functions\n";
+    printDistribution(OS, FractionECUnreachables, /*Fraction=*/true);
+  }
+}
+
+void printAll(BinaryContext &BC,
+              size_t NumFunctionsPerBucket,
+              size_t NumTopFunctions) {
+
+  // Create a list of functions with valid profiles.
+  FunctionListType ValidFunctions;
+  for (const auto &BFI : BC.getBinaryFunctions()) {
+    const BinaryFunction *Function = &BFI.second;
+    if (Function->empty() || !Function->hasValidProfile() ||
+        !Function->isSimple())
+      continue;
+    ValidFunctions.push_back(Function);
+  }
+
+  // Sort the list of functions by execution counts (reverse).
+  llvm::sort(ValidFunctions,
+             [&](const BinaryFunction *A, const BinaryFunction *B) {
+               return A->getKnownExecutionCount() > B->getKnownExecutionCount();
+             });
+
+  size_t RealNumTopFunctions = std::min(NumTopFunctions, ValidFunctions.size());
+  if (RealNumTopFunctions <= opts::MinNumFunctions)
+    return;
+  BC.outs() << format("BOLT-INFO: among the hottest %zu functions ", RealNumTopFunctions);
+  iterator_range<function_iterator> Functions(
+      ValidFunctions.begin(), ValidFunctions.begin() + RealNumTopFunctions);
+  printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/false);
+
+  // Print more detailed bucketed stats if requested.
+  if (opts::PrintBucketedStats) {
+    size_t PerBucketSize = std::min(NumFunctionsPerBucket, ValidFunctions.size());
+    if (PerBucketSize == 0)
+      return;
+    size_t NumBuckets = RealNumTopFunctions / PerBucketSize +
+                        (RealNumTopFunctions % PerBucketSize != 0);
+    BC.outs() << format("Detailed stats for %zu buckets, each with at most %zu functions:\n",
+                NumBuckets, PerBucketSize);
+    BC.outs() << "For each considered function, identify positive execution-count basic blocks\n"
+       << "(abbr. POS BBs) that are *unreachable* from the function entry through a\n"
+       << "positive-execution-count path.\n";
+
+    // For each bucket, print the CFG continuity stats of the functions in the bucket.
+    for (size_t BucketIndex = 0; BucketIndex < NumBuckets; ++BucketIndex) {
+      const size_t StartIndex = BucketIndex * PerBucketSize;
+      size_t EndIndex = std::min(StartIndex + PerBucketSize, NumTopFunctions);
+      EndIndex = std::min(EndIndex, ValidFunctions.size());
+      iterator_range<function_iterator> Functions(
+          ValidFunctions.begin() + StartIndex, ValidFunctions.begin() + EndIndex);
+      const size_t MaxFunctionExecutionCount =
+          ValidFunctions[StartIndex]->getKnownExecutionCount();
+      const size_t MinFunctionExecutionCount =
+          ValidFunctions[EndIndex - 1]->getKnownExecutionCount();
+      BC.outs() << format("----------------\n|   Bucket %zu:  "
+                  "|\n----------------\nExecution counts of the %zu functions in the bucket: "
+                  "%zu-%zu\n",
+                  BucketIndex + 1, EndIndex - StartIndex,
+                  MinFunctionExecutionCount, MaxFunctionExecutionCount);
+      printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/true);
+    }
+  }
+}
+} // namespace
+
+Error PrintContinuityStats::runOnFunctions(BinaryContext &BC) {
+  printAll(BC, opts::NumFunctionsPerBucket, opts::NumTopFunctions);
+  return Error::success();
+}
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index 5dfef0b71cc79f..aad4d15cf2869d 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -11,6 +11,7 @@
 #include "bolt/Passes/Aligner.h"
 #include "bolt/Passes/AllocCombiner.h"
 #include "bolt/Passes/AsmDump.h"
+#include "bolt/Passes/ContinuityStats.h"
 #include "bolt/Passes/CMOVConversion.h"
 #include "bolt/Passes/FixRISCVCallsPass.h"
 #include "bolt/Passes/FixRelaxationPass.h"
@@ -154,6 +155,11 @@ static cl::opt<bool>
                       cl::desc("print profile quality/bias analysis"),
                       cl::cat(BoltCategory));
 
+static cl::opt<bool>
+    PrintContinuityStats("print-continuity-stats",
+                      cl::desc("print profile function CFG continuity stats"),
+                      cl::init(true), cl::cat(BoltCategory));
+
 static cl::opt<bool>
     PrintRegReAssign("print-regreassign",
                      cl::desc("print functions after regreassign pass"),
@@ -373,6 +379,9 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   if (opts::PrintProfileStats)
     Manager.registerPass(std::make_unique<PrintProfileStats>(NeverPrint));
 
+  if (opts::PrintContinuityStats)
+    Manager.registerPass(std::make_unique<PrintContinuityStats>(NeverPrint));
+
   Manager.registerPass(std::make_unique<ValidateInternalCalls>(NeverPrint));
 
   Manager.registerPass(std::make_unique<ValidateMemRefs>(NeverPrint));
diff --git a/bolt/test/X86/cfg-discontinuity-reporting.test b/bolt/test/X86/cfg-discontinuity-reporting.test
new file mode 100644
index 00000000000000..563d55c3013ff8
--- /dev/null
+++ b/bolt/test/X86/cfg-discontinuity-reporting.test
@@ -0,0 +1,5 @@
+## Check profile discontinuity reporting
+RUN: yaml2obj %p/Inputs/blarge_new.yaml &> %t.exe
+RUN: llvm-bolt %t.exe -o %t.out --pa -p %p/Inputs/blarge_new.preagg.txt \
+RUN:   --print-continuity-stats --min-num-functions=1 | FileCheck %s
+CHECK: among the hottest 5 functions the TOP 5% function CFG discontinuity is 100.00%

>From 9f6023e7764c0326ae04058b7973e1a0a1b8adf5 Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Mon, 23 Sep 2024 10:56:30 -0700
Subject: [PATCH 2/7] fixup! [BOLT] Profile quality stats -- CFG discontinuity

---
 bolt/include/bolt/Passes/ContinuityStats.h |  5 +-
 bolt/lib/Passes/ContinuityStats.cpp        | 86 +++++++++++++---------
 bolt/lib/Rewrite/BinaryPassManager.cpp     | 10 +--
 3 files changed, 58 insertions(+), 43 deletions(-)

diff --git a/bolt/include/bolt/Passes/ContinuityStats.h b/bolt/include/bolt/Passes/ContinuityStats.h
index b9edc09cbf55cc..4a39ada3529e9c 100644
--- a/bolt/include/bolt/Passes/ContinuityStats.h
+++ b/bolt/include/bolt/Passes/ContinuityStats.h
@@ -1,4 +1,5 @@
-//===- bolt/Passes/ContinuityStats.h - function cfg continuity analysis ---*- C++ -*-===//
+//===- bolt/Passes/ContinuityStats.h - function cfg continuity analysis ---*-
+//C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -13,8 +14,8 @@
 #ifndef BOLT_PASSES_CONTINUITYSTATS_H
 #define BOLT_PASSES_CONTINUITYSTATS_H
 
-#include <vector>
 #include "bolt/Passes/BinaryPasses.h"
+#include <vector>
 
 namespace llvm {
 
diff --git a/bolt/lib/Passes/ContinuityStats.cpp b/bolt/lib/Passes/ContinuityStats.cpp
index d2ce97949c1c12..ae1a5ef03e728e 100644
--- a/bolt/lib/Passes/ContinuityStats.cpp
+++ b/bolt/lib/Passes/ContinuityStats.cpp
@@ -1,4 +1,5 @@
-//===- bolt/Passes/ContinuityStats.cpp - function cfg continuity analysis ---*- C++ -*-===//
+//===- bolt/Passes/ContinuityStats.cpp - function cfg continuity analysis ---*-
+//C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -26,16 +27,18 @@ using namespace bolt;
 
 namespace opts {
 extern cl::opt<unsigned> Verbosity;
-cl::opt<unsigned> NumTopFunctions(
-    "num-top-functions",
-    cl::desc(
-        "number of hottest functions to print aggregated CFG discontinuity stats of."),
-    cl::init(1000), cl::ZeroOrMore, cl::Hidden, cl::cat(BoltOptCategory));
-cl::opt<bool> PrintBucketedStats(
-    "print-bucketed-stats",
-    cl::desc("print CFG discontinuity stats for the top functions divided into buckets "
-             "based on their execution counts."),
-    cl::Hidden, cl::cat(BoltCategory));
+cl::opt<unsigned>
+    NumTopFunctions("num-top-functions",
+                    cl::desc("number of hottest functions to print aggregated "
+                             "CFG discontinuity stats of."),
+                    cl::init(1000), cl::ZeroOrMore, cl::Hidden,
+                    cl::cat(BoltOptCategory));
+cl::opt<bool>
+    PrintBucketedStats("print-bucketed-stats",
+                       cl::desc("print CFG discontinuity stats for the top "
+                                "functions divided into buckets "
+                                "based on their execution counts."),
+                       cl::Hidden, cl::cat(BoltCategory));
 cl::opt<unsigned>
     NumFunctionsPerBucket("num-functions-per-bucket",
                           cl::desc("maximum number of functions per bucket."),
@@ -43,10 +46,10 @@ cl::opt<unsigned>
                           cl::cat(BoltOptCategory));
 cl::opt<unsigned>
     MinNumFunctions("min-num-functions",
-                          cl::desc("minimum number of hot functions in the binary to "
-                          "trigger profile CFG continuity check."),
-                          cl::init(5), cl::ZeroOrMore, cl::Hidden,
-                          cl::cat(BoltOptCategory));
+                    cl::desc("minimum number of hot functions in the binary to "
+                             "trigger profile CFG continuity check."),
+                    cl::init(5), cl::ZeroOrMore, cl::Hidden,
+                    cl::cat(BoltOptCategory));
 } // namespace opts
 
 namespace {
@@ -84,8 +87,8 @@ void printDistribution(raw_ostream &OS, std::vector<T> &values,
 }
 
 void printCFGContinuityStats(raw_ostream &OS,
-                               iterator_range<function_iterator> &Functions, 
-                               bool Verbose=false) {
+                             iterator_range<function_iterator> &Functions,
+                             bool Verbose = false) {
   // Given a perfect profile, every positive-execution-count BB should be
   // connected to an entry of the function through a positive-execution-count
   // directed path in the control flow graph.
@@ -144,7 +147,8 @@ void printCFGContinuityStats(raw_ostream &OS,
 
     size_t NumReachableBBs = Visited.size();
 
-    // Loop through Visited, and sum the corresponding BBs' execution counts (ECs).
+    // Loop through Visited, and sum the corresponding BBs' execution counts
+    // (ECs).
     size_t SumReachableBBEC = 0;
     for (unsigned BBIndex : Visited) {
       const BinaryBasicBlock *BB = IndexToBB[BBIndex];
@@ -169,15 +173,18 @@ void printCFGContinuityStats(raw_ostream &OS,
 
   if (!Verbose) {
     if (FractionECUnreachables.empty()) {
-      OS << "no functions have more than 1 basic block and hence no CFG discontinuity.\n";
+      OS << "no functions have more than 1 basic block and hence no CFG "
+            "discontinuity.\n";
       return;
     }
     std::sort(FractionECUnreachables.begin(), FractionECUnreachables.end());
     int Rank = int(FractionECUnreachables.size() * 0.95);
-    OS << format("the TOP 5%% function CFG discontinuity is %.2lf%%", FractionECUnreachables[Rank] * 100) << "\n";
+    OS << format("the TOP 5%% function CFG discontinuity is %.2lf%%",
+                 FractionECUnreachables[Rank] * 100)
+       << "\n";
     return;
   }
-  
+
   OS << format("Focus on %zu (%.2lf%%) of considered functions that have at "
                "least 2 basic blocks\n",
                SumECUnreachables.size(),
@@ -201,8 +208,7 @@ void printCFGContinuityStats(raw_ostream &OS,
   }
 }
 
-void printAll(BinaryContext &BC,
-              size_t NumFunctionsPerBucket,
+void printAll(BinaryContext &BC, size_t NumFunctionsPerBucket,
               size_t NumTopFunctions) {
 
   // Create a list of functions with valid profiles.
@@ -224,40 +230,48 @@ void printAll(BinaryContext &BC,
   size_t RealNumTopFunctions = std::min(NumTopFunctions, ValidFunctions.size());
   if (RealNumTopFunctions <= opts::MinNumFunctions)
     return;
-  BC.outs() << format("BOLT-INFO: among the hottest %zu functions ", RealNumTopFunctions);
+  BC.outs() << format("BOLT-INFO: among the hottest %zu functions ",
+                      RealNumTopFunctions);
   iterator_range<function_iterator> Functions(
       ValidFunctions.begin(), ValidFunctions.begin() + RealNumTopFunctions);
   printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/false);
 
   // Print more detailed bucketed stats if requested.
   if (opts::PrintBucketedStats) {
-    size_t PerBucketSize = std::min(NumFunctionsPerBucket, ValidFunctions.size());
+    size_t PerBucketSize =
+        std::min(NumFunctionsPerBucket, ValidFunctions.size());
     if (PerBucketSize == 0)
       return;
     size_t NumBuckets = RealNumTopFunctions / PerBucketSize +
                         (RealNumTopFunctions % PerBucketSize != 0);
-    BC.outs() << format("Detailed stats for %zu buckets, each with at most %zu functions:\n",
-                NumBuckets, PerBucketSize);
-    BC.outs() << "For each considered function, identify positive execution-count basic blocks\n"
-       << "(abbr. POS BBs) that are *unreachable* from the function entry through a\n"
-       << "positive-execution-count path.\n";
+    BC.outs() << format(
+        "Detailed stats for %zu buckets, each with at most %zu functions:\n",
+        NumBuckets, PerBucketSize);
+    BC.outs() << "For each considered function, identify positive "
+                 "execution-count basic blocks\n"
+              << "(abbr. POS BBs) that are *unreachable* from the function "
+                 "entry through a\n"
+              << "positive-execution-count path.\n";
 
-    // For each bucket, print the CFG continuity stats of the functions in the bucket.
+    // For each bucket, print the CFG continuity stats of the functions in the
+    // bucket.
     for (size_t BucketIndex = 0; BucketIndex < NumBuckets; ++BucketIndex) {
       const size_t StartIndex = BucketIndex * PerBucketSize;
       size_t EndIndex = std::min(StartIndex + PerBucketSize, NumTopFunctions);
       EndIndex = std::min(EndIndex, ValidFunctions.size());
       iterator_range<function_iterator> Functions(
-          ValidFunctions.begin() + StartIndex, ValidFunctions.begin() + EndIndex);
+          ValidFunctions.begin() + StartIndex,
+          ValidFunctions.begin() + EndIndex);
       const size_t MaxFunctionExecutionCount =
           ValidFunctions[StartIndex]->getKnownExecutionCount();
       const size_t MinFunctionExecutionCount =
           ValidFunctions[EndIndex - 1]->getKnownExecutionCount();
       BC.outs() << format("----------------\n|   Bucket %zu:  "
-                  "|\n----------------\nExecution counts of the %zu functions in the bucket: "
-                  "%zu-%zu\n",
-                  BucketIndex + 1, EndIndex - StartIndex,
-                  MinFunctionExecutionCount, MaxFunctionExecutionCount);
+                          "|\n----------------\nExecution counts of the %zu "
+                          "functions in the bucket: "
+                          "%zu-%zu\n",
+                          BucketIndex + 1, EndIndex - StartIndex,
+                          MinFunctionExecutionCount, MaxFunctionExecutionCount);
       printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/true);
     }
   }
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index aad4d15cf2869d..4591649e21b496 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -11,8 +11,8 @@
 #include "bolt/Passes/Aligner.h"
 #include "bolt/Passes/AllocCombiner.h"
 #include "bolt/Passes/AsmDump.h"
-#include "bolt/Passes/ContinuityStats.h"
 #include "bolt/Passes/CMOVConversion.h"
+#include "bolt/Passes/ContinuityStats.h"
 #include "bolt/Passes/FixRISCVCallsPass.h"
 #include "bolt/Passes/FixRelaxationPass.h"
 #include "bolt/Passes/FrameOptimizer.h"
@@ -155,10 +155,10 @@ static cl::opt<bool>
                       cl::desc("print profile quality/bias analysis"),
                       cl::cat(BoltCategory));
 
-static cl::opt<bool>
-    PrintContinuityStats("print-continuity-stats",
-                      cl::desc("print profile function CFG continuity stats"),
-                      cl::init(true), cl::cat(BoltCategory));
+static cl::opt<bool> PrintContinuityStats(
+    "print-continuity-stats",
+    cl::desc("print profile function CFG continuity stats"), cl::init(true),
+    cl::cat(BoltCategory));
 
 static cl::opt<bool>
     PrintRegReAssign("print-regreassign",

>From 5b9e07a9c0b5e4c7dcefe573781c2c30fdf626e4 Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Mon, 23 Sep 2024 11:58:44 -0700
Subject: [PATCH 3/7] fixup! fixup! [BOLT] Profile quality stats -- CFG
 discontinuity

---
 bolt/include/bolt/Passes/ContinuityStats.h |  3 +-
 bolt/lib/Passes/ContinuityStats.cpp        | 35 ++++++++++++----------
 bolt/lib/Rewrite/BinaryPassManager.cpp     |  2 +-
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/bolt/include/bolt/Passes/ContinuityStats.h b/bolt/include/bolt/Passes/ContinuityStats.h
index 4a39ada3529e9c..ea6bb380d7ebef 100644
--- a/bolt/include/bolt/Passes/ContinuityStats.h
+++ b/bolt/include/bolt/Passes/ContinuityStats.h
@@ -1,5 +1,5 @@
 //===- bolt/Passes/ContinuityStats.h - function cfg continuity analysis ---*-
-//C++ -*-===//
+// C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -30,6 +30,7 @@ class PrintContinuityStats : public BinaryFunctionPass {
   explicit PrintContinuityStats(const cl::opt<bool> &PrintPass)
       : BinaryFunctionPass(PrintPass) {}
 
+  bool shouldOptimize(const BinaryFunction &BF) const override;
   const char *getName() const override { return "continuity-stats"; }
   bool shouldPrint(const BinaryFunction &) const override { return false; }
   Error runOnFunctions(BinaryContext &BC) override;
diff --git a/bolt/lib/Passes/ContinuityStats.cpp b/bolt/lib/Passes/ContinuityStats.cpp
index ae1a5ef03e728e..ec0906158841d9 100644
--- a/bolt/lib/Passes/ContinuityStats.cpp
+++ b/bolt/lib/Passes/ContinuityStats.cpp
@@ -1,5 +1,5 @@
 //===- bolt/Passes/ContinuityStats.cpp - function cfg continuity analysis ---*-
-//C++ -*-===//
+// C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -208,19 +208,8 @@ void printCFGContinuityStats(raw_ostream &OS,
   }
 }
 
-void printAll(BinaryContext &BC, size_t NumFunctionsPerBucket,
-              size_t NumTopFunctions) {
-
-  // Create a list of functions with valid profiles.
-  FunctionListType ValidFunctions;
-  for (const auto &BFI : BC.getBinaryFunctions()) {
-    const BinaryFunction *Function = &BFI.second;
-    if (Function->empty() || !Function->hasValidProfile() ||
-        !Function->isSimple())
-      continue;
-    ValidFunctions.push_back(Function);
-  }
-
+void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
+              size_t NumFunctionsPerBucket, size_t NumTopFunctions) {
   // Sort the list of functions by execution counts (reverse).
   llvm::sort(ValidFunctions,
              [&](const BinaryFunction *A, const BinaryFunction *B) {
@@ -278,7 +267,23 @@ void printAll(BinaryContext &BC, size_t NumFunctionsPerBucket,
 }
 } // namespace
 
+bool PrintContinuityStats::shouldOptimize(const BinaryFunction &BF) const {
+  // Apply execution count threshold
+  if (BF.empty() || !BF.hasValidProfile())
+    return false;
+
+  return BinaryFunctionPass::shouldOptimize(BF);
+}
+
 Error PrintContinuityStats::runOnFunctions(BinaryContext &BC) {
-  printAll(BC, opts::NumFunctionsPerBucket, opts::NumTopFunctions);
+  // Create a list of functions with valid profiles.
+  FunctionListType ValidFunctions;
+  for (const auto &BFI : BC.getBinaryFunctions()) {
+    const BinaryFunction *Function = &BFI.second;
+    if (PrintContinuityStats::shouldOptimize(*Function))
+      ValidFunctions.push_back(Function);
+  }
+  printAll(BC, ValidFunctions, opts::NumFunctionsPerBucket,
+           opts::NumTopFunctions);
   return Error::success();
 }
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index 4591649e21b496..116a2a0ab4361c 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -157,7 +157,7 @@ static cl::opt<bool>
 
 static cl::opt<bool> PrintContinuityStats(
     "print-continuity-stats",
-    cl::desc("print profile function CFG continuity stats"), cl::init(true),
+    cl::desc("print profile function CFG continuity stats"),
     cl::cat(BoltCategory));
 
 static cl::opt<bool>

>From ff2309578988ed13243ff5ec932df243d30fe108 Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Mon, 23 Sep 2024 17:29:44 -0700
Subject: [PATCH 4/7] fixup! fixup! fixup! [BOLT] Profile quality stats -- CFG
 discontinuity

---
 bolt/lib/Passes/ContinuityStats.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/bolt/lib/Passes/ContinuityStats.cpp b/bolt/lib/Passes/ContinuityStats.cpp
index ec0906158841d9..365e65a1f270dc 100644
--- a/bolt/lib/Passes/ContinuityStats.cpp
+++ b/bolt/lib/Passes/ContinuityStats.cpp
@@ -268,7 +268,6 @@ void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
 } // namespace
 
 bool PrintContinuityStats::shouldOptimize(const BinaryFunction &BF) const {
-  // Apply execution count threshold
   if (BF.empty() || !BF.hasValidProfile())
     return false;
 

>From 2da1163dcc35bc0bd0330c4af5f8d1d60e6d7899 Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Wed, 2 Oct 2024 12:28:32 -0700
Subject: [PATCH 5/7] fixup! fixup! fixup! fixup! [BOLT] Profile quality stats
 -- CFG discontinuity

---
 bolt/include/bolt/Passes/ContinuityStats.h    |  25 +++-
 bolt/lib/Passes/ContinuityStats.cpp           | 109 +++++++-----------
 bolt/lib/Rewrite/BinaryPassManager.cpp        |   8 +-
 .../test/X86/cfg-discontinuity-reporting.test |   5 +-
 4 files changed, 65 insertions(+), 82 deletions(-)

diff --git a/bolt/include/bolt/Passes/ContinuityStats.h b/bolt/include/bolt/Passes/ContinuityStats.h
index ea6bb380d7ebef..ed149535e1b115 100644
--- a/bolt/include/bolt/Passes/ContinuityStats.h
+++ b/bolt/include/bolt/Passes/ContinuityStats.h
@@ -1,5 +1,4 @@
-//===- bolt/Passes/ContinuityStats.h - function cfg continuity analysis ---*-
-// C++ -*-===//
+//===- bolt/Passes/ContinuityStats.h ----------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -7,7 +6,27 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// Conduct function CFG continuity analysis.
+// This pass checks how well the BOLT input profile satisfies the following
+// "CFG continuity" property of a perfect profile:
+//
+//        Each positive-execution-count block in the function’s CFG
+//        should be *reachable* from a positive-execution-count function
+//        entry block through a positive-execution-count path.
+//
+// More specifically, for each of the hottest 1000 functions, the pass
+// calculates the function’s fraction of basic block execution counts
+// that is *unreachable*. It then reports the 95th percentile of the
+// distribution of the 1000 unreachable fractions in a single BOLT-INFO line.
+// The smaller the reported value is, the better the BOLT profile
+// satisfies the CFG continuity property.
+
+// The default value of 1000 above can be changed via the hidden BOLT option
+// `-num-hottest-functions-for-continuity-check=[N]`.
+// If more detailed stats are needed, `-v=1` can be used: the hottest N
+// functions will be grouped into 5 equally-sized buckets, from the hottest
+// to the coldest; for each bucket, various summary statistics of the
+// distribution of the unreachable fractions and the raw unreachable execution
+// counts will be reported.
 //
 //===----------------------------------------------------------------------===//
 
diff --git a/bolt/lib/Passes/ContinuityStats.cpp b/bolt/lib/Passes/ContinuityStats.cpp
index 365e65a1f270dc..4492a2e05a2229 100644
--- a/bolt/lib/Passes/ContinuityStats.cpp
+++ b/bolt/lib/Passes/ContinuityStats.cpp
@@ -1,5 +1,4 @@
-//===- bolt/Passes/ContinuityStats.cpp - function cfg continuity analysis ---*-
-// C++ -*-===//
+//===- bolt/Passes/ContinuityStats.cpp --------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -7,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// Conduct function CFG continuity analysis.
+// This file implements the continuity stats calculation pass.
 //
 //===----------------------------------------------------------------------===//
 
@@ -27,29 +26,11 @@ using namespace bolt;
 
 namespace opts {
 extern cl::opt<unsigned> Verbosity;
-cl::opt<unsigned>
-    NumTopFunctions("num-top-functions",
-                    cl::desc("number of hottest functions to print aggregated "
-                             "CFG discontinuity stats of."),
-                    cl::init(1000), cl::ZeroOrMore, cl::Hidden,
-                    cl::cat(BoltOptCategory));
-cl::opt<bool>
-    PrintBucketedStats("print-bucketed-stats",
-                       cl::desc("print CFG discontinuity stats for the top "
-                                "functions divided into buckets "
-                                "based on their execution counts."),
-                       cl::Hidden, cl::cat(BoltCategory));
-cl::opt<unsigned>
-    NumFunctionsPerBucket("num-functions-per-bucket",
-                          cl::desc("maximum number of functions per bucket."),
-                          cl::init(500), cl::ZeroOrMore, cl::Hidden,
-                          cl::cat(BoltOptCategory));
-cl::opt<unsigned>
-    MinNumFunctions("min-num-functions",
-                    cl::desc("minimum number of hot functions in the binary to "
-                             "trigger profile CFG continuity check."),
-                    cl::init(5), cl::ZeroOrMore, cl::Hidden,
-                    cl::cat(BoltOptCategory));
+cl::opt<unsigned> NumFunctionsForContinuityCheck(
+    "num-functions-for-continuity-check",
+    cl::desc("number of hottest functions to print aggregated "
+             "CFG discontinuity stats of."),
+    cl::init(1000), cl::ZeroOrMore, cl::Hidden, cl::cat(BoltOptCategory));
 } // namespace opts
 
 namespace {
@@ -159,8 +140,7 @@ void printCFGContinuityStats(raw_ostream &OS,
     size_t SumUnreachableBBEC = SumAllBBEC - SumReachableBBEC;
     double FractionECUnreachable = (double)SumUnreachableBBEC / SumAllBBEC;
 
-    if (opts::Verbosity >= 2 && FractionECUnreachable > 0.1 &&
-        SumUnreachableBBEC > 50) {
+    if (opts::Verbosity >= 2 && FractionECUnreachable >= 0.05) {
       OS << "Non-trivial CFG discontinuity observed in function "
          << Function->getPrintName() << "\n";
       LLVM_DEBUG(Function->dump());
@@ -171,15 +151,17 @@ void printCFGContinuityStats(raw_ostream &OS,
     FractionECUnreachables.push_back(FractionECUnreachable);
   }
 
+  if (FractionECUnreachables.empty())
+    return;
+
+  size_t NumConsideredFunctions =
+      std::distance(Functions.begin(), Functions.end());
   if (!Verbose) {
-    if (FractionECUnreachables.empty()) {
-      OS << "no functions have more than 1 basic block and hence no CFG "
-            "discontinuity.\n";
-      return;
-    }
     std::sort(FractionECUnreachables.begin(), FractionECUnreachables.end());
     int Rank = int(FractionECUnreachables.size() * 0.95);
-    OS << format("the TOP 5%% function CFG discontinuity is %.2lf%%",
+    OS << format("BOLT-INFO: among the hottest %zu functions ",
+                 NumConsideredFunctions)
+       << format("the top 5%% function CFG discontinuity is %.2lf%%",
                  FractionECUnreachables[Rank] * 100)
        << "\n";
     return;
@@ -189,27 +171,23 @@ void printCFGContinuityStats(raw_ostream &OS,
                "least 2 basic blocks\n",
                SumECUnreachables.size(),
                100.0 * (double)SumECUnreachables.size() /
-                   (std::distance(Functions.begin(), Functions.end())));
+                   NumConsideredFunctions);
 
-  if (!NumUnreachables.empty()) {
-    OS << "- Distribution of NUM(unreachable POS BBs) among all focal "
-          "functions\n";
-    printDistribution(OS, NumUnreachables);
-  }
-  if (!SumECUnreachables.empty()) {
-    OS << "- Distribution of SUM(unreachable POS BBs) among all focal "
-          "functions\n";
-    printDistribution(OS, SumECUnreachables);
-  }
-  if (!FractionECUnreachables.empty()) {
-    OS << "- Distribution of [(SUM(unreachable POS BBs) / SUM(all "
-          "POS BBs))] among all focal functions\n";
-    printDistribution(OS, FractionECUnreachables, /*Fraction=*/true);
-  }
+  OS << "- Distribution of NUM(unreachable POS BBs) among all focal "
+        "functions\n";
+  printDistribution(OS, NumUnreachables);
+
+  OS << "- Distribution of SUM(unreachable POS BBs) among all focal "
+        "functions\n";
+  printDistribution(OS, SumECUnreachables);
+
+  OS << "- Distribution of [(SUM(unreachable POS BBs) / SUM(all "
+        "POS BBs))] among all focal functions\n";
+  printDistribution(OS, FractionECUnreachables, /*Fraction=*/true);
 }
 
 void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
-              size_t NumFunctionsPerBucket, size_t NumTopFunctions) {
+              size_t NumTopFunctions) {
   // Sort the list of functions by execution counts (reverse).
   llvm::sort(ValidFunctions,
              [&](const BinaryFunction *A, const BinaryFunction *B) {
@@ -217,25 +195,17 @@ void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
              });
 
   size_t RealNumTopFunctions = std::min(NumTopFunctions, ValidFunctions.size());
-  if (RealNumTopFunctions <= opts::MinNumFunctions)
-    return;
-  BC.outs() << format("BOLT-INFO: among the hottest %zu functions ",
-                      RealNumTopFunctions);
+
   iterator_range<function_iterator> Functions(
       ValidFunctions.begin(), ValidFunctions.begin() + RealNumTopFunctions);
   printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/false);
 
   // Print more detailed bucketed stats if requested.
-  if (opts::PrintBucketedStats) {
-    size_t PerBucketSize =
-        std::min(NumFunctionsPerBucket, ValidFunctions.size());
-    if (PerBucketSize == 0)
-      return;
-    size_t NumBuckets = RealNumTopFunctions / PerBucketSize +
-                        (RealNumTopFunctions % PerBucketSize != 0);
+  if (opts::Verbosity >= 1 && RealNumTopFunctions >= 5) {
+    size_t PerBucketSize = RealNumTopFunctions / 5;
     BC.outs() << format(
-        "Detailed stats for %zu buckets, each with at most %zu functions:\n",
-        NumBuckets, PerBucketSize);
+        "Detailed stats for 5 buckets, each with  %zu functions:\n",
+        PerBucketSize);
     BC.outs() << "For each considered function, identify positive "
                  "execution-count basic blocks\n"
               << "(abbr. POS BBs) that are *unreachable* from the function "
@@ -244,10 +214,9 @@ void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
 
     // For each bucket, print the CFG continuity stats of the functions in the
     // bucket.
-    for (size_t BucketIndex = 0; BucketIndex < NumBuckets; ++BucketIndex) {
+    for (size_t BucketIndex = 0; BucketIndex < 5; ++BucketIndex) {
       const size_t StartIndex = BucketIndex * PerBucketSize;
-      size_t EndIndex = std::min(StartIndex + PerBucketSize, NumTopFunctions);
-      EndIndex = std::min(EndIndex, ValidFunctions.size());
+      size_t EndIndex = StartIndex + PerBucketSize;
       iterator_range<function_iterator> Functions(
           ValidFunctions.begin() + StartIndex,
           ValidFunctions.begin() + EndIndex);
@@ -282,7 +251,9 @@ Error PrintContinuityStats::runOnFunctions(BinaryContext &BC) {
     if (PrintContinuityStats::shouldOptimize(*Function))
       ValidFunctions.push_back(Function);
   }
-  printAll(BC, ValidFunctions, opts::NumFunctionsPerBucket,
-           opts::NumTopFunctions);
+  if (ValidFunctions.empty() || opts::NumFunctionsForContinuityCheck == 0)
+    return Error::success();
+
+  printAll(BC, ValidFunctions, opts::NumFunctionsForContinuityCheck);
   return Error::success();
 }
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index 116a2a0ab4361c..b0906041833484 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -155,11 +155,6 @@ static cl::opt<bool>
                       cl::desc("print profile quality/bias analysis"),
                       cl::cat(BoltCategory));
 
-static cl::opt<bool> PrintContinuityStats(
-    "print-continuity-stats",
-    cl::desc("print profile function CFG continuity stats"),
-    cl::cat(BoltCategory));
-
 static cl::opt<bool>
     PrintRegReAssign("print-regreassign",
                      cl::desc("print functions after regreassign pass"),
@@ -379,8 +374,7 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   if (opts::PrintProfileStats)
     Manager.registerPass(std::make_unique<PrintProfileStats>(NeverPrint));
 
-  if (opts::PrintContinuityStats)
-    Manager.registerPass(std::make_unique<PrintContinuityStats>(NeverPrint));
+  Manager.registerPass(std::make_unique<PrintContinuityStats>(NeverPrint));
 
   Manager.registerPass(std::make_unique<ValidateInternalCalls>(NeverPrint));
 
diff --git a/bolt/test/X86/cfg-discontinuity-reporting.test b/bolt/test/X86/cfg-discontinuity-reporting.test
index 563d55c3013ff8..2616ebc89fabd0 100644
--- a/bolt/test/X86/cfg-discontinuity-reporting.test
+++ b/bolt/test/X86/cfg-discontinuity-reporting.test
@@ -1,5 +1,4 @@
 ## Check profile discontinuity reporting
 RUN: yaml2obj %p/Inputs/blarge_new.yaml &> %t.exe
-RUN: llvm-bolt %t.exe -o %t.out --pa -p %p/Inputs/blarge_new.preagg.txt \
-RUN:   --print-continuity-stats --min-num-functions=1 | FileCheck %s
-CHECK: among the hottest 5 functions the TOP 5% function CFG discontinuity is 100.00%
+RUN: llvm-bolt %t.exe -o %t.out --pa -p %p/Inputs/blarge_new.preagg.txt | FileCheck %s
+CHECK: among the hottest 5 functions the top 5% function CFG discontinuity is 100.00%

>From 424f06274ba3b6e9f2118480236f802a67adeec2 Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Thu, 3 Oct 2024 14:17:24 -0700
Subject: [PATCH 6/7] change typo

---
 bolt/include/bolt/Passes/ContinuityStats.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bolt/include/bolt/Passes/ContinuityStats.h b/bolt/include/bolt/Passes/ContinuityStats.h
index ed149535e1b115..bd4d491ad4a55b 100644
--- a/bolt/include/bolt/Passes/ContinuityStats.h
+++ b/bolt/include/bolt/Passes/ContinuityStats.h
@@ -21,7 +21,7 @@
 // satisfies the CFG continuity property.
 
 // The default value of 1000 above can be changed via the hidden BOLT option
-// `-num-hottest-functions-for-continuity-check=[N]`.
+// `-num-functions-for-continuity-check=[N]`.
 // If more detailed stats are needed, `-v=1` can be used: the hottest N
 // functions will be grouped into 5 equally-sized buckets, from the hottest
 // to the coldest; for each bucket, various summary statistics of the

>From 4216fb6a7467a770d1640f9e178e628c7d523f56 Mon Sep 17 00:00:00 2001
From: Shatian Wang <shatian at meta.com>
Date: Tue, 8 Oct 2024 13:10:00 -0700
Subject: [PATCH 7/7] Addressed additional comments about usage of Verbose &
 add const qualifiers when applicable

---
 bolt/lib/Passes/ContinuityStats.cpp           | 99 +++++++++----------
 .../test/X86/cfg-discontinuity-reporting.test |  2 +-
 2 files changed, 46 insertions(+), 55 deletions(-)

diff --git a/bolt/lib/Passes/ContinuityStats.cpp b/bolt/lib/Passes/ContinuityStats.cpp
index 4492a2e05a2229..b32365b59065dc 100644
--- a/bolt/lib/Passes/ContinuityStats.cpp
+++ b/bolt/lib/Passes/ContinuityStats.cpp
@@ -60,7 +60,7 @@ void printDistribution(raw_ostream &OS, std::vector<T> &values,
   };
 
   printLine("MAX", 0);
-  int percentages[] = {1, 5, 10, 20, 50, 80};
+  const int percentages[] = {1, 5, 10, 20, 50, 80};
   for (size_t i = 0; i < sizeof(percentages) / sizeof(percentages[0]); ++i) {
     printLine("TOP " + std::to_string(percentages[i]) + "%", percentages[i]);
   }
@@ -68,8 +68,7 @@ void printDistribution(raw_ostream &OS, std::vector<T> &values,
 }
 
 void printCFGContinuityStats(raw_ostream &OS,
-                             iterator_range<function_iterator> &Functions,
-                             bool Verbose = false) {
+                             iterator_range<function_iterator> &Functions) {
   // Given a perfect profile, every positive-execution-count BB should be
   // connected to an entry of the function through a positive-execution-count
   // directed path in the control flow graph.
@@ -86,7 +85,7 @@ void printCFGContinuityStats(raw_ostream &OS,
     size_t NumPosECBBs = 0;
     size_t SumAllBBEC = 0;
     for (const BinaryBasicBlock &BB : *Function) {
-      size_t BBEC = BB.getKnownExecutionCount();
+      const size_t BBEC = BB.getKnownExecutionCount();
       NumPosECBBs += BBEC > 0 ? 1 : 0;
       SumAllBBEC += BBEC;
     }
@@ -107,12 +106,12 @@ void printCFGContinuityStats(raw_ostream &OS,
       }
     }
     while (!Queue.empty()) {
-      unsigned BBIndex = Queue.front();
+      const unsigned BBIndex = Queue.front();
       const BinaryBasicBlock *BB = IndexToBB[BBIndex];
       Queue.pop();
       auto SuccBIIter = BB->branch_info_begin();
-      for (BinaryBasicBlock *Succ : BB->successors()) {
-        uint64_t Count = SuccBIIter->Count;
+      for (const BinaryBasicBlock *Succ : BB->successors()) {
+        const uint64_t Count = SuccBIIter->Count;
         if (Count == BinaryBasicBlock::COUNT_NO_PROFILE || Count == 0) {
           ++SuccBIIter;
           continue;
@@ -126,19 +125,21 @@ void printCFGContinuityStats(raw_ostream &OS,
       }
     }
 
-    size_t NumReachableBBs = Visited.size();
+    const size_t NumReachableBBs = Visited.size();
 
     // Loop through Visited, and sum the corresponding BBs' execution counts
     // (ECs).
     size_t SumReachableBBEC = 0;
-    for (unsigned BBIndex : Visited) {
+    for (const unsigned BBIndex : Visited) {
       const BinaryBasicBlock *BB = IndexToBB[BBIndex];
       SumReachableBBEC += BB->getKnownExecutionCount();
     }
 
-    size_t NumPosECBBsUnreachableFromEntry = NumPosECBBs - NumReachableBBs;
-    size_t SumUnreachableBBEC = SumAllBBEC - SumReachableBBEC;
-    double FractionECUnreachable = (double)SumUnreachableBBEC / SumAllBBEC;
+    const size_t NumPosECBBsUnreachableFromEntry =
+        NumPosECBBs - NumReachableBBs;
+    const size_t SumUnreachableBBEC = SumAllBBEC - SumReachableBBEC;
+    const double FractionECUnreachable =
+        (double)SumUnreachableBBEC / SumAllBBEC;
 
     if (opts::Verbosity >= 2 && FractionECUnreachable >= 0.05) {
       OS << "Non-trivial CFG discontinuity observed in function "
@@ -154,36 +155,25 @@ void printCFGContinuityStats(raw_ostream &OS,
   if (FractionECUnreachables.empty())
     return;
 
-  size_t NumConsideredFunctions =
-      std::distance(Functions.begin(), Functions.end());
-  if (!Verbose) {
-    std::sort(FractionECUnreachables.begin(), FractionECUnreachables.end());
-    int Rank = int(FractionECUnreachables.size() * 0.95);
-    OS << format("BOLT-INFO: among the hottest %zu functions ",
-                 NumConsideredFunctions)
-       << format("the top 5%% function CFG discontinuity is %.2lf%%",
-                 FractionECUnreachables[Rank] * 100)
-       << "\n";
-    return;
-  }
-
-  OS << format("Focus on %zu (%.2lf%%) of considered functions that have at "
-               "least 2 basic blocks\n",
-               SumECUnreachables.size(),
-               100.0 * (double)SumECUnreachables.size() /
-                   NumConsideredFunctions);
+  std::sort(FractionECUnreachables.begin(), FractionECUnreachables.end());
+  const int Rank = int(FractionECUnreachables.size() * 0.95);
+  OS << format("top 5%% function CFG discontinuity is %.2lf%%\n",
+               FractionECUnreachables[Rank] * 100);
 
-  OS << "- Distribution of NUM(unreachable POS BBs) among all focal "
-        "functions\n";
-  printDistribution(OS, NumUnreachables);
+  if (opts::Verbosity >= 1) {
+    OS << "abbreviations: EC = execution count, POS BBs = positive EC BBs\n"
+       << "distribution of NUM(unreachable POS BBs) among all focal "
+          "functions\n";
+    printDistribution(OS, NumUnreachables);
 
-  OS << "- Distribution of SUM(unreachable POS BBs) among all focal "
-        "functions\n";
-  printDistribution(OS, SumECUnreachables);
+    OS << "distribution of SUM_EC(unreachable POS BBs) among all focal "
+          "functions\n";
+    printDistribution(OS, SumECUnreachables);
 
-  OS << "- Distribution of [(SUM(unreachable POS BBs) / SUM(all "
-        "POS BBs))] among all focal functions\n";
-  printDistribution(OS, FractionECUnreachables, /*Fraction=*/true);
+    OS << "distribution of [(SUM_EC(unreachable POS BBs) / SUM_EC(all "
+          "POS BBs))] among all focal functions\n";
+    printDistribution(OS, FractionECUnreachables, /*Fraction=*/true);
+  }
 }
 
 void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
@@ -194,29 +184,28 @@ void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
                return A->getKnownExecutionCount() > B->getKnownExecutionCount();
              });
 
-  size_t RealNumTopFunctions = std::min(NumTopFunctions, ValidFunctions.size());
+  const size_t RealNumTopFunctions =
+      std::min(NumTopFunctions, ValidFunctions.size());
 
   iterator_range<function_iterator> Functions(
       ValidFunctions.begin(), ValidFunctions.begin() + RealNumTopFunctions);
-  printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/false);
+
+  BC.outs() << format("BOLT-INFO: among the hottest %zu functions ",
+                      RealNumTopFunctions);
+  printCFGContinuityStats(BC.outs(), Functions);
 
   // Print more detailed bucketed stats if requested.
   if (opts::Verbosity >= 1 && RealNumTopFunctions >= 5) {
-    size_t PerBucketSize = RealNumTopFunctions / 5;
+    const size_t PerBucketSize = RealNumTopFunctions / 5;
     BC.outs() << format(
         "Detailed stats for 5 buckets, each with  %zu functions:\n",
         PerBucketSize);
-    BC.outs() << "For each considered function, identify positive "
-                 "execution-count basic blocks\n"
-              << "(abbr. POS BBs) that are *unreachable* from the function "
-                 "entry through a\n"
-              << "positive-execution-count path.\n";
 
     // For each bucket, print the CFG continuity stats of the functions in the
     // bucket.
     for (size_t BucketIndex = 0; BucketIndex < 5; ++BucketIndex) {
       const size_t StartIndex = BucketIndex * PerBucketSize;
-      size_t EndIndex = StartIndex + PerBucketSize;
+      const size_t EndIndex = StartIndex + PerBucketSize;
       iterator_range<function_iterator> Functions(
           ValidFunctions.begin() + StartIndex,
           ValidFunctions.begin() + EndIndex);
@@ -225,12 +214,14 @@ void printAll(BinaryContext &BC, FunctionListType &ValidFunctions,
       const size_t MinFunctionExecutionCount =
           ValidFunctions[EndIndex - 1]->getKnownExecutionCount();
       BC.outs() << format("----------------\n|   Bucket %zu:  "
-                          "|\n----------------\nExecution counts of the %zu "
-                          "functions in the bucket: "
-                          "%zu-%zu\n",
-                          BucketIndex + 1, EndIndex - StartIndex,
-                          MinFunctionExecutionCount, MaxFunctionExecutionCount);
-      printCFGContinuityStats(BC.outs(), Functions, /*Verbose=*/true);
+                          "|\n----------------\n",
+                          BucketIndex + 1)
+                << format(
+                       "execution counts of the %zu functions in the bucket: "
+                       "%zu-%zu\n",
+                       EndIndex - StartIndex, MinFunctionExecutionCount,
+                       MaxFunctionExecutionCount);
+      printCFGContinuityStats(BC.outs(), Functions);
     }
   }
 }
diff --git a/bolt/test/X86/cfg-discontinuity-reporting.test b/bolt/test/X86/cfg-discontinuity-reporting.test
index 2616ebc89fabd0..4d7d3305cdb751 100644
--- a/bolt/test/X86/cfg-discontinuity-reporting.test
+++ b/bolt/test/X86/cfg-discontinuity-reporting.test
@@ -1,4 +1,4 @@
 ## Check profile discontinuity reporting
 RUN: yaml2obj %p/Inputs/blarge_new.yaml &> %t.exe
 RUN: llvm-bolt %t.exe -o %t.out --pa -p %p/Inputs/blarge_new.preagg.txt | FileCheck %s
-CHECK: among the hottest 5 functions the top 5% function CFG discontinuity is 100.00%
+CHECK: among the hottest 5 functions top 5% function CFG discontinuity is 100.00%



More information about the llvm-commits mailing list