[llvm] [BOLT][NFC] Move CFG profile attachment to a late pass via FBD (PR #195986)
Amir Ayupov via llvm-commits
llvm-commits at lists.llvm.org
Tue May 5 19:57:43 PDT 2026
https://github.com/aaupov created https://github.com/llvm/llvm-project/pull/195986
Consolidate CFG profile attachment between DataAggregator and DataReader
so both follow the same pipeline:
parser -> FuncBranchData -> attachProfileToCFG -> CFG
DataAggregator's parser becomes a pure `FuncBranchData` producer with
no inline CFG side effects; CFG attachment runs once per function in a
late pass, mirroring DataReader's existing flow.
Changes:
* Drop inline CFG bumps in `getFallthroughsInTrace`: the function is
now decode-only (returns the fall-through list; `Count` parameter
removed). CFG edge attachment for fall-throughs goes through the
late `recordBranch` loop instead.
* Drop inline `recordBranch` in `doBranch`'s recursive
intra-branch case. The branch ends up in `FuncBranchData` only;
the late pass attaches it to the CFG.
* Delete `DataAggregator::recordEntry` / `recordExit`.
`doInterBranch` now records function-entry counts on the FBD
instead: when the inter-function branch lands at offset 0, bump
`FBD->ExecutionCount` (and `FBD->ExternEntryCount` when the
source is outside any known function), mirroring
`DataReader::parse`. `processProfile` transfers
`FBD->{ExecutionCount,ExternEntryCount}` into the
`BinaryFunction`, mirroring `DataReader::preprocessProfile`.
* Add `DataReader::attachProfileToCFG(BF)` and call it from both
`DataReader::readProfile(BF)` and `DataAggregator::readProfile`.
The shared method does:
EntryData -> entry/landing-pad BB::ExecutionCount
Data -> recordBranch (intra-function CFG edges)
-> convertBranchData (MCInst call annotations)
The `EntryData -> BB exec count` loop moves out of
`DataReader::readProfile(BF)` into the shared helper, so the
DataAggregator path picks it up too — gaining landing-pad
attribution that the deleted `recordEntry` did not handle.
Test Plan: NFC
>From 6201b4ed2fe3ee076705980c4089da6a78c4b9a3 Mon Sep 17 00:00:00 2001
From: Amir Ayupov <aaupov at fb.com>
Date: Tue, 5 May 2026 19:57:22 -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.4
---
bolt/include/bolt/Profile/DataAggregator.h | 14 +----
bolt/include/bolt/Profile/DataReader.h | 9 +++
bolt/lib/Profile/DataAggregator.cpp | 73 ++++++----------------
bolt/lib/Profile/DataReader.cpp | 4 ++
4 files changed, 32 insertions(+), 68 deletions(-)
diff --git a/bolt/include/bolt/Profile/DataAggregator.h b/bolt/include/bolt/Profile/DataAggregator.h
index f7c9e31915d74..d3ff37aa1a801 100644
--- a/bolt/include/bolt/Profile/DataAggregator.h
+++ b/bolt/include/bolt/Profile/DataAggregator.h
@@ -234,21 +234,9 @@ class DataAggregator : public DataReader {
/// Return a vector of offsets corresponding to a trace in a function
/// if the trace is valid, std::nullopt otherwise.
std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
- getFallthroughsInTrace(BinaryFunction &BF, const Trace &Trace, uint64_t Count,
+ getFallthroughsInTrace(BinaryFunction &BF, const Trace &Trace,
bool IsReturn) const;
- /// Record external entry into the function \p BF.
- ///
- /// Return true if the entry is valid, false otherwise.
- bool recordEntry(BinaryFunction &BF, uint64_t To, bool Mispred,
- uint64_t Count = 1) const;
-
- /// Record exit from the function \p BF via a call or return.
- ///
- /// Return true if the exit point is valid, false otherwise.
- bool recordExit(BinaryFunction &BF, uint64_t From, bool Mispred,
- uint64_t Count = 1) const;
-
/// Branch stacks aggregation statistics
uint64_t NumTraces{0};
uint64_t NumInvalidTraces{0};
diff --git a/bolt/include/bolt/Profile/DataReader.h b/bolt/include/bolt/Profile/DataReader.h
index 31b23ff4cdd8e..d293aee19d052 100644
--- a/bolt/include/bolt/Profile/DataReader.h
+++ b/bolt/include/bolt/Profile/DataReader.h
@@ -293,6 +293,15 @@ class DataReader : public ProfileReaderBase {
/// Convert function-level branch data into instruction annotations.
void convertBranchData(BinaryFunction &BF) const;
+ /// Attach the function's \c FuncBranchData to its CFG: bump entry /
+ /// landing-pad BB execution counts from \c FBD->EntryData; attach
+ /// intra-function edges via \c recordBranch from \c FBD->Data; and
+ /// convert call-site branch data into instruction annotations via
+ /// \c convertBranchData. Shared between \c DataReader::readProfile (after
+ /// \c matchProfileData) and \c DataAggregator::readProfile (after the
+ /// parser populates \c FBD).
+ void attachProfileToCFG(BinaryFunction &BF) const;
+
/// Update function \p BF profile with a taken branch.
/// \p Count could be 0 if verification of the branch is required.
///
diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp
index 344682f9ae2f4..bb4de5cf235a4 100644
--- a/bolt/lib/Profile/DataAggregator.cpp
+++ b/bolt/lib/Profile/DataAggregator.cpp
@@ -717,10 +717,8 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) {
Error DataAggregator::readProfile(BinaryContext &BC) {
processProfile(BC);
- for (auto &BFI : BC.getBinaryFunctions()) {
- BinaryFunction &Function = BFI.second;
- convertBranchData(Function);
- }
+ for (auto &BFI : BC.getBinaryFunctions())
+ attachProfileToCFG(BFI.second);
if (opts::AggregateOnly) {
if (opts::ProfileFormat == opts::ProfileFormatKind::PF_Fdata)
@@ -760,6 +758,11 @@ void DataAggregator::processProfile(BinaryContext &BC) {
if (FuncBranchData *FBD = getBranchData(BF)) {
BF.markProfiled(BinaryFunction::PF_BRANCH);
BF.RawSampleCount = FBD->getNumExecutedBranches();
+ // Transfer entry counts from FBD to BF (mirrors
+ // DataReader::preprocessProfile). BB-level entry/landing-pad counts
+ // are bumped later in attachProfileToCFG.
+ BF.ExecutionCount = FBD->ExecutionCount;
+ BF.ExternEntryCount = FBD->ExternEntryCount;
} else if (FuncBasicSampleData *FSD =
getFuncBasicSampleData(BF.getNames())) {
BF.markProfiled(BinaryFunction::PF_BASIC);
@@ -880,8 +883,6 @@ bool DataAggregator::doInterBranch(BinaryFunction *FromFunc,
FromAggrData->Name = SrcFunc;
setBranchData(*FromFunc, FromAggrData);
}
-
- recordExit(*FromFunc, From, Mispreds, Count);
}
if (ToFunc) {
DstFunc = getLocationName(*ToFunc, BAT);
@@ -891,8 +892,15 @@ bool DataAggregator::doInterBranch(BinaryFunction *FromFunc,
ToAggrData->Name = DstFunc;
setBranchData(*ToFunc, ToAggrData);
}
-
- recordEntry(*ToFunc, To, Mispreds, Count);
+ // Mirror DataReader::parse: branches landing at the function entry
+ // (offset 0) bump FBD ExecutionCount; if originating from outside any
+ // known function, also bump ExternEntryCount. CFG entry/landing-pad
+ // BB exec counts are bumped later in attachProfileToCFG via EntryData.
+ if (To == 0) {
+ ToAggrData->ExecutionCount += Count;
+ if (!FromFunc)
+ ToAggrData->ExternEntryCount += Count;
+ }
}
if (FromAggrData)
@@ -942,7 +950,6 @@ bool DataAggregator::doBranch(uint64_t From, uint64_t To, uint64_t Count,
// Treat recursive control transfers as inter-branches.
if (FromFunc == ToFunc && To != 0) {
- recordBranch(*FromFunc, From, To, Count, Mispreds);
return doIntraBranch(*FromFunc, From, To, Count, Mispreds);
}
@@ -976,7 +983,7 @@ bool DataAggregator::doTrace(const Trace &Trace, uint64_t Count,
std::optional<BoltAddressTranslation::FallthroughListTy> FTs =
BAT && BAT->isBATFunction(FuncAddress)
? BAT->getFallthroughsInTrace(FuncAddress, From - IsReturn, To)
- : getFallthroughsInTrace(*FromFunc, Trace, Count, IsReturn);
+ : getFallthroughsInTrace(*FromFunc, Trace, IsReturn);
if (!FTs) {
LLVM_DEBUG(dbgs() << "Invalid trace " << Trace << '\n');
NumInvalidTraces += Count;
@@ -993,7 +1000,7 @@ bool DataAggregator::doTrace(const Trace &Trace, uint64_t Count,
std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, const Trace &Trace,
- uint64_t Count, bool IsReturn) const {
+ bool IsReturn) const {
SmallVector<std::pair<uint64_t, uint64_t>, 16> Branches;
BinaryContext &BC = BF.getBinaryContext();
@@ -1073,53 +1080,9 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, const Trace &Trace,
BB = NextBB;
}
- // Record fall-through jumps
- for (const auto &[FromOffset, ToOffset] : Branches) {
- BinaryBasicBlock *FromBB = BF.getBasicBlockContainingOffset(FromOffset);
- BinaryBasicBlock *ToBB = BF.getBasicBlockAtOffset(ToOffset);
- assert(FromBB && ToBB);
- BinaryBasicBlock::BinaryBranchInfo &BI = FromBB->getBranchInfo(*ToBB);
- BI.Count += Count;
- }
-
return Branches;
}
-bool DataAggregator::recordEntry(BinaryFunction &BF, uint64_t To, bool Mispred,
- uint64_t Count) const {
- if (To > BF.getSize())
- return false;
-
- if (!BF.hasProfile())
- BF.ExecutionCount = 0;
-
- BinaryBasicBlock *EntryBB = nullptr;
- if (To == 0) {
- BF.ExecutionCount += Count;
- if (!BF.empty())
- EntryBB = &BF.front();
- } else if (BinaryBasicBlock *BB = BF.getBasicBlockAtOffset(To)) {
- if (BB->isEntryPoint())
- EntryBB = BB;
- }
-
- if (EntryBB)
- EntryBB->setExecutionCount(EntryBB->getKnownExecutionCount() + Count);
-
- return true;
-}
-
-bool DataAggregator::recordExit(BinaryFunction &BF, uint64_t From, bool Mispred,
- uint64_t Count) const {
- if (!BF.isSimple() || From > BF.getSize())
- return false;
-
- if (!BF.hasProfile())
- BF.ExecutionCount = 0;
-
- return true;
-}
-
ErrorOr<DataAggregator::LBREntry> DataAggregator::parseLBREntry() {
LBREntry Res;
ErrorOr<StringRef> FromStrRes = parseString('/');
diff --git a/bolt/lib/Profile/DataReader.cpp b/bolt/lib/Profile/DataReader.cpp
index 38e32d12028d3..6eec4b38e9e7a 100644
--- a/bolt/lib/Profile/DataReader.cpp
+++ b/bolt/lib/Profile/DataReader.cpp
@@ -347,6 +347,10 @@ void DataReader::readProfile(BinaryFunction &BF) {
// Possibly assign/re-assign branch profile data.
matchProfileData(BF);
+ attachProfileToCFG(BF);
+}
+
+void DataReader::attachProfileToCFG(BinaryFunction &BF) const {
FuncBranchData *FBD = getBranchData(BF);
if (!FBD)
return;
More information about the llvm-commits
mailing list