[llvm] [BOLT][merge-fdata]Fix support for fdata files starting with no_lbr on ARM platform (PR #112328)

via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 15 00:54:09 PDT 2024


https://github.com/zrxxzz created https://github.com/llvm/llvm-project/pull/112328

The merge-fdata tool did not support fdata files starting with no_lbr, which was not user-friendly for the ARM platform.
Therefore, I fixed this issue to enable its use on the ARM platform (with no_lbr). Additionally, I validated the merging of samples across both LBR and Non-LBR scenarios.

>From 880916dfc73f0b8a2647be221541ece9c75c6f46 Mon Sep 17 00:00:00 2001
From: ranxinzhong <ranxinzhong at tencent.com>
Date: Tue, 15 Oct 2024 15:08:45 +0800
Subject: [PATCH 1/2] The merge-fdata tool did not support fdata files starting
 with no_lbr, which was not user-friendly for the ARM platform. Therefore, I
 fixed this issue to enable its use on the ARM platform (with no_lbr).
 Additionally, I validated the merging of samples across both LBR and Non-LBR
 scenarios.

---
 bolt/tools/merge-fdata/merge-fdata.cpp | 83 +++++++++++++++-----------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/bolt/tools/merge-fdata/merge-fdata.cpp b/bolt/tools/merge-fdata/merge-fdata.cpp
index 89ca46c1c0a8fa..ef85fd345d4505 100644
--- a/bolt/tools/merge-fdata/merge-fdata.cpp
+++ b/bolt/tools/merge-fdata/merge-fdata.cpp
@@ -34,45 +34,32 @@ cl::OptionCategory MergeFdataCategory("merge-fdata options");
 
 enum SortType : char {
   ST_NONE,
-  ST_EXEC_COUNT,      /// Sort based on function execution count.
-  ST_TOTAL_BRANCHES,  /// Sort based on all branches in the function.
+  ST_EXEC_COUNT,     /// Sort based on function execution count.
+  ST_TOTAL_BRANCHES, /// Sort based on all branches in the function.
 };
 
 static cl::list<std::string>
-InputDataFilenames(
-  cl::Positional,
-  cl::CommaSeparated,
-  cl::desc("<fdata1> [<fdata2>]..."),
-  cl::OneOrMore,
-  cl::cat(MergeFdataCategory));
-
-static cl::opt<SortType>
-PrintFunctionList("print",
-  cl::desc("print the list of objects with count to stderr"),
-  cl::init(ST_NONE),
-  cl::values(clEnumValN(ST_NONE,
-      "none",
-      "do not print objects/functions"),
-    clEnumValN(ST_EXEC_COUNT,
-      "exec",
-      "print functions sorted by execution count"),
-    clEnumValN(ST_TOTAL_BRANCHES,
-      "branches",
-      "print functions sorted by total branch count")),
-  cl::cat(MergeFdataCategory));
-
-static cl::opt<bool>
-SuppressMergedDataOutput("q",
-  cl::desc("do not print merged data to stdout"),
-  cl::init(false),
-  cl::Optional,
-  cl::cat(MergeFdataCategory));
-
-static cl::opt<std::string>
-OutputFilePath("o",
-  cl::value_desc("file"),
-  cl::desc("Write output to <file>"),
-  cl::cat(MergeFdataCategory));
+    InputDataFilenames(cl::Positional, cl::CommaSeparated,
+                       cl::desc("<fdata1> [<fdata2>]..."), cl::OneOrMore,
+                       cl::cat(MergeFdataCategory));
+
+static cl::opt<SortType> PrintFunctionList(
+    "print", cl::desc("print the list of objects with count to stderr"),
+    cl::init(ST_NONE),
+    cl::values(clEnumValN(ST_NONE, "none", "do not print objects/functions"),
+               clEnumValN(ST_EXEC_COUNT, "exec",
+                          "print functions sorted by execution count"),
+               clEnumValN(ST_TOTAL_BRANCHES, "branches",
+                          "print functions sorted by total branch count")),
+    cl::cat(MergeFdataCategory));
+
+static cl::opt<bool> SuppressMergedDataOutput(
+    "q", cl::desc("do not print merged data to stdout"), cl::init(false),
+    cl::Optional, cl::cat(MergeFdataCategory));
+
+static cl::opt<std::string> OutputFilePath("o", cl::value_desc("file"),
+                                           cl::desc("Write output to <file>"),
+                                           cl::cat(MergeFdataCategory));
 
 } // namespace opts
 
@@ -265,7 +252,10 @@ bool isYAML(const StringRef Filename) {
 void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
   errs() << "Using legacy profile format.\n";
   std::optional<bool> BoltedCollection;
+  std::optional<bool> NoLbr;
   std::mutex BoltedCollectionMutex;
+  std::mutex NoLbrMutex;
+  std::string NoLbrLabel;
   typedef StringMap<uint64_t> ProfileTy;
 
   auto ParseProfile = [&](const std::string &Filename, auto &Profiles) {
@@ -298,6 +288,25 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
         BoltedCollection = false;
       }
 
+      std::lock_guard<std::mutex> Lock1(NoLbrMutex);
+      // Check if the string "no_lbr" is in the first line
+      if (Buf.starts_with("no_lbr")) {
+        if (!NoLbr.value_or(true))
+          report_error(
+              Filename,
+              "cannot mix profile collected on LBR and non-LBR architectures");
+        NoLbr = true;
+        size_t Pos = Buf.find("\n");
+        NoLbrLabel = Buf.substr(0, Pos).str();
+        Buf = Buf.drop_front(Pos + 1);
+      } else {
+        if (NoLbr.value_or(false))
+          report_error(
+              Filename,
+              "cannot mix profile collected on LBR and non-LBR architectures");
+        NoLbr = false;
+      }
+
       Profile = &Profiles[tid];
     }
 
@@ -336,6 +345,8 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
 
   if (BoltedCollection.value_or(false))
     output() << "boltedcollection\n";
+  if (NoLbr.value_or(false))
+    output() << NoLbrLabel << "\n";
   for (const auto &[Key, Value] : MergedProfile)
     output() << Key << " " << Value << "\n";
 

>From 32ffb290c120ac5a8440f429f50d9c54e64f5c23 Mon Sep 17 00:00:00 2001
From: ranxinzhong <ranxinzhong at tencent.com>
Date: Tue, 15 Oct 2024 15:52:17 +0800
Subject: [PATCH 2/2] sry for forgot the test

---
 .../merge-fdata-between-no-lbr-and-lbr.test     | 14 ++++++++++++++
 bolt/test/merge-fdata-between-two-no-lbr.test   | 17 +++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 bolt/test/merge-fdata-between-no-lbr-and-lbr.test
 create mode 100644 bolt/test/merge-fdata-between-two-no-lbr.test

diff --git a/bolt/test/merge-fdata-between-no-lbr-and-lbr.test b/bolt/test/merge-fdata-between-no-lbr-and-lbr.test
new file mode 100644
index 00000000000000..1a7efccd45d817
--- /dev/null
+++ b/bolt/test/merge-fdata-between-no-lbr-and-lbr.test
@@ -0,0 +1,14 @@
+## Test that merge-fdata fails to mix fdata files with and without the no_lbr marker.
+
+# REQUIRES: system-linux
+
+# RUN: split-file %s %t
+# RUN: not merge-fdata %t/a.fdata %t/b.fdata 2>&1 | FileCheck %s
+
+# CHECK: cannot mix profile collected on LBR and non-LBR architectures
+
+#--- a.fdata
+no_lbr
+main 1
+#--- b.fdata
+main 1
diff --git a/bolt/test/merge-fdata-between-two-no-lbr.test b/bolt/test/merge-fdata-between-two-no-lbr.test
new file mode 100644
index 00000000000000..e2a3df7716afd5
--- /dev/null
+++ b/bolt/test/merge-fdata-between-two-no-lbr.test
@@ -0,0 +1,17 @@
+## Test that merge-fdata correctly handles merging two fdata files with the no_lbr marker.
+
+# REQUIRES: system-linux
+
+# RUN: split-file %s %t
+# RUN: merge-fdata %t/a.fdata %t/b.fdata -o %t/merged.fdata
+# RUN: FileCheck %s --input-file %t/merged.fdata
+
+# CHECK: no_lbr
+# CHECK: main 2
+
+#--- a.fdata
+no_lbr
+main 1
+#--- b.fdata
+no_lbr
+main 1



More information about the llvm-commits mailing list