[PATCH] D120696: [BOLT][NFC] Fix getDynoStats to handle BCs with no functions

[Amir Ayupov via Phabricator via llvm-commits]

Amir created this revision.
Herald added a reviewer: rafauler.
Herald added a subscriber: ayermolo.
Herald added a reviewer: maksfb.
Amir requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120696

Files:
  bolt/include/bolt/Core/DynoStats.h
  bolt/include/bolt/Passes/BinaryPasses.h
  bolt/lib/Rewrite/BinaryPassManager.cpp
  bolt/test/Inputs/bc-zero-funcs
  bolt/test/zero-funcs.test


Index: bolt/test/zero-funcs.test
===================================================================
--- /dev/null
+++ bolt/test/zero-funcs.test
@@ -0,0 +1,3 @@
+# This reproduces a bug triggered by invoking getDynoStats on a BC with zero
+# BinaryFunctions
+RUN: llvm-bolt %p/Inputs/bc-zero-funcs -o /dev/null
Index: bolt/lib/Rewrite/BinaryPassManager.cpp
===================================================================
--- bolt/lib/Rewrite/BinaryPassManager.cpp
+++ bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -276,8 +276,9 @@
     NamedRegionTimer T(Pass->getName(), Pass->getName(), TimerGroupName,
                        TimerGroupDesc, TimeOpts);
 
-    callWithDynoStats([this, &Pass] { Pass->runOnFunctions(BC); }, BFs,
-                      Pass->getName(), opts::DynoStatsAll);
+    callWithDynoStats(
+        BC, [this, &Pass] { Pass->runOnFunctions(BC); }, BFs, Pass->getName(),
+        opts::DynoStatsAll);
 
     if (opts::VerifyCFG &&
         !std::accumulate(
@@ -316,7 +317,7 @@
 void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   BinaryFunctionPassManager Manager(BC);
 
-  const DynoStats InitialDynoStats = getDynoStats(BC.getBinaryFunctions());
+  const DynoStats InitialDynoStats = getDynoStats(BC, BC.getBinaryFunctions());
 
   Manager.registerPass(std::make_unique<AsmDumpPass>(),
                        opts::AsmDump.getNumOccurrences());
Index: bolt/include/bolt/Passes/BinaryPasses.h
===================================================================
--- bolt/include/bolt/Passes/BinaryPasses.h
+++ bolt/include/bolt/Passes/BinaryPasses.h
@@ -71,7 +71,7 @@
   bool shouldPrint(const BinaryFunction &BF) const override { return false; }
 
   void runOnFunctions(BinaryContext &BC) override {
-    const DynoStats NewDynoStats = getDynoStats(BC.getBinaryFunctions());
+    const DynoStats NewDynoStats = getDynoStats(BC, BC.getBinaryFunctions());
     const bool Changed = (NewDynoStats != PrevDynoStats);
     outs() << "BOLT-INFO: program-wide dynostats " << Title
            << (Changed ? "" : " (no change)") << ":\n\n"
Index: bolt/include/bolt/Core/DynoStats.h
===================================================================
--- bolt/include/bolt/Core/DynoStats.h
+++ bolt/include/bolt/Core/DynoStats.h
@@ -14,6 +14,7 @@
 #ifndef BOLT_CORE_DYNO_STATS_H
 #define BOLT_CORE_DYNO_STATS_H
 
+#include "bolt/Core/BinaryContext.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/MC/MCInstPrinter.h"
 #include "llvm/Support/raw_ostream.h"
@@ -146,9 +147,8 @@
 
 /// Return program-wide dynostats.
 template <typename FuncsType>
-inline DynoStats getDynoStats(const FuncsType &Funcs) {
-  bool IsAArch64 = Funcs.begin()->second.getBinaryContext().isAArch64();
-  DynoStats dynoStats(IsAArch64);
+inline DynoStats getDynoStats(const BinaryContext &BC, const FuncsType &Funcs) {
+  DynoStats dynoStats(BC.isAArch64());
   for (auto &BFI : Funcs) {
     auto &BF = BFI.second;
     if (BF.isSimple())
@@ -159,17 +159,17 @@
 
 /// Call a function with optional before and after dynostats printing.
 template <typename FnType, typename FuncsType>
-inline void callWithDynoStats(FnType &&Func, const FuncsType &Funcs,
-                              StringRef Phase, const bool Flag) {
-  bool IsAArch64 = Funcs.begin()->second.getBinaryContext().isAArch64();
-  DynoStats DynoStatsBefore(IsAArch64);
+inline void callWithDynoStats(const BinaryContext &BC, FnType &&Func,
+                              const FuncsType &Funcs, StringRef Phase,
+                              const bool Flag) {
+  DynoStats DynoStatsBefore(BC.isAArch64());
   if (Flag)
-    DynoStatsBefore = getDynoStats(Funcs);
+    DynoStatsBefore = getDynoStats(BC, Funcs);
 
   Func();
 
   if (Flag) {
-    const DynoStats DynoStatsAfter = getDynoStats(Funcs);
+    const DynoStats DynoStatsAfter = getDynoStats(BC, Funcs);
     const bool Changed = (DynoStatsAfter != DynoStatsBefore);
     outs() << "BOLT-INFO: program-wide dynostats after running " << Phase
            << (Changed ? "" : " (no change)") << ":\n\n"


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D120696.411957.patch
Type: text/x-patch
Size: 4041 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220301/3c44eb37/attachment.bin>