[llvm] 941191a - [llvm-profgen] Refactor and better diagnostics

Wenlei He via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 29 22:55:59 PDT 2021


Author: Wenlei He
Date: 2021-09-29T22:55:50-07:00
New Revision: 941191aae4abaf88b176bec8cfe1c841a5832642

URL: https://github.com/llvm/llvm-project/commit/941191aae4abaf88b176bec8cfe1c841a5832642
DIFF: https://github.com/llvm/llvm-project/commit/941191aae4abaf88b176bec8cfe1c841a5832642.diff

LOG: [llvm-profgen] Refactor and better diagnostics

This change contains diagnostics improvments, refactoring and preparation for consuming perf data directly.

Diagnostics:
 - We now have more detailed diagnostics when no mmap is found.
 - We also print warning for abnormal transition to external code.

Refactoring:
 - Simplify input perf trace processing to only allow a single input file. This is because 1) using multiple input perf trace (perf script) is error prone because we may miss key mmap events. 2) the functionality is not really being used anyways.
 - Make more functions private for Readers, move non-trivial definitions out of header. Cleanup some inconsistency.
 - Prepare for consuming perf data as input directly.

Differential Revision: https://reviews.llvm.org/D110729

Added: 
    

Modified: 
    llvm/tools/llvm-profgen/PerfReader.cpp
    llvm/tools/llvm-profgen/PerfReader.h
    llvm/tools/llvm-profgen/llvm-profgen.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index 80d70044378e4..c1a6b3bbcaaa2 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -275,15 +275,18 @@ bool VirtualUnwinder::unwind(const PerfSample *Sample, uint64_t Repeat) {
   return true;
 }
 
-std::unique_ptr<PerfReaderBase>
-PerfReaderBase::create(ProfiledBinary *Binary,
-                       cl::list<std::string> &PerfTraceFilenames) {
-  PerfScriptType PerfType = extractPerfType(PerfTraceFilenames);
+std::unique_ptr<PerfReaderBase> PerfReaderBase::create(ProfiledBinary *Binary,
+                                                       StringRef PerfInputFile,
+                                                       bool IsPerfData) {
+  // TODO: for perf data input, we need to convert them into perf script first.
+  if (IsPerfData)
+    exitWithError("Perf data input not supported yet.");
+  PerfScriptType PerfType = checkPerfScriptType(PerfInputFile);
   std::unique_ptr<PerfReaderBase> PerfReader;
   if (PerfType == PERF_LBR_STACK) {
-    PerfReader.reset(new HybridPerfReader(Binary));
+    PerfReader.reset(new HybridPerfReader(Binary, PerfInputFile));
   } else if (PerfType == PERF_LBR) {
-    PerfReader.reset(new LBRPerfReader(Binary));
+    PerfReader.reset(new LBRPerfReader(Binary, PerfInputFile));
   } else {
     exitWithError("Unsupported perfscript!");
   }
@@ -369,7 +372,7 @@ void HybridPerfReader::unwindSamples() {
   for (auto Address : AllUntrackedCallsites)
     WithColor::warning() << "Profile context truncated due to missing probe "
                          << "for call instruction at "
-                         << format("%" PRIx64, Address) << "\n";
+                         << format("0x%" PRIx64, Address) << "\n";
 }
 
 bool PerfReaderBase::extractLBRStack(TraceStream &TraceIt,
@@ -426,9 +429,20 @@ bool PerfReaderBase::extractLBRStack(TraceStream &TraceIt,
     bool IsOutgoing = SrcIsInternal && !DstIsInternal;
     bool IsArtificial = false;
 
-    // Ignore branches outside the current binary.
-    if (IsExternal)
-      continue;
+    // Ignore branches outside the current binary. Ignore all remaining branches
+    // if there's no incoming branch before the external branch in reverse
+    // order.
+    if (IsExternal) {
+      if (PrevTrDst)
+        continue;
+      else if (!LBRStack.empty()) {
+        WithColor::warning()
+            << "Invalid transfer to external code in LBR record at line "
+            << TraceIt.getLineNumber() << ": " << TraceIt.getCurrentLine()
+            << "\n";
+        break;
+      }
+    }
 
     if (IsOutgoing) {
       if (!PrevTrDst) {
@@ -540,8 +554,12 @@ bool PerfReaderBase::extractCallstack(TraceStream &TraceIt,
 
 void PerfReaderBase::warnIfMissingMMap() {
   if (!Binary->getMissingMMapWarned() && !Binary->getIsLoadedByMMap()) {
-    WithColor::warning() << "No relevant mmap event is matched, will use "
-                            "preferred address as the base loading address!\n";
+    WithColor::warning() << "No relevant mmap event is matched for "
+                         << Binary->getName()
+                         << ", will use preferred address ("
+                         << format("0x%" PRIx64,
+                                   Binary->getPreferredBaseAddress())
+                         << ") as the base loading address!\n";
     // Avoid redundant warning, only warn at the first unmatched sample.
     Binary->setMissingMMapWarned(true);
   }
@@ -759,25 +777,66 @@ void PerfReaderBase::parseEventOrSample(TraceStream &TraceIt) {
     parseSample(TraceIt);
 }
 
-void PerfReaderBase::parseAndAggregateTrace(StringRef Filename) {
+void PerfReaderBase::parseAndAggregateTrace() {
   // Trace line iterator
-  TraceStream TraceIt(Filename);
+  TraceStream TraceIt(PerfTraceFile);
   while (!TraceIt.isAtEoF())
     parseEventOrSample(TraceIt);
 }
 
-PerfScriptType
-PerfReaderBase::extractPerfType(cl::list<std::string> &PerfTraceFilenames) {
-  PerfScriptType PerfType = PERF_UNKNOWN;
-  for (auto FileName : PerfTraceFilenames) {
-    PerfScriptType Type = checkPerfScriptType(FileName);
-    if (Type == PERF_INVALID)
-      exitWithError("Invalid perf script input!");
-    if (PerfType != PERF_UNKNOWN && PerfType != Type)
-      exitWithError("Inconsistent sample among 
diff erent perf scripts");
-    PerfType = Type;
+// A LBR sample is like:
+// 40062f 0x5c6313f/0x5c63170/P/-/-/0  0x5c630e7/0x5c63130/P/-/-/0 ...
+// A heuristic for fast detection by checking whether a
+// leading "  0x" and the '/' exist.
+bool PerfReaderBase::isLBRSample(StringRef Line) {
+  // Skip the leading instruction pointer
+  SmallVector<StringRef, 32> Records;
+  Line.trim().split(Records, " ", 2, false);
+  if (Records.size() < 2)
+    return false;
+  if (Records[1].startswith("0x") && Records[1].find('/') != StringRef::npos)
+    return true;
+  return false;
+}
+
+// The raw hybird sample is like
+// e.g.
+// 	          4005dc    # call stack leaf
+//	          400634
+//	          400684    # call stack root
+// 0x4005c8/0x4005dc/P/-/-/0   0x40062f/0x4005b0/P/-/-/0 ...
+//          ... 0x4005c8/0x4005dc/P/-/-/0    # LBR Entries
+// Determine the perfscript contains hybrid samples(call stack + LBRs) by
+// checking whether there is a non-empty call stack immediately followed by
+// a LBR sample
+PerfScriptType PerfReaderBase::checkPerfScriptType(StringRef FileName) {
+  TraceStream TraceIt(FileName);
+  uint64_t FrameAddr = 0;
+  while (!TraceIt.isAtEoF()) {
+    // Skip the aggregated count
+    if (!TraceIt.getCurrentLine().getAsInteger(10, FrameAddr))
+      TraceIt.advance();
+
+    // Detect sample with call stack
+    int32_t Count = 0;
+    while (!TraceIt.isAtEoF() &&
+           !TraceIt.getCurrentLine().ltrim().getAsInteger(16, FrameAddr)) {
+      Count++;
+      TraceIt.advance();
+    }
+    if (!TraceIt.isAtEoF()) {
+      if (isLBRSample(TraceIt.getCurrentLine())) {
+        if (Count > 0)
+          return PERF_LBR_STACK;
+        else
+          return PERF_LBR;
+      }
+      TraceIt.advance();
+    }
   }
-  return PerfType;
+
+  exitWithError("Invalid perf script input!");
+  return PERF_INVALID;
 }
 
 void HybridPerfReader::generateRawProfile() {
@@ -797,12 +856,11 @@ void PerfReaderBase::warnTruncatedStack() {
   }
 }
 
-void PerfReaderBase::parsePerfTraces(
-    cl::list<std::string> &PerfTraceFilenames) {
+void PerfReaderBase::parsePerfTraces() {
   // Parse perf traces and do aggregation.
-  for (auto Filename : PerfTraceFilenames)
-    parseAndAggregateTrace(Filename);
+  parseAndAggregateTrace();
 
+  // Generate unsymbolized profile.
   warnTruncatedStack();
   generateRawProfile();
 

diff  --git a/llvm/tools/llvm-profgen/PerfReader.h b/llvm/tools/llvm-profgen/PerfReader.h
index 86de6555457a3..c9f2240d0151b 100644
--- a/llvm/tools/llvm-profgen/PerfReader.h
+++ b/llvm/tools/llvm-profgen/PerfReader.h
@@ -503,67 +503,25 @@ class VirtualUnwinder {
 // Read perf trace to parse the events and samples.
 class PerfReaderBase {
 public:
-  PerfReaderBase(ProfiledBinary *B) : Binary(B) {
+  PerfReaderBase(ProfiledBinary *B, StringRef PerfTrace,
+                 PerfScriptType Type = PERF_UNKNOWN)
+      : Binary(B), PerfTraceFile(PerfTrace), PerfType(Type) {
     // Initialize the base address to preferred address.
     Binary->setBaseAddress(Binary->getPreferredBaseAddress());
   };
   virtual ~PerfReaderBase() = default;
   static std::unique_ptr<PerfReaderBase>
-  create(ProfiledBinary *Binary, cl::list<std::string> &PerfTraceFilenames);
-
-  // A LBR sample is like:
-  // 40062f 0x5c6313f/0x5c63170/P/-/-/0  0x5c630e7/0x5c63130/P/-/-/0 ...
-  // A heuristic for fast detection by checking whether a
-  // leading "  0x" and the '/' exist.
-  static bool isLBRSample(StringRef Line) {
-    // Skip the leading instruction pointer
-    SmallVector<StringRef, 32> Records;
-    Line.trim().split(Records, " ", 2, false);
-    if (Records.size() < 2)
-      return false;
-    if (Records[1].startswith("0x") && Records[1].find('/') != StringRef::npos)
-      return true;
-    return false;
-  }
+  create(ProfiledBinary *Binary, StringRef PerfInputFile, bool IsPerfData);
 
-  // The raw hybird sample is like
-  // e.g.
-  // 	          4005dc    # call stack leaf
-  //	          400634
-  //	          400684    # call stack root
-  // 0x4005c8/0x4005dc/P/-/-/0   0x40062f/0x4005b0/P/-/-/0 ...
-  //          ... 0x4005c8/0x4005dc/P/-/-/0    # LBR Entries
-  // Determine the perfscript contains hybrid samples(call stack + LBRs) by
-  // checking whether there is a non-empty call stack immediately followed by
-  // a LBR sample
-  static PerfScriptType checkPerfScriptType(StringRef FileName) {
-    TraceStream TraceIt(FileName);
-    uint64_t FrameAddr = 0;
-    while (!TraceIt.isAtEoF()) {
-      // Skip the aggregated count
-      if (!TraceIt.getCurrentLine().getAsInteger(10, FrameAddr))
-        TraceIt.advance();
-
-      // Detect sample with call stack
-      int32_t Count = 0;
-      while (!TraceIt.isAtEoF() &&
-             !TraceIt.getCurrentLine().ltrim().getAsInteger(16, FrameAddr)) {
-        Count++;
-        TraceIt.advance();
-      }
-      if (!TraceIt.isAtEoF()) {
-        if (isLBRSample(TraceIt.getCurrentLine())) {
-          if (Count > 0)
-            return PERF_LBR_STACK;
-          else
-            return PERF_LBR;
-        }
-        TraceIt.advance();
-      }
-    }
-    return PERF_INVALID;
+  PerfScriptType getPerfScriptType() const { return PerfType; }
+  // Entry of the reader to parse multiple perf traces
+  void parsePerfTraces();
+  const ContextSampleCounterMap &getSampleCounters() const {
+    return SampleCounters;
   }
+  bool profileIsCS() { return ProfileIsCS; }
 
+protected:
   // The parsed MMap event
   struct MMapEvent {
     uint64_t PID = 0;
@@ -573,23 +531,17 @@ class PerfReaderBase {
     StringRef BinaryPath;
   };
 
+  // Check whther a given line is LBR sample
+  static bool isLBRSample(StringRef Line);
+  // Extract perf script type by peaking at the input
+  static PerfScriptType checkPerfScriptType(StringRef FileName);
+  // Update base address based on mmap events
   void updateBinaryAddress(const MMapEvent &Event);
-  // Entry of the reader to parse multiple perf traces
-  void parsePerfTraces(cl::list<std::string> &PerfTraceFilenames);
-  const ContextSampleCounterMap &getSampleCounters() const {
-    return SampleCounters;
-  }
-  bool profileIsCS() { return ProfileIsCS; }
-
-protected:
-  static PerfScriptType
-  extractPerfType(cl::list<std::string> &PerfTraceFilenames);
-  /// Parse a single line of a PERF_RECORD_MMAP2 event looking for a
-  /// mapping between the binary name and its memory layout.
-  ///
+  // Parse a single line of a PERF_RECORD_MMAP2 event looking for a
+  // mapping between the binary name and its memory layout.
   void parseMMap2Event(TraceStream &TraceIt);
   // Parse perf events/samples and do aggregation
-  void parseAndAggregateTrace(StringRef Filename);
+  void parseAndAggregateTrace();
   // Parse either an MMAP event or a perf sample
   void parseEventOrSample(TraceStream &TraceIt);
   // Warn if the relevant mmap event is missing.
@@ -616,7 +568,7 @@ class PerfReaderBase {
   void writeRawProfile(raw_fd_ostream &OS);
 
   ProfiledBinary *Binary = nullptr;
-
+  StringRef PerfTraceFile;
   ContextSampleCounterMap SampleCounters;
   // Samples with the repeating time generated by the perf reader
   AggregatedCounter AggregatedSamples;
@@ -635,9 +587,9 @@ class PerfReaderBase {
 */
 class LBRPerfReader : public PerfReaderBase {
 public:
-  LBRPerfReader(ProfiledBinary *Binary) : PerfReaderBase(Binary) {
-    PerfType = PERF_LBR;
-  };
+  LBRPerfReader(ProfiledBinary *Binary, StringRef PerfTrace,
+                PerfScriptType Type = PERF_LBR)
+      : PerfReaderBase(Binary, PerfTrace, Type){};
   // Parse the LBR only sample.
   virtual void parseSample(TraceStream &TraceIt, uint64_t Count) override;
   virtual void generateRawProfile() override;
@@ -657,9 +609,8 @@ class LBRPerfReader : public PerfReaderBase {
 */
 class HybridPerfReader : public LBRPerfReader {
 public:
-  HybridPerfReader(ProfiledBinary *Binary) : LBRPerfReader(Binary) {
-    PerfType = PERF_LBR_STACK;
-  };
+  HybridPerfReader(ProfiledBinary *Binary, StringRef PerfTrace)
+      : LBRPerfReader(Binary, PerfTrace, PERF_LBR_STACK){};
   // Parse the hybrid sample including the call and LBR line
   void parseSample(TraceStream &TraceIt, uint64_t Count) override;
   void generateRawProfile() override;

diff  --git a/llvm/tools/llvm-profgen/llvm-profgen.cpp b/llvm/tools/llvm-profgen/llvm-profgen.cpp
index 0cdf0fa8621cd..48edd4519e4c0 100644
--- a/llvm/tools/llvm-profgen/llvm-profgen.cpp
+++ b/llvm/tools/llvm-profgen/llvm-profgen.cpp
@@ -21,13 +21,20 @@
 
 static cl::OptionCategory ProfGenCategory("ProfGen Options");
 
-static cl::list<std::string> PerfTraceFilenames(
-    "perfscript", cl::value_desc("perfscript"), cl::OneOrMore,
+static cl::opt<std::string> PerfTraceFilename(
+    "perfscript", cl::value_desc("perfscript"), cl::ZeroOrMore,
     llvm::cl::MiscFlags::CommaSeparated,
     cl::desc("Path of perf-script trace created by Linux perf tool with "
              "`script` command(the raw perf.data should be profiled with -b)"),
     cl::cat(ProfGenCategory));
 
+static cl::opt<std::string> PerfDataFilename(
+    "perfdata", cl::value_desc("perfdata"), cl::ZeroOrMore,
+    llvm::cl::MiscFlags::CommaSeparated,
+    cl::desc("Path of raw perf data created by Linux perf tool (it should be "
+             "profiled with -b)"),
+    cl::cat(ProfGenCategory));
+
 static cl::opt<std::string> BinaryPath(
     "binary", cl::value_desc("binary"), cl::Required,
     cl::desc("Path of profiled binary, only one binary is supported."),
@@ -41,20 +48,41 @@ using namespace llvm;
 using namespace sampleprof;
 
 // Validate the command line input.
-static void validateCommandLine(StringRef BinaryPath,
-                                cl::list<std::string> &PerfTraceFilenames) {
+static void validateCommandLine() {
+  // Validate input profile is provided only once
+  if (PerfDataFilename.getNumOccurrences() &&
+      PerfTraceFilename.getNumOccurrences()) {
+    std::string Msg = "`-perfdata` and `-perfscript` cannot be used together.";
+    exitWithError(Msg);
+  }
+
+  // Validate input profile is provided
+  if (!PerfDataFilename.getNumOccurrences() &&
+      !PerfTraceFilename.getNumOccurrences()) {
+    std::string Msg =
+        "Use `-perfdata` or `-perfscript` to provide input perf profile.";
+    exitWithError(Msg);
+  }
+
   // Allow the invalid perfscript if we only use to show binary disassembly.
   if (!ShowDisassemblyOnly) {
-    for (auto &File : PerfTraceFilenames) {
-      if (!llvm::sys::fs::exists(File)) {
-        std::string Msg = "Input perf script(" + File + ") doesn't exist!";
-        exitWithError(Msg);
-      }
+    if (PerfTraceFilename.getNumOccurrences() &&
+        !llvm::sys::fs::exists(PerfTraceFilename)) {
+      std::string Msg =
+          "Input perf script(" + PerfTraceFilename + ") doesn't exist.";
+      exitWithError(Msg);
+    }
+
+    if (PerfDataFilename.getNumOccurrences() &&
+        !llvm::sys::fs::exists(PerfDataFilename)) {
+      std::string Msg =
+          "Input perf data(" + PerfDataFilename + ") doesn't exist.";
+      exitWithError(Msg);
     }
   }
 
   if (!llvm::sys::fs::exists(BinaryPath)) {
-    std::string Msg = "Input binary(" + BinaryPath.str() + ") doesn't exist!";
+    std::string Msg = "Input binary(" + BinaryPath + ") doesn't exist.";
     exitWithError(Msg);
   }
 
@@ -77,7 +105,7 @@ int main(int argc, const char *argv[]) {
 
   cl::HideUnrelatedOptions({&ProfGenCategory, &getColorCategory()});
   cl::ParseCommandLineOptions(argc, argv, "llvm SPGO profile generator\n");
-  validateCommandLine(BinaryPath, PerfTraceFilenames);
+  validateCommandLine();
 
   // Load symbols and disassemble the code of a given binary.
   std::unique_ptr<ProfiledBinary> Binary =
@@ -86,9 +114,15 @@ int main(int argc, const char *argv[]) {
     return EXIT_SUCCESS;
 
   // Parse perf events and samples
+  StringRef PerfInputFile;
+  bool IsPerfData = PerfDataFilename.getNumOccurrences();
+  if (IsPerfData)
+    PerfInputFile = PerfDataFilename;
+  else
+    PerfInputFile = PerfTraceFilename;
   std::unique_ptr<PerfReaderBase> Reader =
-      PerfReaderBase::create(Binary.get(), PerfTraceFilenames);
-  Reader->parsePerfTraces(PerfTraceFilenames);
+      PerfReaderBase::create(Binary.get(), PerfInputFile, IsPerfData);
+  Reader->parsePerfTraces();
 
   if (SkipSymbolization)
     return EXIT_SUCCESS;


        


More information about the llvm-commits mailing list