Addressing reviewers (1)

Author: paschalis-mpeis

bolt/include/bolt/Utils/CommandLineOpts.h

@@ -33,6 +33,7 @@ extern llvm::cl::OptionCategory BinaryAnalysisCategory;
 extern llvm::cl::opt<unsigned> AlignText;
 extern llvm::cl::opt<unsigned> AlignFunctions;
 extern llvm::cl::opt<bool> AggregateOnly;
+extern llvm::cl::opt<bool> ArmSPE;
 extern llvm::cl::opt<unsigned> BucketsPerLine;
 extern llvm::cl::opt<bool> DiffOnly;
 extern llvm::cl::opt<bool> EnableBAT;

bolt/lib/Profile/DataAggregator.cpp

@@ -1277,6 +1277,7 @@ DataAggregator::parseSpeAsBasicSamples() {
   ErrorOr<uint64_t> AddrResTo = parseHexField(FieldSeparator);
   if (std::error_code EC = AddrResTo.getError())
     return EC;
+
   consumeAllRemainingFS();
 
   ErrorOr<uint64_t> AddrResFrom = parseHexField(FieldSeparator, true);
@@ -1296,12 +1297,13 @@ DataAggregator::parseSpeAsBasicSamples() {
 
     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 ? "branch-spe:" : "instruction-spe:";
+    Event = *AddrResTo != 0x0 ? "branches-spe:" : "instructions-spe:";
 
   return std::make_pair(genBasicSample(*AddrResFrom),
                         genBasicSample(*AddrResTo));

bolt/tools/driver/llvm-bolt.cpp

@@ -51,8 +51,6 @@ static cl::opt<std::string> InputFilename(cl::Positional,
                                           cl::Required, cl::cat(BoltCategory),
                                           cl::sub(cl::SubCommand::getAll()));
 
-extern cl::opt<bool> ArmSPE;
-
 static cl::opt<std::string>
 InputDataFilename("data",
   cl::desc("<data file>"),
@@ -249,7 +247,7 @@ int main(int argc, char **argv) {
       RewriteInstance &RI = *RIOrErr.get();
 
       if (opts::AggregateOnly && !RI.getBinaryContext().isAArch64() &&
-          opts::ArmSPE == 1) {
+          opts::ArmSPE) {
         errs() << "BOLT-ERROR: -spe is available only on AArch64.\n";
         exit(1);
       }

bolt/unittests/Profile/PerfSpeEvents.cpp

@@ -110,7 +110,7 @@ TEST_F(PerfSpeEventsTestHelper, SpeBranches) {
       "1234          instructions:              d002    d001\n"
       "1234          instructions:              e002    e001\n";
 
-  EXPECT_TRUE(checkEvents(1234, 10, {"branch-spe:"}));
+  EXPECT_TRUE(checkEvents(1234, 10, {"branches-spe:"}));
 }
 
 TEST_F(PerfSpeEventsTestHelper, SpeBranchesAndCycles) {
@@ -127,7 +127,7 @@ TEST_F(PerfSpeEventsTestHelper, SpeBranchesAndCycles) {
       "1234          instructions:              d002    d001\n"
       "1234          instructions:              e002    e001\n";
 
-  EXPECT_TRUE(checkEvents(1234, 8, {"branch-spe:", "cycles:u:"}));
+  EXPECT_TRUE(checkEvents(1234, 8, {"branches-spe:", "cycles:u:"}));
 }
 
 TEST_F(PerfSpeEventsTestHelper, SpeAnyEventAndCycles) {
@@ -144,8 +144,8 @@ TEST_F(PerfSpeEventsTestHelper, SpeAnyEventAndCycles) {
       "1234          instructions:                0     d001\n"
       "1234          instructions:              e002    e001\n";
 
-  EXPECT_TRUE(
-      checkEvents(1234, 6, {"cycles:u:", "instruction-spe:", "branch-spe:"}));
+  EXPECT_TRUE(checkEvents(1234, 6,
+                          {"cycles:u:", "instructions-spe:", "branches-spe:"}));
 }
 
 TEST_F(PerfSpeEventsTestHelper, SpeNoBranchPairsRecorded) {
@@ -164,7 +164,7 @@ TEST_F(PerfSpeEventsTestHelper, SpeNoBranchPairsRecorded) {
       "1234              cycles:u:                 0    d001\n"
       "1234          instructions:                 0    e001\n";
 
-  EXPECT_TRUE(checkEvents(1234, 5, {"instruction-spe:", "cycles:u:"}));
+  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");