Removing dependency of the SPE BasicAggregation

This commit aim is to uncouple the SPE BRStack and SPE BasicAggregation approaches
based on the decision in issue #115333.

BRStack change relies on the unit test logic which was introduced by
Paschalis Mpeis (ARM) in #120741. Since it is a common part of the two aggregation
type technique, needs to retain an essential part of it.

All relevant tests to BasicAggregation is removed.

Co-Authored-By: Paschalis Mpeis <[email protected]>

Author: kaadam

bolt/include/bolt/Profile/DataAggregator.h

@@ -288,15 +288,6 @@ class DataAggregator : public DataReader {
   /// and a PC
   ErrorOr<PerfBasicSample> parseBasicSample();
 
-  /// Parse an Arm SPE entry into the non-lbr format by generating two basic
-  /// samples. The format of an input SPE entry is:
-  /// ```
-  /// PID   EVENT-TYPE   ADDR   IP
-  /// ```
-  /// SPE branch events will have 'ADDR' set to a branch target address while
-  /// other perf or SPE events will have it set to zero.
-  ErrorOr<std::pair<PerfBasicSample, PerfBasicSample>> parseSpeAsBasicSamples();
-
   /// Parse a single perf sample containing a PID associated with an IP and
   /// address.
   ErrorOr<PerfMemSample> parseMemSample();
@@ -343,9 +334,6 @@ class DataAggregator : public DataReader {
   /// Process non-LBR events.
   void processBasicEvents();
 
-  /// Parse Arm SPE events into the non-LBR format.
-  std::error_code parseSpeAsBasicEvents();
-
   /// Parse the full output generated by perf script to report memory events.
   std::error_code parseMemEvents();
 

bolt/lib/Profile/DataAggregator.cpp

@@ -181,17 +181,11 @@ void DataAggregator::start() {
   findPerfExecutable();
 
   if (opts::ArmSPE) {
-    if (!opts::BasicAggregation) {
-      // pid    from_ip      to_ip        predicted/missed not-taken?
-      // 12345  0x123/0x456/PN/-/-/8/RET/-
-      launchPerfProcess("SPE brstack events", MainEventsPPI,
-                        "script -F pid,brstack --itrace=bl",
-                        /*Wait = */ false);
-    } else {
-      launchPerfProcess("SPE branch events (non-lbr)", MainEventsPPI,
-                        "script -F pid,event,ip,addr --itrace=i1i",
-                        /*Wait = */ false);
-    }
+    // pid    from_ip      to_ip        predicted/missed not-taken?
+    // 12345  0x123/0x456/PN/-/-/8/RET/-
+    launchPerfProcess("SPE brstack events", MainEventsPPI,
+                      "script -F pid,brstack --itrace=bl",
+                      /*Wait = */ false);
   } else if (opts::BasicAggregation) {
     launchPerfProcess("events without LBR", MainEventsPPI,
                       "script -F pid,event,ip",
@@ -464,20 +458,14 @@ int DataAggregator::prepareToParse(StringRef Name, PerfProcessInfo &Process,
 Error DataAggregator::preprocessProfile(BinaryContext &BC) {
   this->BC = &BC;
 
-  const Regex NoData("Samples for '.*' event do not have ADDR attribute set. "
-                     "Cannot print 'addr' field.");
-
-  auto ErrorCallback = [&NoData](int ReturnCode, StringRef ErrBuf) {
-    if (opts::ArmSPE && NoData.match(ErrBuf)) {
-      errs() << "PERF2BOLT-ERROR: perf data are incompatible for Arm SPE mode "
-                "consumption. ADDR attribute is unset.\n";
-      exit(1);
-    }
+  auto ErrorCallback = [](int ReturnCode, StringRef ErrBuf) {
     errs() << "PERF-ERROR: return code " << ReturnCode << "\n" << ErrBuf;
     exit(1);
   };
 
   auto MemEventsErrorCallback = [&](int ReturnCode, StringRef ErrBuf) {
+    Regex NoData("Samples for '.*' event do not have ADDR attribute set. "
+                 "Cannot print 'addr' field.");
     if (!NoData.match(ErrBuf))
       ErrorCallback(ReturnCode, ErrBuf);
   };
@@ -524,7 +512,6 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) {
   prepareToParse("events", MainEventsPPI, ErrorCallback);
 
   if ((!opts::BasicAggregation && parseBranchEvents()) ||
-      (opts::BasicAggregation && opts::ArmSPE && parseSpeAsBasicEvents()) ||
       (opts::BasicAggregation && parseBasicEvents()))
     errs() << "PERF2BOLT: failed to parse samples\n";
 
@@ -1163,68 +1150,6 @@ ErrorOr<DataAggregator::PerfBasicSample> DataAggregator::parseBasicSample() {
   return PerfBasicSample{Event.get(), Address};
 }
 
-ErrorOr<
-    std::pair<DataAggregator::PerfBasicSample, DataAggregator::PerfBasicSample>>
-DataAggregator::parseSpeAsBasicSamples() {
-  while (checkAndConsumeFS()) {
-  }
-
-  ErrorOr<int64_t> PIDRes = parseNumberField(FieldSeparator, true);
-  if (std::error_code EC = PIDRes.getError())
-    return EC;
-
-  constexpr PerfBasicSample EmptySample = PerfBasicSample{StringRef(), 0};
-  auto MMapInfoIter = BinaryMMapInfo.find(*PIDRes);
-  if (MMapInfoIter == BinaryMMapInfo.end()) {
-    consumeRestOfLine();
-    return std::make_pair(EmptySample, EmptySample);
-  }
-
-  while (checkAndConsumeFS()) {
-  }
-
-  ErrorOr<StringRef> Event = parseString(FieldSeparator);
-  if (std::error_code EC = Event.getError())
-    return EC;
-
-  while (checkAndConsumeFS()) {
-  }
-
-  ErrorOr<uint64_t> AddrResTo = parseHexField(FieldSeparator);
-  if (std::error_code EC = AddrResTo.getError())
-    return EC;
-
-  consumeAllRemainingFS();
-
-  ErrorOr<uint64_t> AddrResFrom = parseHexField(FieldSeparator, true);
-  if (std::error_code EC = AddrResFrom.getError())
-    return EC;
-
-  if (!checkAndConsumeNewLine()) {
-    reportError("expected end of line");
-    return make_error_code(llvm::errc::io_error);
-  }
-
-  auto genBasicSample = [&](uint64_t Address) {
-    // When fed with non SPE branch events the target address will be null.
-    // This is expected and ignored.
-    if (Address == 0x0)
-      return EmptySample;
-
-    if (!BC->HasFixedLoadAddress)
-      adjustAddress(Address, MMapInfoIter->second);
-
-    return PerfBasicSample{Event.get(), Address};
-  };
-
-  // Show more meaningful event names on boltdata.
-  if (Event->str() == "instructions:")
-    Event = *AddrResTo != 0x0 ? "branches-spe:" : "instructions-spe:";
-
-  return std::make_pair(genBasicSample(*AddrResFrom),
-                        genBasicSample(*AddrResTo));
-}
-
 ErrorOr<DataAggregator::PerfMemSample> DataAggregator::parseMemSample() {
   PerfMemSample Res{0, 0};
 
@@ -1734,46 +1659,6 @@ std::error_code DataAggregator::parseBasicEvents() {
   return std::error_code();
 }
 
-std::error_code DataAggregator::parseSpeAsBasicEvents() {
-  outs() << "PERF2BOLT: parsing SPE data as basic events (no LBR)...\n";
-  NamedRegionTimer T("parseSPEBasic", "Parsing SPE as basic events",
-                     TimerGroupName, TimerGroupDesc, opts::TimeAggregator);
-  uint64_t NumSpeBranchSamples = 0;
-
-  // Convert entries to one or two basic samples, depending on whether there is
-  // branch target information.
-  while (hasData()) {
-    auto SamplePair = parseSpeAsBasicSamples();
-    if (std::error_code EC = SamplePair.getError())
-      return EC;
-
-    auto registerSample = [this](const PerfBasicSample *Sample) {
-      if (!Sample->PC)
-        return;
-
-      if (BinaryFunction *BF = getBinaryFunctionContainingAddress(Sample->PC))
-        BF->setHasProfileAvailable();
-
-      ++BasicSamples[Sample->PC];
-      EventNames.insert(Sample->EventName);
-    };
-
-    if (SamplePair->first.PC != 0x0 && SamplePair->second.PC != 0x0)
-      ++NumSpeBranchSamples;
-
-    registerSample(&SamplePair->first);
-    registerSample(&SamplePair->second);
-  }
-
-  if (NumSpeBranchSamples == 0)
-    errs() << "PERF2BOLT-WARNING: no SPE branches found\n";
-  else
-    outs() << "PERF2BOLT: found " << NumSpeBranchSamples
-           << " SPE branch sample pairs.\n";
-
-  return std::error_code();
-}
-
 void DataAggregator::processBasicEvents() {
   outs() << "PERF2BOLT: processing basic events (without LBR)...\n";
   NamedRegionTimer T("processBasic", "Processing basic events", TimerGroupName,

bolt/test/perf2bolt/AArch64/perf2bolt-spe.test

@@ -4,10 +4,7 @@ REQUIRES: system-linux,perf,target=aarch64{{.*}}
 
 RUN: perf record -e cycles -q -o %t.perf.data -- %t.exe 2> /dev/null
 
-RUN: (perf2bolt -p %t.perf.data -o %t.perf.boltdata --spe --nl %t.exe 2> /dev/null; exit 0) | FileCheck %s --check-prefix=CHECK-SPE-NO-LBR
-
 RUN: (perf2bolt -p %t.perf.data -o %t.perf.boltdata --spe %t.exe 2> /dev/null; exit 0) | FileCheck %s --check-prefix=CHECK-SPE-LBR
 
-CHECK-SPE-NO-LBR: PERF2BOLT: spawning perf job to read SPE branch events (non-lbr)
 CHECK-SPE-LBR: PERF2BOLT: spawning perf job to read SPE brstack events
 

bolt/unittests/Profile/PerfSpeEvents.cpp

@@ -73,25 +73,6 @@ struct PerfSpeEventsTestHelper : public testing::Test {
   std::unique_ptr<ObjectFile> ObjFile;
   std::unique_ptr<BinaryContext> BC;
 
-  /// Return true when the expected \p SampleSize profile data are generated and
-  /// contain all the \p ExpectedEventNames.
-  bool checkEvents(uint64_t PID, size_t SampleSize,
-                   const StringSet<> &ExpectedEventNames) {
-    DataAggregator DA("<pseudo input>");
-    DA.ParsingBuf = opts::ReadPerfEvents;
-    DA.BC = BC.get();
-    DataAggregator::MMapInfo MMap;
-    DA.BinaryMMapInfo.insert(std::make_pair(PID, MMap));
-
-    DA.parseSpeAsBasicEvents();
-
-    for (auto &EE : ExpectedEventNames)
-      if (!DA.EventNames.contains(EE.first()))
-        return false;
-
-    return SampleSize == DA.BasicSamples.size();
-  }
-
   /// Compare LBREntries
   bool checkLBREntry(const LBREntry &Lhs, const LBREntry &Rhs) {
     return Lhs.From == Rhs.From && Lhs.To == Rhs.To &&
@@ -135,26 +116,6 @@ struct PerfSpeEventsTestHelper : public testing::Test {
 } // namespace bolt
 } // namespace llvm
 
-// Check that DataAggregator can parseSpeAsBasicEvents for branch events when
-// combined with other event types.
-
-TEST_F(PerfSpeEventsTestHelper, SpeBranches) {
-  // Check perf input with SPE branch events.
-  // Example collection command:
-  // ```
-  // perf record -e 'arm_spe_0/branch_filter=1/u' -- BINARY
-  // ```
-
-  opts::ReadPerfEvents =
-      "1234          instructions:              a002    a001\n"
-      "1234          instructions:              b002    b001\n"
-      "1234          instructions:              c002    c001\n"
-      "1234          instructions:              d002    d001\n"
-      "1234          instructions:              e002    e001\n";
-
-  EXPECT_TRUE(checkEvents(1234, 10, {"branches-spe:"}));
-}
-
 TEST_F(PerfSpeEventsTestHelper, SpeBranchesWithBrstack) {
   // Check perf input with SPE branch events as brstack format.
   // Example collection command:
@@ -182,61 +143,4 @@ TEST_F(PerfSpeEventsTestHelper, SpeBranchesWithBrstack) {
   parseAndCheckBrstackEvents(1234, ExpectedSamples);
 }
 
-TEST_F(PerfSpeEventsTestHelper, SpeBranchesAndCycles) {
-  // Check perf input with SPE branch events and cycles.
-  // Example collection command:
-  // ```
-  // perf record -e cycles:u -e 'arm_spe_0/branch_filter=1/u' -- BINARY
-  // ```
-
-  opts::ReadPerfEvents =
-      "1234          instructions:              a002    a001\n"
-      "1234              cycles:u:                 0    b001\n"
-      "1234              cycles:u:                 0    c001\n"
-      "1234          instructions:              d002    d001\n"
-      "1234          instructions:              e002    e001\n";
-
-  EXPECT_TRUE(checkEvents(1234, 8, {"branches-spe:", "cycles:u:"}));
-}
-
-TEST_F(PerfSpeEventsTestHelper, SpeAnyEventAndCycles) {
-  // Check perf input with any SPE event type and cycles.
-  // Example collection command:
-  // ```
-  // perf record -e cycles:u -e 'arm_spe_0//u' -- BINARY
-  // ```
-
-  opts::ReadPerfEvents =
-      "1234              cycles:u:                0     a001\n"
-      "1234              cycles:u:                0     b001\n"
-      "1234          instructions:                0     c001\n"
-      "1234          instructions:                0     d001\n"
-      "1234          instructions:              e002    e001\n";
-
-  EXPECT_TRUE(checkEvents(1234, 6,
-                          {"cycles:u:", "instructions-spe:", "branches-spe:"}));
-}
-
-TEST_F(PerfSpeEventsTestHelper, SpeNoBranchPairsRecorded) {
-  // Check perf input that has no SPE branch pairs recorded.
-  // Example collection command:
-  // ```
-  // perf record -e cycles:u -e 'arm_spe_0/load_filter=1,branch_filter=0/u' --
-  // BINARY
-  // ```
-
-  testing::internal::CaptureStderr();
-  opts::ReadPerfEvents =
-      "1234          instructions:                 0    a001\n"
-      "1234              cycles:u:                 0    b001\n"
-      "1234          instructions:                 0    c001\n"
-      "1234              cycles:u:                 0    d001\n"
-      "1234          instructions:                 0    e001\n";
-
-  EXPECT_TRUE(checkEvents(1234, 5, {"instructions-spe:", "cycles:u:"}));
-
-  std::string Stderr = testing::internal::GetCapturedStderr();
-  EXPECT_EQ(Stderr, "PERF2BOLT-WARNING: no SPE branches found\n");
-}
-
 #endif