[llvm] [BOLT] Fix perf return identification (PR #203609)

[Amir Ayupov via llvm-commits]

https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/203609

If perf data doesn't have branch type recorded, missing value would
incorrectly be interpreted as "not-a-return".
Only populate Returns map if the branch type is available.
Fixes bug introduced in #202813.

Introduce `--parse-branch-type` (default on) currently used for testing.

Test Plan: updated perf_brstack.test


>From 19c1150d3c781be35cd8d65d7452026f04d001b9 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Fri, 12 Jun 2026 11:55:13 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
 =?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created using spr 1.3.8-beta.1
---
 bolt/include/bolt/Profile/DataAggregator.h |  8 ++---
 bolt/lib/Profile/DataAggregator.cpp        | 37 +++++++++++-----------
 bolt/test/perf2bolt/perf_brstack.test      |  9 ++++++
 3 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/bolt/include/bolt/Profile/DataAggregator.h b/bolt/include/bolt/Profile/DataAggregator.h
index fde7187973edd..31ae8cfa53ee9 100644
--- a/bolt/include/bolt/Profile/DataAggregator.h
+++ b/bolt/include/bolt/Profile/DataAggregator.h
@@ -90,10 +90,10 @@ class DataAggregator : public DataReader {
 
 private:
   struct LBREntry {
-    uint64_t From;
-    uint64_t To;
-    bool Mispred;
-    bool IsReturn;
+    uint64_t From{0};
+    uint64_t To{0};
+    bool Mispred{false};
+    bool IsReturn{false};
   };
   friend raw_ostream &operator<<(raw_ostream &OS, const LBREntry &);
 
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 19bb63e88bdae..71f3cb66c3818 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -84,6 +84,11 @@ static cl::opt<bool> ParseMemProfile(
              "on by default unless `--itrace` is set."),
     cl::init(true), cl::cat(AggregatorCategory));
 
+static cl::opt<bool>
+    ParseBranchType("parse-branch-type",
+                    cl::desc("enable perf branch type parsing"), cl::init(true),
+                    cl::cat(AggregatorCategory));
+
 static cl::opt<unsigned long long>
 FilterPID("pid",
   cl::desc("only use samples from process with specified PID"),
@@ -1371,24 +1376,20 @@ ErrorOr<DataAggregator::LBREntry> DataAggregator::parseLBREntry() {
     MispredWarning = false;
   }
 
-  // Transaction, abort, cycles
-  for (unsigned I = 0; I < 3; ++I) {
-    ErrorOr<StringRef> IgnoredStr = parseString('/');
-    if (std::error_code EC = IgnoredStr.getError())
-      return EC;
-  }
-
-  // Type: COND/UNCOND/IND/CALL/IND_CALL/RET
-  ErrorOr<StringRef> TypeStr = parseString('/');
-  if (std::error_code EC = TypeStr.getError())
-    return EC;
-  Res.IsReturn = TypeStr.get() == "RET";
-
-  // Branch speculation
+  // Rest of the entry:
+  // INTX/ABORT/CYCLES/TYPE/SPEC
   ErrorOr<StringRef> Rest = parseString(FieldSeparator, true);
   if (std::error_code EC = Rest.getError())
     return EC;
-  if (Rest.get().size() < 1) {
+
+  bool ValidRest = Rest.get().size() >= 5;
+  if (opts::ParseBranchType) {
+    SmallVector<StringRef, 5> RestFields;
+    Rest.get().split(RestFields, '/');
+    ValidRest = RestFields.size() >= 5 && !RestFields[4].empty();
+    Res.IsReturn = ValidRest && RestFields[3] == "RET";
+  }
+  if (!ValidRest) {
     reportError("expected rest of brstack entry");
     Diag << "Found: " << Rest.get() << "\n";
     return make_error_code(llvm::errc::io_error);
@@ -1832,7 +1833,8 @@ void DataAggregator::parseLBRSample(const PerfBranchSample &Sample,
     TakenBranchInfo &Info = TraceMap[Trace{LBR.From, LBR.To, TraceTo}];
     ++Info.TakenCount;
     Info.MispredCount += LBR.Mispred;
-    Returns.emplace(LBR.From, LBR.IsReturn);
+    if (LBR.IsReturn)
+      Returns.emplace(LBR.From, true);
   }
   // Record LBR addresses not covered by fallthroughs (bottom-of-stack source
   // and top-of-stack target) as basic samples for heatmap.
@@ -2517,8 +2519,7 @@ DataAggregator::writePreAggregatedFile(StringRef OutputFilename) const {
     return EC;
 
   for (const auto &[Trace, Info] : Traces) {
-    auto ReturnIt = Returns.find(Trace.Branch);
-    const bool IsRet = ReturnIt != Returns.end() && ReturnIt->second;
+    const bool IsRet = Returns.count(Trace.Branch);
     OS << (IsRet ? 'R' : 'T') << " " << Trace << " " << Info << '\n';
   }
   if (!EventNames.empty())
diff --git a/bolt/test/perf2bolt/perf_brstack.test b/bolt/test/perf2bolt/perf_brstack.test
index d7fe6abebebc6..af19df0989adf 100644
--- a/bolt/test/perf2bolt/perf_brstack.test
+++ b/bolt/test/perf2bolt/perf_brstack.test
@@ -10,3 +10,12 @@ RUN: perf2bolt %t -pa -p=%t.preagg -o %t.roundtrip.fdata -ignore-build-id
 RUN: sort %t.fdata > %t.fdata.sorted
 RUN: sort %t.roundtrip.fdata > %t.roundtrip.fdata.sorted
 RUN: diff %t.fdata.sorted %t.roundtrip.fdata.sorted
+## Check branch type (return) parsing: pre-aggregated profile is dumped before
+## Returns map is populated from disassembly, so we can use it to check if there
+## are/aren't any pre-aggregated returns.
+RUN: perf record -Fmax -j any,u -e cycles:u -o %t.perf.data.notype -- %t
+RUN: perf2bolt %t -p=%t.perf.data.notype -o %t.preagg.notype -ignore-build-id \
+RUN:   --profile-format=preagg --parse-branch-type=false
+## Expect no returns without branch type
+RUN: FileCheck %s --input-file=%t.preagg.notype --check-prefix=CHECK-NOTYPE
+CHECK-NOTYPE-NOT: R