Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BOLT] Fix merge-fdata for memory events #128108

Open
wants to merge 1 commit into
base: users/aaupov/spr/main.bolt-fix-merge-fdata-for-memory-events
Choose a base branch
from

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented Feb 21, 2025

Don't attempt to parse mispredictions for memory entries in LBR profile.

Test Plan: added merge-fdata-mem-prof.test

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2025

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

Don't attempt to parse mispredictions for memory entries in LBR profile.

Test Plan: added merge-fdata-mem-prof.test


Full diff: https://github.com/llvm/llvm-project/pull/128108.diff

2 Files Affected:

  • (added) bolt/test/merge-fdata-mem-prof.test (+13)
  • (modified) bolt/tools/merge-fdata/merge-fdata.cpp (+10-3)
diff --git a/bolt/test/merge-fdata-mem-prof.test b/bolt/test/merge-fdata-mem-prof.test
new file mode 100644
index 0000000000000..166d6028f7737
--- /dev/null
+++ b/bolt/test/merge-fdata-mem-prof.test
@@ -0,0 +1,13 @@
+## Check that merge-fdata tool correctly handles memory profile
+
+# REQUIRES: system-linux
+
+# RUN: split-file %s %t
+# RUN: merge-fdata %t/a.fdata -o %t/merged.fdata
+# RUN: FileCheck %s --input-file %t/merged.fdata
+
+# CHECK: 4 Curl_cf_def_query c 4 Curl_cft_h1_proxy 68 3
+
+#--- a.fdata
+4 Curl_cf_def_query c 4 Curl_cft_h1_proxy 68 1
+4 Curl_cf_def_query c 4 Curl_cft_h1_proxy 68 2
diff --git a/bolt/tools/merge-fdata/merge-fdata.cpp b/bolt/tools/merge-fdata/merge-fdata.cpp
index 74a5f8ca2d477..0cf5a03501728 100644
--- a/bolt/tools/merge-fdata/merge-fdata.cpp
+++ b/bolt/tools/merge-fdata/merge-fdata.cpp
@@ -316,11 +316,15 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
     do {
       StringRef Line(FdataLine);
       CounterTy Count;
+      unsigned Type = 0;
+      if (Line.split(' ').first.getAsInteger(10, Type))
+        report_error(Filename, "Malformed / corrupted entry type");
+      bool IsBranchEntry = Type < 3;
       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)) {
+      // Only LBR profile has misprediction field, branch entries
+      if (!NoLBRCollection.value_or(false) && IsBranchEntry) {
         auto [SignatureLBR, MispredCount] = Signature.rsplit(' ');
         Signature = SignatureLBR;
         if (MispredCount.getAsInteger(10, Count.Mispred))
@@ -356,7 +360,10 @@ void mergeLegacyProfiles(const SmallVectorImpl<std::string> &Filenames) {
     output() << "no_lbr\n";
   for (const auto &[Key, Value] : MergedProfile) {
     output() << Key << " ";
-    if (!NoLBRCollection.value_or(false))
+    unsigned Type = 0;
+    Key.split(' ').first.getAsInteger(10, Type);
+    bool IsBranchEntry = Type < 3;
+    if (!NoLBRCollection.value_or(false) && IsBranchEntry)
       output() << Value.Mispred << " ";
     output() << Value.Exec << "\n";
   }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants