[llvm-branch-commits] [llvm] [BOLT] Fix counts aggregation in merge-fdata (PR #119652)

[Amir Ayupov via llvm-branch-commits]

https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/119652

>From cc7cea0276fef36e896ecef149ef680b66bb9c1f Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 11 Dec 2024 19:11:07 -0800
Subject: [PATCH 1/2] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
 =?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.4
---
 bolt/test/merge-fdata.test             |  8 ++++++
 bolt/tools/merge-fdata/merge-fdata.cpp | 39 ++++++++++++++++++--------
 2 files changed, 36 insertions(+), 11 deletions(-)
 create mode 100644 bolt/test/merge-fdata.test

diff --git a/bolt/test/merge-fdata.test b/bolt/test/merge-fdata.test
new file mode 100644
index 00000000000000..0ea0f2cb4398b6
--- /dev/null
+++ b/bolt/test/merge-fdata.test
@@ -0,0 +1,8 @@
+## Reproduces an issue where counts were not accumulated by merge-fdata
+
+# RUN: split-file %s %t
+# RUN: merge-fdata %t/fdata | FileCheck %s
+# CHECK: 1 main 10 1 main 1a 1 20
+#--- fdata
+1 main 10 1 main 1a 0 10
+1 main 10 1 main 1a 1 10
diff --git a/bolt/tools/merge-fdata/merge-fdata.cpp b/bolt/tools/merge-fdata/merge-fdata.cpp
index 89ca46c1c0a8fa..a74e9099df9059 100644
--- a/bolt/tools/merge-fdata/merge-fdata.cpp
+++ b/bolt/tools/merge-fdata/merge-fdata.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ThreadPool.h"
 #include <algorithm>
@@ -266,7 +267,19 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
   errs() << "Using legacy profile format.\n";
   std::optional<bool> BoltedCollection;
   std::mutex BoltedCollectionMutex;
-  typedef StringMap<uint64_t> ProfileTy;
+  constexpr static const char *const FdataCountersPattern =
+      "(.*) ([0-9]+) ([0-9]+)";
+  Regex FdataRegex(FdataCountersPattern);
+  struct CounterTy {
+    uint64_t Count;
+    uint64_t MispredCount;
+    CounterTy &operator+(const CounterTy &O) {
+      Count += O.Count;
+      MispredCount += O.MispredCount;
+      return *this;
+    }
+  };
+  typedef StringMap<CounterTy> ProfileTy;
 
   auto ParseProfile = [&](const std::string &Filename, auto &Profiles) {
     const llvm::thread::id tid = llvm::this_thread::get_id();
@@ -304,15 +317,19 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
     SmallVector<StringRef> Lines;
     SplitString(Buf, Lines, "\n");
     for (StringRef Line : Lines) {
-      size_t Pos = Line.rfind(" ");
-      if (Pos == StringRef::npos)
+      CounterTy CurrCount;
+      SmallVector<StringRef, 4> Fields;
+      if (!FdataRegex.match(Line, &Fields))
         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");
-      Count += Profile->lookup(Signature);
-      Profile->insert_or_assign(Signature, Count);
+      StringRef Signature = Fields[1];
+      if (Fields[2].getAsInteger(10, CurrCount.MispredCount))
+        report_error(Filename, "Malformed / corrupted execution count");
+      if (Fields[3].getAsInteger(10, CurrCount.Count))
+        report_error(Filename, "Malformed / corrupted misprediction count");
+
+      CounterTy Counter = Profile->lookup(Signature);
+      Counter = Counter + CurrCount;
+      Profile->insert_or_assign(Signature, Counter);
     }
   };
 
@@ -330,14 +347,14 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
   ProfileTy MergedProfile;
   for (const auto &[Thread, Profile] : ParsedProfiles)
     for (const auto &[Key, Value] : Profile) {
-      uint64_t Count = MergedProfile.lookup(Key) + Value;
+      auto Count = MergedProfile.lookup(Key) + Value;
       MergedProfile.insert_or_assign(Key, Count);
     }
 
   if (BoltedCollection.value_or(false))
     output() << "boltedcollection\n";
   for (const auto &[Key, Value] : MergedProfile)
-    output() << Key << " " << Value << "\n";
+    output() << Key << " " << Value.MispredCount << " " << Value.Count << "\n";
 
   errs() << "Profile from " << Filenames.size() << " files merged.\n";
 }

>From 040deec056483f8268d134ce826a20f1e511dcd4 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Wed, 11 Dec 2024 19:22:23 -0800
Subject: [PATCH 2/2] simplify

Created using spr 1.3.4
---
 bolt/tools/merge-fdata/merge-fdata.cpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/bolt/tools/merge-fdata/merge-fdata.cpp b/bolt/tools/merge-fdata/merge-fdata.cpp
index a74e9099df9059..83616e1de99254 100644
--- a/bolt/tools/merge-fdata/merge-fdata.cpp
+++ b/bolt/tools/merge-fdata/merge-fdata.cpp
@@ -271,13 +271,14 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
       "(.*) ([0-9]+) ([0-9]+)";
   Regex FdataRegex(FdataCountersPattern);
   struct CounterTy {
-    uint64_t Count;
-    uint64_t MispredCount;
-    CounterTy &operator+(const CounterTy &O) {
-      Count += O.Count;
-      MispredCount += O.MispredCount;
+    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;
 
@@ -317,19 +318,18 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
     SmallVector<StringRef> Lines;
     SplitString(Buf, Lines, "\n");
     for (StringRef Line : Lines) {
-      CounterTy CurrCount;
       SmallVector<StringRef, 4> Fields;
       if (!FdataRegex.match(Line, &Fields))
         report_error(Filename, "Malformed / corrupted profile");
       StringRef Signature = Fields[1];
-      if (Fields[2].getAsInteger(10, CurrCount.MispredCount))
+      CounterTy Count;
+      if (Fields[2].getAsInteger(10, Count.Mispred))
         report_error(Filename, "Malformed / corrupted execution count");
-      if (Fields[3].getAsInteger(10, CurrCount.Count))
+      if (Fields[3].getAsInteger(10, Count.Exec))
         report_error(Filename, "Malformed / corrupted misprediction count");
 
-      CounterTy Counter = Profile->lookup(Signature);
-      Counter = Counter + CurrCount;
-      Profile->insert_or_assign(Signature, Counter);
+      Count += Profile->lookup(Signature);
+      Profile->insert_or_assign(Signature, Count);
     }
   };
 
@@ -354,7 +354,7 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
   if (BoltedCollection.value_or(false))
     output() << "boltedcollection\n";
   for (const auto &[Key, Value] : MergedProfile)
-    output() << Key << " " << Value.MispredCount << " " << Value.Count << "\n";
+    output() << Key << " " << Value.Mispred << " " << Value.Exec << "\n";
 
   errs() << "Profile from " << Filenames.size() << " files merged.\n";
 }