[llvm] aab1810 - [llvm-profgen] Fix bug of setting function entry
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 12 12:19:16 PST 2021
Author: wlei
Date: 2021-11-12T12:18:43-08:00
New Revision: aab1810006a6788e32ee04e7d40d0b2474754aa2
URL: https://github.com/llvm/llvm-project/commit/aab1810006a6788e32ee04e7d40d0b2474754aa2
DIFF: https://github.com/llvm/llvm-project/commit/aab1810006a6788e32ee04e7d40d0b2474754aa2.diff
LOG: [llvm-profgen] Fix bug of setting function entry
Previously we set `isFuncEntry` flag to true when the funcName from DWARF is equal to the name in symbol table and we use this flag to ignore reporting callsite sample that's from an intra func branch. However, in HHVM, it appears that the symbol table name is inconsistent with the dwarf info func name, it's likely due to `OptimizeGlobalAliases`.
This change is a workaround in llvm-profgen side to mark the only one range as the function entry and add warnings for the remaining inconsistence.
This also fixed a missing `getCanonicalFnName` for symbol name which caused the mismatching as well.
Reviewed By: hoy, wenlei
Differential Revision: https://reviews.llvm.org/D113492
Added:
Modified:
llvm/tools/llvm-profgen/ErrorHandling.h
llvm/tools/llvm-profgen/PerfReader.cpp
llvm/tools/llvm-profgen/PerfReader.h
llvm/tools/llvm-profgen/ProfiledBinary.cpp
llvm/tools/llvm-profgen/ProfiledBinary.h
Removed:
################################################################################
diff --git a/llvm/tools/llvm-profgen/ErrorHandling.h b/llvm/tools/llvm-profgen/ErrorHandling.h
index 5d08c01ab4ceb..b797add8a892f 100644
--- a/llvm/tools/llvm-profgen/ErrorHandling.h
+++ b/llvm/tools/llvm-profgen/ErrorHandling.h
@@ -45,4 +45,12 @@ T unwrapOrError(Expected<T> EO, Ts &&... Args) {
return std::move(*EO);
exitWithError(EO.takeError(), std::forward<Ts>(Args)...);
}
+
+inline void emitWarningSummary(uint64_t Num, uint64_t Total, StringRef Msg) {
+ if (!Total || !Num)
+ return;
+ WithColor::warning() << format("%.2f", static_cast<double>(Num) * 100 / Total)
+ << "%(" << Num << "/" << Total << ") " << Msg << "\n";
+}
+
#endif
diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index 5798783d4b7ac..deafa4dd48102 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -31,10 +31,9 @@ static cl::opt<bool>
IgnoreStackSamples("ignore-stack-samples", cl::init(false), cl::ZeroOrMore,
cl::desc("Ignore call stack samples for hybrid samples "
"and produce context-insensitive profile."));
-static cl::opt<bool>
- ShowDetailedWarning("show-detailed-warning", cl::init(false),
- cl::ZeroOrMore,
- cl::desc("Show detailed warning message."));
+cl::opt<bool> ShowDetailedWarning("show-detailed-warning", cl::init(false),
+ cl::ZeroOrMore,
+ cl::desc("Show detailed warning message."));
extern cl::opt<std::string> PerfTraceFilename;
extern cl::opt<bool> ShowDisassemblyOnly;
@@ -1027,14 +1026,6 @@ void PerfScriptReader::warnTruncatedStack() {
"likely caused by frame pointer omission.");
}
-void PerfScriptReader::emitWarningSummary(uint64_t Num, uint64_t Total,
- StringRef Msg) {
- if (!Total || !Num)
- return;
- WithColor::warning() << format("%.2f", static_cast<double>(Num) * 100 / Total)
- << "%(" << Num << "/" << Total << ") " << Msg << "\n";
-}
-
void PerfScriptReader::warnInvalidRange() {
std::unordered_map<std::pair<uint64_t, uint64_t>, uint64_t,
pair_hash<uint64_t, uint64_t>>
diff --git a/llvm/tools/llvm-profgen/PerfReader.h b/llvm/tools/llvm-profgen/PerfReader.h
index a4e2fccb43958..b5ebfbc3370fd 100644
--- a/llvm/tools/llvm-profgen/PerfReader.h
+++ b/llvm/tools/llvm-profgen/PerfReader.h
@@ -588,7 +588,6 @@ class PerfScriptReader : public PerfReaderBase {
void parseAndAggregateTrace();
// Parse either an MMAP event or a perf sample
void parseEventOrSample(TraceStream &TraceIt);
- void emitWarningSummary(uint64_t Num, uint64_t Total, StringRef Msg);
// Warn if the relevant mmap event is missing.
void warnIfMissingMMap();
// Emit accumulate warnings.
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
index 4fe737eb96c68..1cf1d7b8b612c 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
@@ -48,6 +48,8 @@ static cl::list<std::string> DisassembleFunctions(
cl::desc("List of functions to print disassembly for. Accept demangled "
"names only. Only work with show-disassembly-only"));
+extern cl::opt<bool> ShowDetailedWarning;
+
namespace llvm {
namespace sampleprof {
@@ -154,6 +156,34 @@ void BinarySizeContextTracker::trackInlineesOptimizedAway(
ProbeContext.pop_back();
}
+void ProfiledBinary::warnNoFuncEntry() {
+ uint64_t NoFuncEntryNum = 0;
+ for (auto &F : BinaryFunctions) {
+ if (F.second.Ranges.empty())
+ continue;
+ bool hasFuncEntry = false;
+ for (auto &R : F.second.Ranges) {
+ if (FuncRange *FR = findFuncRangeForStartOffset(R.first)) {
+ if (FR->IsFuncEntry) {
+ hasFuncEntry = true;
+ break;
+ }
+ }
+ }
+
+ if (!hasFuncEntry) {
+ NoFuncEntryNum++;
+ if (ShowDetailedWarning)
+ WithColor::warning()
+ << "Failed to determine function entry for " << F.first
+ << " due to inconsistent name from symbol table and dwarf info.\n";
+ }
+ }
+ emitWarningSummary(NoFuncEntryNum, BinaryFunctions.size(),
+ "of functions failed to determine function entry due to "
+ "inconsistent name from symbol table and dwarf info.");
+}
+
void ProfiledBinary::load() {
// Attempt to open the binary.
OwningBinary<Binary> OBinary = unwrapOrError(createBinary(Path), Path);
@@ -189,6 +219,8 @@ void ProfiledBinary::load() {
ProEpilogTracker.inferPrologOffsets(StartOffset2FuncRangeMap);
ProEpilogTracker.inferEpilogOffsets(RetOffsets);
+ warnNoFuncEntry();
+
// TODO: decode other sections.
}
@@ -317,9 +349,10 @@ void ProfiledBinary::setIsFuncEntry(uint64_t Offset, StringRef RangeSymName) {
if (!FuncRange)
return;
- // Set IsFuncEntry to ture if the RangeSymName from ELF is equal to its
- // DWARF-based function name.
- if (!FuncRange->IsFuncEntry && FuncRange->getFuncName() == RangeSymName)
+ // Set IsFuncEntry to ture if there is only one range in the function or the
+ // RangeSymName from ELF is equal to its DWARF-based function name.
+ if (FuncRange->Func->Ranges.size() == 1 ||
+ (!FuncRange->IsFuncEntry && FuncRange->getFuncName() == RangeSymName))
FuncRange->IsFuncEntry = true;
}
@@ -333,8 +366,8 @@ bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes,
uint64_t NextStartOffset =
(SI + 1 < SE) ? Symbols[SI + 1].Addr - getPreferredBaseAddress()
: SectionOffset + SectSize;
- if (StartOffset > NextStartOffset)
- return true;
+ setIsFuncEntry(StartOffset,
+ FunctionSamples::getCanonicalFnName(Symbols[SI].Name));
StringRef SymbolName =
ShowCanonicalFnName
@@ -420,8 +453,6 @@ bool ProfiledBinary::dissassembleSymbol(std::size_t SI, ArrayRef<uint8_t> Bytes,
if (ShowDisassembly)
outs() << "\n";
- setIsFuncEntry(StartOffset, Symbols[SI].Name);
-
return true;
}
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.h b/llvm/tools/llvm-profgen/ProfiledBinary.h
index 11a8f9b343cc4..3c4ed7b99747a 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.h
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.h
@@ -10,6 +10,7 @@
#define LLVM_TOOLS_LLVM_PROFGEN_PROFILEDBINARY_H
#include "CallContext.h"
+#include "ErrorHandling.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
@@ -261,6 +262,9 @@ class ProfiledBinary {
// function and also set false to the non-function label.
void setIsFuncEntry(uint64_t Offset, StringRef RangeSymName);
+ // Warn if no entry range exists in the function.
+ void warnNoFuncEntry();
+
/// Dissassemble the text section and build various address maps.
void disassemble(const ELFObjectFileBase *O);
More information about the llvm-commits
mailing list