[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:55:04 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-bolt
Author: zrx_p4ul (zrxxzz)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/112328.diff
3 Files Affected:
- (added) bolt/test/merge-fdata-between-no-lbr-and-lbr.test (+14)
- (added) bolt/test/merge-fdata-between-two-no-lbr.test (+17)
- (modified) bolt/tools/merge-fdata/merge-fdata.cpp (+47-36)
``````````diff
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
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";
``````````
</details>
https://github.com/llvm/llvm-project/pull/112328
More information about the llvm-commits
mailing list