resolve comments

Author: mingmingl-llvm

llvm/include/llvm/ProfileData/SampleProf.h

@@ -1027,6 +1027,20 @@ class FunctionSamples {
     return VirtualCallsiteTypeCounts[mapIRLocToProfileLoc(Loc)];
   }
 
+  // At location \p Loc, add a type sample for the given \p Type with
+  // \p Count. This function uses saturating arithmetic to clamp the result to
+  // maximum uint64_t (the counter type) and returns counter_overflow to caller
+  // if the actual result is larger than maximum uint64_t.
+  sampleprof_error addTypeSamplesAt(const LineLocation &Loc, FunctionId Type,
+                                    uint64_t Count) {
+    auto &TypeCounts = getTypeSamplesAt(Loc);
+    bool Overflowed = false;
+    TypeCounts[Type] = SaturatingMultiplyAdd(Count, /* Weight= */ (uint64_t)1,
+                                             TypeCounts[Type], &Overflowed);
+    return Overflowed ? sampleprof_error::counter_overflow
+                      : sampleprof_error::success;
+  }
+
   /// Scale \p Other sample counts by \p Weight and add the scaled result to the
   /// type samples for the undrifted location of \p Loc.
   template <typename T>

llvm/test/tools/llvm-profgen/Inputs/dap-perf-trace.txt

@@ -1,7 +1,7 @@
-0 0x7b10 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x200000(0x60000) @ 0 08:01 527501 0]: r--p /path/to/dap.perfbin
-0 0x7b98 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x260000(0x153000) @ 0x5f000 08:01 527501 0]: r-xp /path/to/dap.perfbin
-0 0x7c20 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x3b3000(0xc000) @ 0x1b1000 08:01 527501 0]: r--p /path/to/dap.perfbin
-0 0x7ca8 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x3bf000(0x3000) @ 0x1bc000 08:01 527501 0]: rw-p /path/to/dap.perfbin
+0 0x7b10 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x200000(0x60000) @ 0 08:01 527501 0]: r--p /path/to/dap.bin
+0 0x7b98 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x260000(0x153000) @ 0x5f000 08:01 527501 0]: r-xp /path/to/dap.bin
+0 0x7c20 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x3b3000(0xc000) @ 0x1b1000 08:01 527501 0]: r--p /path/to/dap.bin
+0 0x7ca8 [0x88]: PERF_RECORD_MMAP2 3446532/3446532: [0x3bf000(0x3000) @ 0x1bc000 08:01 527501 0]: rw-p /path/to/dap.bin
 1282514021937402 0x8660 [0x60]: PERF_RECORD_SAMPLE(IP, 0x4002): 3446532/3446532: 0x2608a2 period: 233 addr: 0x3b3f70
 1282514022939813 0x87b0 [0x60]: PERF_RECORD_SAMPLE(IP, 0x4002): 3446532/3446532: 0x2608a2 period: 233 addr: 0x3b3fb0
 1282514023932029 0x8a00 [0x60]: PERF_RECORD_SAMPLE(IP, 0x4002): 3446532/3446532: 0x2608a2 period: 233 addr: 0x3b3fb0

llvm/test/tools/llvm-profgen/Inputs/dap.perfbin renamed to llvm/test/tools/llvm-profgen/Inputs/dap.bin

@@ -1,5 +1,5 @@
 RUN: llvm-profgen --perfscript=%p/Inputs/lbr-perf-for-dap.script --data-access-profile=%p/Inputs/dap-perf-trace.txt \
-RUN:  --binary=%p/Inputs/dap.perfbin --format=text --pid=3446532 \
+RUN:  --binary=%p/Inputs/dap.bin --format=text --pid=3446532 \
 RUN: -ignore-stack-samples -use-dwarf-correlation -o %t.afdo
 
 RUN: llvm-profdata show --sample --function=_Z9loop_funciii %t.afdo 2>&1 | FileCheck %s

llvm/test/tools/llvm-profgen/afdo-with-vtable.test

@@ -18,8 +18,6 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/ToolOutputFile.h"
 
-#include <regex>
-
 #define DEBUG_TYPE "perf-reader"
 
 cl::opt<bool> SkipSymbolization("skip-symbolization",
@@ -377,15 +375,24 @@ PerfReaderBase::create(ProfiledBinary *Binary, PerfInputFile &PerfInput,
   return PerfReader;
 }
 
-void PerfReaderBase::parseDataAccessPerfTraces(
+Error PerfReaderBase::parseDataAccessPerfTraces(
     StringRef DataAccessPerfTraceFile, std::optional<int32_t> PIDFilter) {
-  std::regex logRegex(
-      R"(^.*?PERF_RECORD_SAMPLE\(.*?\):\s*(\d+)\/(\d+):\s*(0x[0-9a-fA-F]+)\s+period:\s*\d+\s+addr:\s*(0x[0-9a-fA-F]+)$)");
+  // A perf_record_sample line is like
+  // . 1282514022939813 0x87b0 [0x60]: PERF_RECORD_SAMPLE(IP, 0x4002):
+  // 3446532/3446532: 0x2608a2 period: 233 addr: 0x3b3fb0
+  constexpr static const char *const DataAccessSamplePattern =
+      "PERF_RECORD_SAMPLE\\([A-Za-z]+, 0x[0-9a-fA-F]+\\): "
+      "([0-9]+)\\/[0-9]+: (0x[0-9a-fA-F]+) period: [0-9]+ addr: "
+      "(0x[0-9a-fA-F]+)";
+
+  llvm::Regex logRegex(DataAccessSamplePattern);
 
   auto BufferOrErr = MemoryBuffer::getFile(DataAccessPerfTraceFile);
   std::error_code EC = BufferOrErr.getError();
   if (EC)
-    exitWithError("Failed to open perf trace file: " + DataAccessPerfTraceFile);
+    return make_error<StringError>("Failed to open perf trace file: " +
+                                       DataAccessPerfTraceFile,
+                                   inconvertibleErrorCode());
 
   assert(!SampleCounters.empty() && "Sample counters should not be empty!");
   SampleCounter &Counter = SampleCounters.begin()->second;
@@ -403,33 +410,28 @@ void PerfReaderBase::parseDataAccessPerfTraces(
       continue;
     }
 
-    // Skip lines that do not contain "PERF_RECORD_SAMPLE".
-    if (!Line.contains("PERF_RECORD_SAMPLE")) {
-      continue;
-    }
-
-    std::smatch matches;
-    const std::string LineStr = Line.str();
-
-    if (std::regex_search(LineStr.begin(), LineStr.end(), matches, logRegex)) {
-      if (matches.size() != 5)
-        continue;
-
-      const int32_t PID = std::stoi(matches[1].str());
-      if (PIDFilter && *PIDFilter != PID) {
+    SmallVector<StringRef> Fields;
+    if (logRegex.match(Line, &Fields)) {
+      int32_t PID = 0;
+      Fields[1].getAsInteger(0, PID);
+      if (PIDFilter.has_value() && *PIDFilter != PID) {
         continue;
       }
 
-      const uint64_t DataAddress = std::stoull(matches[4].str(), nullptr, 16);
+      uint64_t DataAddress = 0;
+      Fields[3].getAsInteger(0, DataAddress);
+
       StringRef DataSymbol = Binary->symbolizeDataAddress(
           Binary->CanonicalizeNonTextAddress(DataAddress));
       if (DataSymbol.starts_with("_ZTV")) {
-        const uint64_t IP = std::stoull(matches[3].str(), nullptr, 16);
+        uint64_t IP = 0;
+        Fields[2].getAsInteger(0, IP);
         Counter.recordDataAccessCount(Binary->canonicalizeVirtualAddress(IP),
                                       DataSymbol, 1);
       }
     }
   }
+  return Error::success();
 }
 
 PerfInputFile

llvm/tools/llvm-profgen/PerfReader.cpp

@@ -12,6 +12,7 @@
 #include "ProfiledBinary.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/Regex.h"
 #include <cstdint>
 #include <fstream>
@@ -396,6 +397,7 @@ using BranchSample = std::map<std::pair<uint64_t, uint64_t>, uint64_t>;
 // which is represented as the start and end offset pair.
 using RangeSample = std::map<std::pair<uint64_t, uint64_t>, uint64_t>;
 // <<inst-addr, vtable-data-symbol>, count> map for data access samples.
+// The instruction address is the virtual address in the binary.
 using DataAccessSample = std::map<std::pair<uint64_t, StringRef>, uint64_t>;
 // Wrapper for sample counters including range counter and branch counter
 struct SampleCounter {
@@ -581,8 +583,8 @@ class PerfReaderBase {
   virtual void parsePerfTraces() = 0;
 
   // Parse the <ip, vtable-data-symbol> from the data access perf trace file,
-  // and accummuate the data access count for each <ip, data-symbol> pair.
-  void
+  // and accumulate the data access count for each <ip, data-symbol> pair.
+  Error
   parseDataAccessPerfTraces(StringRef DataAccessPerfFile,
                             std::optional<int32_t> PIDFilter = std::nullopt);
 
@@ -615,6 +617,8 @@ class PerfScriptReader : public PerfReaderBase {
 
   // Parse a single line of a PERF_RECORD_MMAP event looking for a
   // mapping between the binary name and its memory layout.
+  // TODO: Move this static method from PerScriptReader (subclass) to
+  // PerfReaderBase (superclass).
   static bool extractMMapEventForBinary(ProfiledBinary *Binary, StringRef Line,
                                         MMapEvent &MMap);
 

llvm/tools/llvm-profgen/PerfReader.h

@@ -552,7 +552,7 @@ void ProfileGenerator::generateLineNumBasedProfile() {
       LineLocation Loc(
           FrameVec.back().Location.LineOffset,
           getBaseDiscriminator(FrameVec.back().Location.Discriminator));
-      FunctionProfile.getTypeSamplesAt(Loc)[FunctionId(IpData.second)] += Count;
+      FunctionProfile.addTypeSamplesAt(Loc, FunctionId(IpData.second), Count);
     }
   }
 

llvm/tools/llvm-profgen/ProfileGenerator.cpp

@@ -340,7 +340,7 @@ void ProfiledBinary::setPreferredTextSegmentAddresses(const ELFFile<ELFT> &Obj,
         PhdrInfo Info;
         Info.FileOffset = Phdr.p_offset;
         Info.FileSz = Phdr.p_filesz;
-        Info.vAddr = Phdr.p_vaddr;
+        Info.VirtualAddr = Phdr.p_vaddr;
         NonTextPhdrInfo.push_back(Info);
       }
     }
@@ -369,7 +369,7 @@ uint64_t ProfiledBinary::CanonicalizeNonTextAddress(uint64_t Address) {
     if (PhdrInfo.FileOffset <= FileOffset &&
         FileOffset < PhdrInfo.FileOffset + PhdrInfo.FileSz) {
       // If it is, return the virtual address of the segment.
-      return PhdrInfo.vAddr + (FileOffset - PhdrInfo.FileOffset);
+      return PhdrInfo.VirtualAddr + (FileOffset - PhdrInfo.FileOffset);
     }
   }
 
@@ -945,11 +945,10 @@ SampleContextFrameVector ProfiledBinary::symbolize(const InstructionPointer &IP,
                                                    bool UseProbeDiscriminator) {
   assert(this == IP.Binary &&
          "Binary should only symbolize its own instruction");
-  auto Addr = object::SectionedAddress{IP.Address,
-                                       object::SectionedAddress::UndefSection};
-  DIInliningInfo InlineStack = unwrapOrError(
-      Symbolizer->symbolizeInlinedCode(SymbolizerPath.str(), Addr),
-      SymbolizerPath);
+  DIInliningInfo InlineStack =
+      unwrapOrError(Symbolizer->symbolizeInlinedCode(
+                        SymbolizerPath.str(), getSectionedAddress(IP.Address)),
+                    SymbolizerPath);
 
   SampleContextFrameVector CallStack;
   for (int32_t I = InlineStack.getNumberOfFrames() - 1; I >= 0; I--) {
@@ -979,11 +978,13 @@ SampleContextFrameVector ProfiledBinary::symbolize(const InstructionPointer &IP,
 }
 
 StringRef ProfiledBinary::symbolizeDataAddress(uint64_t Address) {
-  DIGlobal DataDIGlobal = unwrapOrError(
-      Symbolizer->symbolizeData(SymbolizerPath.str(), {Address, 0}),
-      SymbolizerPath);
-  auto It = NameStrings.insert(DataDIGlobal.Name);
-  return StringRef(*It.first);
+  DIGlobal DataDIGlobal =
+      unwrapOrError(Symbolizer->symbolizeData(SymbolizerPath.str(),
+                                              getSectionedAddress(Address)),
+                    SymbolizerPath);
+  decltype(NameStrings)::iterator Iter;
+  std::tie(Iter, std::ignore) = NameStrings.insert(DataDIGlobal.Name);
+  return StringRef(*Iter);
 }
 
 void ProfiledBinary::computeInlinedContextSizeForRange(uint64_t RangeBegin,

llvm/tools/llvm-profgen/ProfiledBinary.cpp

@@ -293,7 +293,7 @@ class ProfiledBinary {
   struct PhdrInfo {
     uint64_t FileOffset;
     uint64_t FileSz;
-    uint64_t vAddr;
+    uint64_t VirtualAddr;
   };
 
   // Program header information for non-text PT_LOAD segments.
@@ -328,6 +328,16 @@ class ProfiledBinary {
 
   void setPreferredTextSegmentAddresses(const ObjectFile *O);
 
+  // LLVMSymbolizer's symbolize{Code, Data} interfaces requires a section index
+  // for each address to be symbolized. This is a helper function to
+  // construct a SectionedAddress object with the given address and section
+  // index. The section index is set to UndefSection by default.
+  static object::SectionedAddress getSectionedAddress(
+      uint64_t Address,
+      uint64_t SectionIndex = object::SectionedAddress::UndefSection) {
+    return object::SectionedAddress{Address, SectionIndex};
+  }
+
   template <class ELFT>
   void setPreferredTextSegmentAddresses(const ELFFile<ELFT> &Obj,
                                         StringRef FileName);
@@ -634,8 +644,10 @@ class ProfiledBinary {
     MMapNonTextEvents.push_back(MMap);
   }
 
-  // Given a runtime address, canonicalize it to the virtual address in the
-  // binary.
+  // Given a non-text runtime address, canonicalize it to the virtual address in
+  // the binary.
+  // TODO: Consider unifying the canonicalization of text and non-text addresses
+  // in the ProfiledBinary class.
   uint64_t CanonicalizeNonTextAddress(uint64_t Address);
 
   bool getTrackFuncContextSize() { return TrackFuncContextSize; }

llvm/tools/llvm-profgen/ProfiledBinary.h

@@ -69,7 +69,7 @@ static cl::opt<std::string> DebugBinPath(
 
 static cl::opt<std::string> DataAccessProfileFilename(
     "data-access-profile", cl::value_desc("data-access-profile"),
-    cl::desc("Path of the data access profile to be generated."),
+    cl::desc("Path to the data access profile to be generated."),
     cl::cat(ProfGenCategory));
 
 extern cl::opt<bool> ShowDisassemblyOnly;
@@ -188,7 +188,12 @@ int main(int argc, const char *argv[]) {
       // Parse the data access perf traces into <ip, data-addr> pairs, symbolize
       // the data-addr to data-symbol. If the data-addr is a vtable, increment
       // counters for the <ip, data-symbol> pair.
-      Reader->parseDataAccessPerfTraces(DataAccessProfileFilename, PIDFilter);
+      if (Error E = Reader->parseDataAccessPerfTraces(DataAccessProfileFilename,
+                                                      PIDFilter)) {
+        handleAllErrors(std::move(E), [&](const StringError &SE) {
+          exitWithError(SE.getMessage());
+        });
+      }
     }
 
     if (SkipSymbolization)