[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