fixup! fixup! fixup! fixup! [BOLT] Profile quality stats -- CFG discontinuity

ShatianWang · ShatianWang · commit 2da1163dcc35 · 2024-10-03T08:37:09.000-07:00
diff --git a/bolt/include/bolt/Passes/ContinuityStats.h b/bolt/include/bolt/Passes/ContinuityStats.h
@@ -1,13 +1,32 @@
-//===- 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.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
-// 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
@@ -1,13 +1,12 @@
-//===- 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.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
-// 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,53 +171,41 @@ 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) {
                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);
+  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
@@ -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
@@ -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%
