commit 86526084044167b3c753d32ef8dbf79d57cba0c4
author Amir Ayupov <aaupov@fb.com> Sat Dec 14 22:38:24 2024 -0800
committer GitHub <noreply@github.com> Sat Dec 14 22:38:24 2024 -0800
tree 48f10b5761fa194b4ac61767e3be0bc552074ac5
parent 97f43364cc8599bfc64f4f83fb81c7cd0242a1a4

[BOLT] Fix counts aggregation in merge-fdata (#119652)

merge-fdata used to consider misprediction count as part of "signature",
or the aggregation key. This prevented it from collapsing profile lines
with different misprediction counts, which resulted in duplicate
`(from, to)` pairs with different misprediction and execution counts.

Fix that by splitting out misprediction count and accumulating it
separately.

Test Plan: updated bolt/test/merge-fdata-lbr-mode.test

bolt/test/merge-fdata-lbr-mode.test

diff --git a/bolt/test/merge-fdata-lbr-mode.test b/bolt/test/merge-fdata-lbr-mode.test
index 268502a..2cd3853 100644
--- a/bolt/test/merge-fdata-lbr-mode.test
+++ b/bolt/test/merge-fdata-lbr-mode.test
@@ -7,9 +7,9 @@
 # RUN: FileCheck %s --input-file %t/merged.fdata
 
 # CHECK-NOT: no_lbr
-# CHECK: main 2
+# CHECK: 1 main 0 1 main 2 1 3
 
 #--- a.fdata
-main 1
+1 main 0 1 main 2 0 1
 #--- b.fdata
-main 1
+1 main 0 1 main 2 1 2

bolt/tools/merge-fdata/merge-fdata.cpp

diff --git a/bolt/tools/merge-fdata/merge-fdata.cpp b/bolt/tools/merge-fdata/merge-fdata.cpp
index 318ef09..74a5f8c 100644
--- a/bolt/tools/merge-fdata/merge-fdata.cpp
+++ b/bolt/tools/merge-fdata/merge-fdata.cpp
@@ -268,7 +268,17 @@
   std::optional<bool> BoltedCollection;
   std::optional<bool> NoLBRCollection;
   std::mutex BoltedCollectionMutex;
-  typedef StringMap<uint64_t> ProfileTy;
+  struct CounterTy {
+    uint64_t Exec{0};
+    uint64_t Mispred{0};
+    CounterTy &operator+=(const CounterTy &O) {
+      Exec += O.Exec;
+      Mispred += O.Mispred;
+      return *this;
+    }
+    CounterTy operator+(const CounterTy &O) { return *this += O; }
+  };
+  typedef StringMap<CounterTy> ProfileTy;
 
   auto ParseProfile = [&](const std::string &Filename, auto &Profiles) {
     const llvm::thread::id tid = llvm::this_thread::get_id();
@@ -305,13 +315,18 @@
 
     do {
       StringRef Line(FdataLine);
-      size_t Pos = Line.rfind(" ");
-      if (Pos == StringRef::npos)
-        report_error(Filename, "Malformed / corrupted profile");
-      StringRef Signature = Line.substr(0, Pos);
-      uint64_t Count;
-      if (Line.substr(Pos + 1, Line.size() - Pos).getAsInteger(10, Count))
-        report_error(Filename, "Malformed / corrupted profile counter");
+      CounterTy Count;
+      auto [Signature, ExecCount] = Line.rsplit(' ');
+      if (ExecCount.getAsInteger(10, Count.Exec))
+        report_error(Filename, "Malformed / corrupted execution count");
+      // Only LBR profile has misprediction field
+      if (!NoLBRCollection.value_or(false)) {
+        auto [SignatureLBR, MispredCount] = Signature.rsplit(' ');
+        Signature = SignatureLBR;
+        if (MispredCount.getAsInteger(10, Count.Mispred))
+          report_error(Filename, "Malformed / corrupted misprediction count");
+      }
+
       Count += Profile->lookup(Signature);
       Profile->insert_or_assign(Signature, Count);
     } while (std::getline(FdataFile, FdataLine));
@@ -331,7 +346,7 @@
   ProfileTy MergedProfile;
   for (const auto &[Thread, Profile] : ParsedProfiles)
     for (const auto &[Key, Value] : Profile) {
-      uint64_t Count = MergedProfile.lookup(Key) + Value;
+      CounterTy Count = MergedProfile.lookup(Key) + Value;
       MergedProfile.insert_or_assign(Key, Count);
     }
 
@@ -339,8 +354,12 @@
     output() << "boltedcollection\n";
   if (NoLBRCollection.value_or(false))
     output() << "no_lbr\n";
-  for (const auto &[Key, Value] : MergedProfile)
-    output() << Key << " " << Value << "\n";
+  for (const auto &[Key, Value] : MergedProfile) {
+    output() << Key << " ";
+    if (!NoLBRCollection.value_or(false))
+      output() << Value.Mispred << " ";
+    output() << Value.Exec << "\n";
+  }
 
   errs() << "Profile from " << Filenames.size() << " files merged.\n";
 }