[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

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

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>>

llvm/tools/llvm-profgen/PerfReader.h

@@ -588,7 +588,6 @@ protected:
   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.

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;
 }
 

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);