[llvm] [BOLT][NFCI] Unify DataAggregator/DataReader profile attachment (PR #195986)
via llvm-commits
llvm-commits at lists.llvm.org
Tue May 5 20:57:06 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-bolt
Author: Amir Ayupov (aaupov)
<details>
<summary>Changes</summary>
Unify CFG profile attachment between DataAggregator and DataReader:
parser -> FuncBranchData -> attachProfileToCFG -> CFG
DataAggregator becomes a pure FuncBranchData producer with no inline
CFG side effects; CFG attachment runs after parsing.
Changes:
- Drop inline CFG edge bumps in `getFallthroughsInTrace`.
- Drop inline `recordBranch` in `doBranch`.
- Delete `DataAggregator::recordEntry` / `recordExit`.
- Add `DataReader::attachProfileToCFG` and call it from both
DataReader and DataAggregator, handling FuncBranchData:
EntryData -> entry/landing-pad BB ExecutionCount
Data -> recordBranch (intra-function CFG edges)
-> convertBranchData (call annotations)
Test Plan: NFCI
---
Full diff: https://github.com/llvm/llvm-project/pull/195986.diff
4 Files Affected:
- (modified) bolt/include/bolt/Profile/DataAggregator.h (+1-13)
- (modified) bolt/include/bolt/Profile/DataReader.h (+9)
- (modified) bolt/lib/Profile/DataAggregator.cpp (+18-55)
- (modified) bolt/lib/Profile/DataReader.cpp (+4)
``````````diff
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;
``````````
</details>
https://github.com/llvm/llvm-project/pull/195986
More information about the llvm-commits
mailing list