Skip to content

Commit 9188eff

Browse files
committed
Fix suffix strip && refactor the function checks
1 parent 644fce9 commit 9188eff

File tree

3 files changed

+29
-30
lines changed

3 files changed

+29
-30
lines changed

llvm/include/llvm/ProfileData/SampleProf.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1235,7 +1235,7 @@ class FunctionSamples {
12351235
if (It == StringRef::npos)
12361236
continue;
12371237
auto Dit = Cand.rfind('.');
1238-
if (Dit == It + Suffix.size() - 1)
1238+
if (Dit == It || Dit == It + Suffix.size() - 1)
12391239
Cand = Cand.substr(0, It);
12401240
}
12411241
return Cand;

llvm/tools/llvm-profgen/ProfiledBinary.cpp

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ void ProfiledBinary::load() {
265265
decodePseudoProbe(Obj);
266266

267267
if (LoadFunctionFromSymbol && UsePseudoProbes)
268-
populateSymbolsFromBinary(Obj);
268+
loadSymbolsFromSymtab(Obj);
269269

270270
// Disassemble the text sections.
271271
disassemble(Obj);
@@ -830,16 +830,16 @@ void ProfiledBinary::populateSymbolAddressList(const ObjectFile *Obj) {
830830
}
831831
}
832832

833-
void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {
833+
void ProfiledBinary::loadSymbolsFromSymtab(const ObjectFile *Obj) {
834834
// Load binary functions from symbol table when Debug info is incomplete.
835835
// Strip the internal suffixes which are not reflected in the DWARF info.
836-
const SmallVector<StringRef, 6> Suffixes(
836+
const SmallVector<StringRef, 10> Suffixes(
837837
{// Internal suffixes from CoroSplit pass
838838
".cleanup", ".destroy", ".resume",
839839
// Internal suffixes from Bolt
840840
".cold", ".warm",
841-
// Compiler internal
842-
".llvm."});
841+
// Compiler/LTO internal
842+
".llvm.", ".part.", ".isra.", ".constprop.", ".lto_priv."});
843843
StringRef FileName = Obj->getFileName();
844844
for (const SymbolRef &Symbol : Obj->symbols()) {
845845
const SymbolRef::Type Type = unwrapOrError(Symbol.getType(), FileName);
@@ -857,33 +857,34 @@ void ProfiledBinary::populateSymbolsFromBinary(const ObjectFile *Obj) {
857857
const StringRef SymName =
858858
FunctionSamples::getCanonicalFnName(Name, Suffixes);
859859

860-
auto Ret = BinaryFunctions.emplace(SymName, BinaryFunction());
861-
auto &Func = Ret.first->second;
862-
if (Ret.second) {
863-
Func.FuncName = Ret.first->first;
864-
Func.FromSymtab = true;
865-
HashBinaryFunctions[MD5Hash(StringRef(SymName))] = &Func;
866-
}
860+
auto Range = findFuncRange(StartAddr);
861+
if (!Range || Range->StartAddress != StartAddr) {
862+
// Function from symbol table not found previously in DWARF, store ranges.
863+
auto Ret = BinaryFunctions.emplace(SymName, BinaryFunction());
864+
auto &Func = Ret.first->second;
865+
if (Ret.second) {
866+
Func.FuncName = Ret.first->first;
867+
HashBinaryFunctions[MD5Hash(StringRef(SymName))] = &Func;
868+
}
867869

868-
if (auto Range = findFuncRange(StartAddr)) {
869-
if (Ret.second && Range->getFuncName() != SymName && ShowDetailedWarning)
870-
WithColor::warning()
871-
<< "Conflicting symbol " << Name << " already exists in DWARF as "
872-
<< Range->getFuncName() << " at address "
873-
<< format("%8" PRIx64, StartAddr)
874-
<< ". The DWARF indicates a range from "
875-
<< format("%8" PRIx64, Range->StartAddress) << " to "
876-
<< format("%8" PRIx64, Range->EndAddress) << "\n";
877-
} else {
878-
// Store/Update Function Range from SymTab
879-
Func.Ranges.emplace_back(StartAddr, StartAddr + Size);
880870
Func.FromSymtab = true;
871+
Func.Ranges.emplace_back(StartAddr, StartAddr + Size);
881872

882873
auto R = StartAddrToFuncRangeMap.emplace(StartAddr, FuncRange());
883874
FuncRange &FRange = R.first->second;
875+
884876
FRange.Func = &Func;
885877
FRange.StartAddress = StartAddr;
886878
FRange.EndAddress = StartAddr + Size;
879+
880+
} else if (SymName != Range->getFuncName() && ShowDetailedWarning) {
881+
// Function already found from DWARF, check consistency between symbol
882+
// table and DWARF.
883+
WithColor::warning() << "Conflicting name for symbol" << Name
884+
<< " at address " << format("%8" PRIx64, StartAddr)
885+
<< ", but the DWARF symbol " << Range->getFuncName()
886+
<< " indicates a starting address at "
887+
<< format("%8" PRIx64, Range->StartAddress) << "\n";
887888
}
888889
}
889890
}
@@ -912,10 +913,8 @@ void ProfiledBinary::loadSymbolsFromDWARFUnit(DWARFUnit &CompilationUnit) {
912913
// BinaryFunction indexed by the name.
913914
auto Ret = BinaryFunctions.emplace(Name, BinaryFunction());
914915
auto &Func = Ret.first->second;
915-
if (Ret.second) {
916+
if (Ret.second)
916917
Func.FuncName = Ret.first->first;
917-
Func.FromSymtab = false;
918-
}
919918

920919
for (const auto &Range : Ranges) {
921920
uint64_t StartAddress = Range.LowPC;

llvm/tools/llvm-profgen/ProfiledBinary.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ struct BinaryFunction {
7676
StringRef FuncName;
7777
// End of range is an exclusive bound.
7878
RangesTy Ranges;
79-
bool FromSymtab;
79+
bool FromSymtab = false;
8080

8181
uint64_t getFuncSize() {
8282
uint64_t Sum = 0;
@@ -358,7 +358,7 @@ class ProfiledBinary {
358358
void populateSymbolAddressList(const object::ObjectFile *O);
359359

360360
// Load functions from its symbol table (when DWARF info is missing).
361-
void populateSymbolsFromBinary(const object::ObjectFile *O);
361+
void loadSymbolsFromSymtab(const object::ObjectFile *O);
362362

363363
// A function may be spilt into multiple non-continuous address ranges. We use
364364
// this to set whether start a function range is the real entry of the

0 commit comments

Comments
 (0)