Skip to content

Commit 9c4effa

Browse files
aaupovaaupov
committed
Drop changes in doTrace/getFallthroughsInTrace
Created using spr 1.3.4
1 parent 9741297 commit 9c4effa

File tree

2 files changed

+77
-40
lines changed

2 files changed

+77
-40
lines changed

‎bolt/include/bolt/Profile/DataAggregator.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,8 @@ class DataAggregator : public DataReader {
202202
/// Return a vector of offsets corresponding to a trace in a function
203203
/// if the trace is valid, std::nullopt otherwise.
204204
std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
205-
getFallthroughsInTrace(BinaryFunction &BF, uint64_t From, uint64_t To,
206-
uint64_t Count = 1) const;
205+
getFallthroughsInTrace(BinaryFunction &BF, const LBREntry &First,
206+
const LBREntry &Second, uint64_t Count = 1) const;
207207

208208
/// Record external entry into the function \p BF.
209209
///
@@ -268,8 +268,9 @@ class DataAggregator : public DataReader {
268268
/// Register a \p Branch.
269269
bool doBranch(uint64_t From, uint64_t To, uint64_t Count, uint64_t Mispreds);
270270

271-
/// Register a trace between two addresses.
272-
bool doTrace(const uint64_t From, const uint64_t To, uint64_t Count = 1);
271+
/// Register a trace between two LBR entries supplied in execution order.
272+
bool doTrace(const LBREntry &First, const LBREntry &Second,
273+
uint64_t Count = 1);
273274

274275
/// Parser helpers
275276
/// Return false if we exhausted our parser buffer and finished parsing

‎bolt/lib/Profile/DataAggregator.cpp‎

Lines changed: 72 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -805,8 +805,11 @@ bool DataAggregator::doBranch(uint64_t From, uint64_t To, uint64_t Count,
805805

806806
BinaryFunction *FromFunc = handleAddress(From, /*IsFrom=*/true);
807807
// Record returns as call->call continuation fall-through.
808-
if (IsReturn)
809-
return doTrace(To - 1, To, Count);
808+
if (IsReturn) {
809+
LBREntry First{To - 1, To - 1, false};
810+
LBREntry Second{To, To, false};
811+
return doTrace(First, Second, Count);
812+
}
810813

811814
BinaryFunction *ToFunc = handleAddress(To, /*IsFrom=*/false);
812815
if (!FromFunc && !ToFunc)
@@ -821,23 +824,24 @@ bool DataAggregator::doBranch(uint64_t From, uint64_t To, uint64_t Count,
821824
return doInterBranch(FromFunc, ToFunc, From, To, Count, Mispreds);
822825
}
823826

824-
bool DataAggregator::doTrace(const uint64_t From, const uint64_t To,
827+
bool DataAggregator::doTrace(const LBREntry &First, const LBREntry &Second,
825828
uint64_t Count) {
826-
BinaryFunction *FromFunc = getBinaryFunctionContainingAddress(From);
827-
BinaryFunction *ToFunc = getBinaryFunctionContainingAddress(To);
829+
BinaryFunction *FromFunc = getBinaryFunctionContainingAddress(First.To);
830+
BinaryFunction *ToFunc = getBinaryFunctionContainingAddress(Second.From);
828831
if (!FromFunc || !ToFunc) {
829832
LLVM_DEBUG({
830833
dbgs() << "Out of range trace starting in ";
831834
if (FromFunc)
832835
dbgs() << formatv("{0} @ {1:x}", *FromFunc,
833-
From - FromFunc->getAddress());
836+
First.To - FromFunc->getAddress());
834837
else
835-
dbgs() << Twine::utohexstr(From);
838+
dbgs() << Twine::utohexstr(First.To);
836839
dbgs() << " and ending in ";
837840
if (ToFunc)
838-
dbgs() << formatv("{0} @ {1:x}", *ToFunc, To - ToFunc->getAddress());
841+
dbgs() << formatv("{0} @ {1:x}", *ToFunc,
842+
Second.From - ToFunc->getAddress());
839843
else
840-
dbgs() << Twine::utohexstr(To);
844+
dbgs() << Twine::utohexstr(Second.From);
841845
dbgs() << '\n';
842846
});
843847
NumLongRangeTraces += Count;
@@ -847,30 +851,32 @@ bool DataAggregator::doTrace(const uint64_t From, const uint64_t To,
847851
NumInvalidTraces += Count;
848852
LLVM_DEBUG({
849853
dbgs() << "Invalid trace starting in " << FromFunc->getPrintName()
850-
<< formatv(" @ {0:x}", From - FromFunc->getAddress())
854+
<< formatv(" @ {0:x}", First.To - FromFunc->getAddress())
851855
<< " and ending in " << ToFunc->getPrintName()
852-
<< formatv(" @ {0:x}\n", To - ToFunc->getAddress());
856+
<< formatv(" @ {0:x}\n", Second.From - ToFunc->getAddress());
853857
});
854858
return false;
855859
}
856860

857861
std::optional<BoltAddressTranslation::FallthroughListTy> FTs =
858-
BAT ? BAT->getFallthroughsInTrace(FromFunc->getAddress(), From, To)
859-
: getFallthroughsInTrace(*FromFunc, From, To, Count);
862+
BAT ? BAT->getFallthroughsInTrace(FromFunc->getAddress(), First.To,
863+
Second.From)
864+
: getFallthroughsInTrace(*FromFunc, First, Second, Count);
860865
if (!FTs) {
861-
LLVM_DEBUG(dbgs() << "Invalid trace starting in "
862-
<< FromFunc->getPrintName() << " @ "
863-
<< Twine::utohexstr(From - FromFunc->getAddress())
864-
<< " and ending in " << ToFunc->getPrintName() << " @ "
865-
<< ToFunc->getPrintName() << " @ "
866-
<< Twine::utohexstr(To - ToFunc->getAddress()) << '\n');
866+
LLVM_DEBUG(
867+
dbgs() << "Invalid trace starting in " << FromFunc->getPrintName()
868+
<< " @ " << Twine::utohexstr(First.To - FromFunc->getAddress())
869+
<< " and ending in " << ToFunc->getPrintName() << " @ "
870+
<< ToFunc->getPrintName() << " @ "
871+
<< Twine::utohexstr(Second.From - ToFunc->getAddress()) << '\n');
867872
NumInvalidTraces += Count;
868873
return false;
869874
}
870875

871876
LLVM_DEBUG(dbgs() << "Processing " << FTs->size() << " fallthroughs for "
872-
<< FromFunc->getPrintName() << ":" << Twine::utohexstr(From)
873-
<< " to " << Twine::utohexstr(To) << ".\n");
877+
<< FromFunc->getPrintName() << ":"
878+
<< Twine::utohexstr(First.To) << " to "
879+
<< Twine::utohexstr(Second.From) << ".\n");
874880
BinaryFunction *ParentFunc = getBATParentFunction(*FromFunc);
875881
for (auto [From, To] : *FTs) {
876882
if (BAT) {
@@ -884,8 +890,10 @@ bool DataAggregator::doTrace(const uint64_t From, const uint64_t To,
884890
}
885891

886892
std::optional<SmallVector<std::pair<uint64_t, uint64_t>, 16>>
887-
DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, uint64_t From,
888-
uint64_t To, uint64_t Count) const {
893+
DataAggregator::getFallthroughsInTrace(BinaryFunction &BF,
894+
const LBREntry &FirstLBR,
895+
const LBREntry &SecondLBR,
896+
uint64_t Count) const {
889897
SmallVector<std::pair<uint64_t, uint64_t>, 16> Branches;
890898

891899
BinaryContext &BC = BF.getBinaryContext();
@@ -896,8 +904,8 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, uint64_t From,
896904
assert(BF.hasCFG() && "can only record traces in CFG state");
897905

898906
// Offsets of the trace within this function.
899-
From = From - BF.getAddress();
900-
To = To - BF.getAddress();
907+
const uint64_t From = FirstLBR.To - BF.getAddress();
908+
const uint64_t To = SecondLBR.From - BF.getAddress();
901909

902910
if (From > To)
903911
return std::nullopt;
@@ -908,6 +916,24 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, uint64_t From,
908916
if (!FromBB || !ToBB)
909917
return std::nullopt;
910918

919+
// Adjust FromBB if the first LBR is a return from the last instruction in
920+
// the previous block (that instruction should be a call).
921+
if (From == FromBB->getOffset() && !BF.containsAddress(FirstLBR.From) &&
922+
!FromBB->isEntryPoint() && !FromBB->isLandingPad()) {
923+
const BinaryBasicBlock *PrevBB =
924+
BF.getLayout().getBlock(FromBB->getIndex() - 1);
925+
if (PrevBB->getSuccessor(FromBB->getLabel())) {
926+
const MCInst *Instr = PrevBB->getLastNonPseudoInstr();
927+
if (Instr && BC.MIB->isCall(*Instr))
928+
FromBB = PrevBB;
929+
else
930+
LLVM_DEBUG(dbgs() << "invalid incoming LBR (no call): " << FirstLBR
931+
<< '\n');
932+
} else {
933+
LLVM_DEBUG(dbgs() << "invalid incoming LBR: " << FirstLBR << '\n');
934+
}
935+
}
936+
911937
// Fill out information for fall-through edges. The From and To could be
912938
// within the same basic block, e.g. when two call instructions are in the
913939
// same block. In this case we skip the processing.
@@ -924,8 +950,8 @@ DataAggregator::getFallthroughsInTrace(BinaryFunction &BF, uint64_t From,
924950
// Check for bad LBRs.
925951
if (!BB->getSuccessor(NextBB->getLabel())) {
926952
LLVM_DEBUG(dbgs() << "no fall-through for the trace:\n"
927-
<< " " << From << '\n'
928-
<< " " << To << '\n');
953+
<< " " << FirstLBR << '\n'
954+
<< " " << SecondLBR << '\n');
929955
return std::nullopt;
930956
}
931957

@@ -1582,11 +1608,16 @@ void DataAggregator::processBranchEvents() {
15821608
NamedRegionTimer T("processBranch", "Processing branch events",
15831609
TimerGroupName, TimerGroupDesc, opts::TimeAggregator);
15841610

1585-
for (const auto &[Loc, Info] : FallthroughLBRs) {
1611+
for (const auto &AggrLBR : FallthroughLBRs) {
1612+
const Trace &Loc = AggrLBR.first;
1613+
const FTInfo &Info = AggrLBR.second;
1614+
LBREntry First{Loc.From, Loc.From, false};
1615+
LBREntry Second{Loc.To, Loc.To, false};
15861616
if (Info.InternCount)
1587-
doTrace(Loc.From, Loc.To, Info.InternCount);
1617+
doTrace(First, Second, Info.InternCount);
15881618
if (Info.ExternCount) {
1589-
doTrace(0, Loc.To, Info.ExternCount);
1619+
First.From = 0;
1620+
doTrace(First, Second, Info.ExternCount);
15901621
}
15911622
}
15921623

@@ -1750,16 +1781,21 @@ void DataAggregator::processPreAggregated() {
17501781
TimerGroupName, TimerGroupDesc, opts::TimeAggregator);
17511782

17521783
uint64_t NumTraces = 0;
1753-
for (const auto &[From, To, Count, Mispreds, Type] : AggregatedLBRs) {
1754-
bool IsExternalOrigin = Type == AggregatedLBREntry::FT_EXTERNAL_ORIGIN;
1755-
switch (Type) {
1784+
for (const AggregatedLBREntry &AggrEntry : AggregatedLBRs) {
1785+
switch (AggrEntry.EntryType) {
17561786
case AggregatedLBREntry::BRANCH:
1757-
doBranch(From.Offset, To.Offset, Count, Mispreds);
1787+
doBranch(AggrEntry.From.Offset, AggrEntry.To.Offset, AggrEntry.Count,
1788+
AggrEntry.Mispreds);
17581789
break;
17591790
case AggregatedLBREntry::FT:
17601791
case AggregatedLBREntry::FT_EXTERNAL_ORIGIN: {
1761-
doTrace(IsExternalOrigin ? 0 : From.Offset, To.Offset, Count);
1762-
NumTraces += Count;
1792+
LBREntry First{AggrEntry.EntryType == AggregatedLBREntry::FT
1793+
? AggrEntry.From.Offset
1794+
: 0,
1795+
AggrEntry.From.Offset, false};
1796+
LBREntry Second{AggrEntry.To.Offset, AggrEntry.To.Offset, false};
1797+
doTrace(First, Second, AggrEntry.Count);
1798+
NumTraces += AggrEntry.Count;
17631799
break;
17641800
}
17651801
}

0 commit comments

Comments
 (0)