Merge branch 'main' into loop_access_analysis_partial_result

Author: john-brawn-arm

.github/new-prs-labeler.yml

@@ -702,6 +702,7 @@ mlgo:
   - llvm/unittests/CodeGen/ML*
   - llvm/test/CodeGen/MLRegAlloc/**
   - llvm/utils/mlgo-utils/**
+  - llvm/docs/MLGO.rst
 
 tools:llvm-exegesis:
   - llvm/tools/llvm-exegesis/**

bolt/include/bolt/Profile/DataAggregator.h

@@ -212,11 +212,6 @@ class DataAggregator : public DataReader {
   uint64_t NumTraces{0};
   uint64_t NumInvalidTraces{0};
   uint64_t NumLongRangeTraces{0};
-  /// Specifies how many samples were recorded in cold areas if we are dealing
-  /// with profiling data collected in a bolted binary. For LBRs, incremented
-  /// for the source of the branch to avoid counting cold activity twice (one
-  /// for source and another for destination).
-  uint64_t NumColdSamples{0};
   uint64_t NumTotalSamples{0};
 
   /// Looks into system PATH for Linux Perf and set up the aggregator to use it
@@ -473,7 +468,6 @@ class DataAggregator : public DataReader {
   void dump(const PerfMemSample &Sample) const;
 
   /// Profile diagnostics print methods
-  void printColdSamplesDiagnostic() const;
   void printLongRangeTracesDiagnostic() const;
   void printBranchSamplesDiagnostics() const;
   void printBasicSamplesDiagnostics(uint64_t OutOfRangeSamples) const;

bolt/include/bolt/Profile/DataReader.h

@@ -114,10 +114,8 @@ struct FuncBranchData {
 
   FuncBranchData() {}
 
-  FuncBranchData(StringRef Name, ContainerTy Data)
-      : Name(Name), Data(std::move(Data)) {}
-
-  FuncBranchData(StringRef Name, ContainerTy Data, ContainerTy EntryData)
+  FuncBranchData(StringRef Name, ContainerTy Data = ContainerTy(),
+                 ContainerTy EntryData = ContainerTy())
       : Name(Name), Data(std::move(Data)), EntryData(std::move(EntryData)) {}
 
   ErrorOr<const BranchInfo &> getBranch(uint64_t From, uint64_t To) const;
@@ -205,7 +203,7 @@ struct FuncMemData {
 
   FuncMemData() {}
 
-  FuncMemData(StringRef Name, ContainerTy Data)
+  FuncMemData(StringRef Name, ContainerTy Data = ContainerTy())
       : Name(Name), Data(std::move(Data)) {}
 };
 
@@ -241,7 +239,7 @@ struct FuncBasicSampleData {
   StringRef Name;
   ContainerTy Data;
 
-  FuncBasicSampleData(StringRef Name, ContainerTy Data)
+  FuncBasicSampleData(StringRef Name, ContainerTy Data = ContainerTy())
       : Name(Name), Data(std::move(Data)) {}
 
   /// Get the number of samples recorded in [Start, End)

bolt/include/bolt/Profile/Heatmap.h

@@ -52,6 +52,9 @@ class Heatmap {
       : BucketSize(BucketSize), MinAddress(MinAddress), MaxAddress(MaxAddress),
         TextSections(TextSections) {}
 
+  uint64_t HotStart{0};
+  uint64_t HotEnd{0};
+
   inline bool ignoreAddress(uint64_t Address) const {
     return (Address > MaxAddress) || (Address < MinAddress);
   }

bolt/include/bolt/Utils/CommandLineOpts.h

@@ -17,7 +17,13 @@
 
 namespace opts {
 
-extern bool HeatmapMode;
+enum HeatmapModeKind {
+  HM_None = 0,
+  HM_Exclusive, // llvm-bolt-heatmap
+  HM_Optional   // perf2bolt --heatmap
+};
+
+extern HeatmapModeKind HeatmapMode;
 extern bool BinaryAnalysisMode;
 
 extern llvm::cl::OptionCategory BoltCategory;
@@ -45,6 +51,7 @@ extern llvm::cl::opt<unsigned> HeatmapBlock;
 extern llvm::cl::opt<unsigned long long> HeatmapMaxAddress;
 extern llvm::cl::opt<unsigned long long> HeatmapMinAddress;
 extern llvm::cl::opt<bool> HeatmapPrintMappings;
+extern llvm::cl::opt<std::string> HeatmapOutput;
 extern llvm::cl::opt<bool> HotData;
 extern llvm::cl::opt<bool> HotFunctionsAtEnd;
 extern llvm::cl::opt<bool> HotText;

bolt/lib/Core/BinaryFunction.cpp

@@ -66,7 +66,7 @@ extern cl::opt<bool> UpdateDebugSections;
 extern cl::opt<unsigned> Verbosity;
 
 extern bool BinaryAnalysisMode;
-extern bool HeatmapMode;
+extern HeatmapModeKind HeatmapMode;
 extern bool processAllFunctions();
 
 static cl::opt<bool> CheckEncoding(
@@ -3326,7 +3326,7 @@ void BinaryFunction::duplicateConstantIslands() {
 static std::string constructFilename(std::string Filename,
                                      std::string Annotation,
                                      std::string Suffix) {
-  std::replace(Filename.begin(), Filename.end(), '/', '-');
+  llvm::replace(Filename, '/', '-');
   if (!Annotation.empty())
     Annotation.insert(0, "-");
   if (Filename.size() + Annotation.size() + Suffix.size() > MAX_PATH) {

bolt/lib/Core/DIEBuilder.cpp

@@ -437,10 +437,10 @@ getUnitForOffset(DIEBuilder &Builder, DWARFContext &DWCtx,
       // This is a work around for XCode clang. There is a build error when we
       // pass DWCtx.compile_units() to llvm::upper_bound
       std::call_once(InitVectorFlag, initCUVector);
-      auto CUIter = std::upper_bound(CUOffsets.begin(), CUOffsets.end(), Offset,
-                                     [](uint64_t LHS, const DWARFUnit *RHS) {
-                                       return LHS < RHS->getNextUnitOffset();
-                                     });
+      auto CUIter = llvm::upper_bound(CUOffsets, Offset,
+                                      [](uint64_t LHS, const DWARFUnit *RHS) {
+                                        return LHS < RHS->getNextUnitOffset();
+                                      });
       CU = CUIter != CUOffsets.end() ? (*CUIter) : nullptr;
     }
     return CU;

bolt/lib/Core/DebugData.cpp

@@ -676,7 +676,6 @@ static void writeDWARF5LocList(uint32_t &NumberOfEntries, DIEValue &AttrInfo,
     return;
   }
 
-  std::vector<uint64_t> OffsetsArray;
   auto writeExpression = [&](uint32_t Index) -> void {
     const DebugLocationEntry &Entry = LocList[Index];
     encodeULEB128(Entry.Expr.size(), LocBodyStream);

bolt/lib/Passes/AsmDump.cpp

@@ -109,7 +109,7 @@ void dumpFunction(const BinaryFunction &BF) {
   }
 
   std::string PrintName = BF.getPrintName();
-  std::replace(PrintName.begin(), PrintName.end(), '/', '-');
+  llvm::replace(PrintName, '/', '-');
   std::string Filename =
       opts::AsmDump.empty()
           ? (PrintName + ".s")

bolt/lib/Passes/BinaryPasses.cpp

@@ -35,7 +35,7 @@ static const char *dynoStatsOptName(const bolt::DynoStats::Category C) {
 
   OptNames[C] = bolt::DynoStats::Description(C);
 
-  std::replace(OptNames[C].begin(), OptNames[C].end(), ' ', '-');
+  llvm::replace(OptNames[C], ' ', '-');
 
   return OptNames[C].c_str();
 }

bolt/lib/Passes/FrameAnalysis.cpp

@@ -320,7 +320,6 @@ bool FrameAnalysis::updateArgsTouchedFor(const BinaryFunction &BF, MCInst &Inst,
   if (!BC.MIB->isCall(Inst))
     return false;
 
-  std::set<int64_t> Res;
   const MCSymbol *TargetSymbol = BC.MIB->getTargetSymbol(Inst);
   // If indirect call, we conservatively assume it accesses all stack positions
   if (TargetSymbol == nullptr) {

bolt/lib/Passes/HFSort.cpp

@@ -239,7 +239,6 @@ std::vector<Cluster> clusterize(const CallGraph &Cg) {
 }
 
 std::vector<Cluster> randomClusters(const CallGraph &Cg) {
-  std::vector<NodeId> FuncIds(Cg.numNodes(), 0);
   std::vector<Cluster> Clusters;
   Clusters.reserve(Cg.numNodes());
 

bolt/lib/Passes/PettisAndHansen.cpp

@@ -143,11 +143,10 @@ std::vector<Cluster> pettisAndHansen(const CallGraph &Cg) {
   // Find an arc with max weight and merge its nodes
 
   while (!Carcs.empty()) {
-    auto Maxpos =
-        std::max_element(Carcs.begin(), Carcs.end(),
-                         [&](const ClusterArc &Carc1, const ClusterArc &Carc2) {
-                           return Carc1.Weight < Carc2.Weight;
-                         });
+    auto Maxpos = llvm::max_element(
+        Carcs, [&](const ClusterArc &Carc1, const ClusterArc &Carc2) {
+          return Carc1.Weight < Carc2.Weight;
+        });
 
     ClusterArc Max = *Maxpos;
     Carcs.erase(Maxpos);

bolt/lib/Passes/ShrinkWrapping.cpp

@@ -825,14 +825,13 @@ void ShrinkWrapping::computeSaveLocations() {
     if (!CSA.CalleeSaved[I])
       continue;
 
-    std::stable_sort(BestSavePos[I].begin(), BestSavePos[I].end(),
-                     [&](const MCInst *A, const MCInst *B) {
-                       const BinaryBasicBlock *BBA = InsnToBB[A];
-                       const BinaryBasicBlock *BBB = InsnToBB[B];
-                       const uint64_t CountA = BBA->getKnownExecutionCount();
-                       const uint64_t CountB = BBB->getKnownExecutionCount();
-                       return CountB < CountA;
-                     });
+    llvm::stable_sort(BestSavePos[I], [&](const MCInst *A, const MCInst *B) {
+      const BinaryBasicBlock *BBA = InsnToBB[A];
+      const BinaryBasicBlock *BBB = InsnToBB[B];
+      const uint64_t CountA = BBA->getKnownExecutionCount();
+      const uint64_t CountB = BBB->getKnownExecutionCount();
+      return CountB < CountA;
+    });
 
     for (MCInst *Pos : BestSavePos[I]) {
       const BinaryBasicBlock *BB = InsnToBB[Pos];

bolt/lib/Profile/DataAggregator.cpp

@@ -164,6 +164,10 @@ void DataAggregator::findPerfExecutable() {
 void DataAggregator::start() {
   outs() << "PERF2BOLT: Starting data aggregation job for " << Filename << "\n";
 
+  // Turn on heatmap building if requested by --heatmap flag.
+  if (!opts::HeatmapMode && opts::HeatmapOutput.getNumOccurrences())
+    opts::HeatmapMode = opts::HeatmapModeKind::HM_Optional;
+
   // Don't launch perf for pre-aggregated files or when perf input is specified
   // by the user.
   if (opts::ReadPreAggregated || !opts::ReadPerfEvents.empty())
@@ -502,24 +506,25 @@ Error DataAggregator::preprocessProfile(BinaryContext &BC) {
     errs() << "PERF2BOLT: failed to parse samples\n";
 
   // Special handling for memory events
-  if (prepareToParse("mem events", MemEventsPPI, MemEventsErrorCallback))
-    return Error::success();
-
-  if (const std::error_code EC = parseMemEvents())
-    errs() << "PERF2BOLT: failed to parse memory events: " << EC.message()
-           << '\n';
+  if (!prepareToParse("mem events", MemEventsPPI, MemEventsErrorCallback))
+    if (const std::error_code EC = parseMemEvents())
+      errs() << "PERF2BOLT: failed to parse memory events: " << EC.message()
+             << '\n';
 
   deleteTempFiles();
 
 heatmap:
-  if (opts::HeatmapMode) {
-    if (std::error_code EC = printLBRHeatMap()) {
-      errs() << "ERROR: failed to print heat map: " << EC.message() << '\n';
-      exit(1);
-    }
-    exit(0);
-  }
-  return Error::success();
+  if (!opts::HeatmapMode)
+    return Error::success();
+
+  if (std::error_code EC = printLBRHeatMap())
+    return errorCodeToError(EC);
+
+  if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Optional)
+    return Error::success();
+
+  assert(opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive);
+  exit(0);
 }
 
 Error DataAggregator::readProfile(BinaryContext &BC) {
@@ -635,8 +640,6 @@ bool DataAggregator::doBasicSample(BinaryFunction &OrigFunc, uint64_t Address,
 
   BinaryFunction *ParentFunc = getBATParentFunction(OrigFunc);
   BinaryFunction &Func = ParentFunc ? *ParentFunc : OrigFunc;
-  if (ParentFunc || (BAT && !BAT->isBATFunction(Func.getAddress())))
-    NumColdSamples += Count;
   // Attach executed bytes to parent function in case of cold fragment.
   Func.SampleCountInBytes += Count * BlockSize;
 
@@ -740,15 +743,10 @@ bool DataAggregator::doBranch(uint64_t From, uint64_t To, uint64_t Count,
     if (BAT)
       Addr = BAT->translate(Func->getAddress(), Addr, IsFrom);
 
-    BinaryFunction *ParentFunc = getBATParentFunction(*Func);
-    if (IsFrom &&
-        (ParentFunc || (BAT && !BAT->isBATFunction(Func->getAddress()))))
-      NumColdSamples += Count;
-
-    if (!ParentFunc)
-      return std::pair{Func, IsRet};
+    if (BinaryFunction *ParentFunc = getBATParentFunction(*Func))
+      Func = ParentFunc;
 
-    return std::pair{ParentFunc, IsRet};
+    return std::pair{Func, IsRet};
   };
 
   auto [FromFunc, IsReturn] = handleAddress(From, /*IsFrom*/ true);
@@ -1318,6 +1316,14 @@ std::error_code DataAggregator::printLBRHeatMap() {
   }
   Heatmap HM(opts::HeatmapBlock, opts::HeatmapMinAddress,
              opts::HeatmapMaxAddress, getTextSections(BC));
+  auto getSymbolValue = [&](const MCSymbol *Symbol) -> uint64_t {
+    if (Symbol)
+      if (ErrorOr<uint64_t> SymValue = BC->getSymbolValue(*Symbol))
+        return SymValue.get();
+    return 0;
+  };
+  HM.HotStart = getSymbolValue(BC->getHotTextStartSymbol());
+  HM.HotEnd = getSymbolValue(BC->getHotTextEndSymbol());
 
   if (!NumTotalSamples) {
     if (opts::BasicAggregation) {
@@ -1351,15 +1357,14 @@ std::error_code DataAggregator::printLBRHeatMap() {
     exit(1);
   }
 
-  HM.print(opts::OutputFilename);
-  if (opts::OutputFilename == "-")
-    HM.printCDF(opts::OutputFilename);
-  else
-    HM.printCDF(opts::OutputFilename + ".csv");
-  if (opts::OutputFilename == "-")
-    HM.printSectionHotness(opts::OutputFilename);
-  else
-    HM.printSectionHotness(opts::OutputFilename + "-section-hotness.csv");
+  HM.print(opts::HeatmapOutput);
+  if (opts::HeatmapOutput == "-") {
+    HM.printCDF(opts::HeatmapOutput);
+    HM.printSectionHotness(opts::HeatmapOutput);
+  } else {
+    HM.printCDF(opts::HeatmapOutput + ".csv");
+    HM.printSectionHotness(opts::HeatmapOutput + "-section-hotness.csv");
+  }
 
   return std::error_code();
 }
@@ -1386,7 +1391,7 @@ void DataAggregator::parseLBRSample(const PerfBranchSample &Sample,
       const uint64_t TraceTo = NextLBR->From;
       const BinaryFunction *TraceBF =
           getBinaryFunctionContainingAddress(TraceFrom);
-      if (opts::HeatmapMode) {
+      if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive) {
         FTInfo &Info = FallthroughLBRs[Trace(TraceFrom, TraceTo)];
         ++Info.InternCount;
       } else if (TraceBF && TraceBF->containsAddress(TraceTo)) {
@@ -1424,7 +1429,7 @@ void DataAggregator::parseLBRSample(const PerfBranchSample &Sample,
     NextLBR = &LBR;
 
     // Record branches outside binary functions for heatmap.
-    if (opts::HeatmapMode) {
+    if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive) {
       TakenBranchInfo &Info = BranchLBRs[Trace(LBR.From, LBR.To)];
       ++Info.TakenCount;
       continue;
@@ -1439,26 +1444,13 @@ void DataAggregator::parseLBRSample(const PerfBranchSample &Sample,
   }
   // Record LBR addresses not covered by fallthroughs (bottom-of-stack source
   // and top-of-stack target) as basic samples for heatmap.
-  if (opts::HeatmapMode && !Sample.LBR.empty()) {
+  if (opts::HeatmapMode == opts::HeatmapModeKind::HM_Exclusive &&
+      !Sample.LBR.empty()) {
     ++BasicSamples[Sample.LBR.front().To];
     ++BasicSamples[Sample.LBR.back().From];
   }
 }
 
-void DataAggregator::printColdSamplesDiagnostic() const {
-  if (NumColdSamples > 0) {
-    const float ColdSamples = NumColdSamples * 100.0f / NumTotalSamples;
-    outs() << "PERF2BOLT: " << NumColdSamples
-           << format(" (%.1f%%)", ColdSamples)
-           << " samples recorded in cold regions of split functions.\n";
-    if (ColdSamples > 5.0f)
-      outs()
-          << "WARNING: The BOLT-processed binary where samples were collected "
-             "likely used bad data or your service observed a large shift in "
-             "profile. You may want to audit this\n";
-  }
-}
-
 void DataAggregator::printLongRangeTracesDiagnostic() const {
   outs() << "PERF2BOLT: out of range traces involving unknown regions: "
          << NumLongRangeTraces;
@@ -1499,7 +1491,6 @@ void DataAggregator::printBranchSamplesDiagnostics() const {
               "collection. The generated data may be ineffective for improving "
               "performance\n\n";
   printLongRangeTracesDiagnostic();
-  printColdSamplesDiagnostic();
 }
 
 void DataAggregator::printBasicSamplesDiagnostics(
@@ -1511,7 +1502,6 @@ void DataAggregator::printBasicSamplesDiagnostics(
               "binary is probably not the same binary used during profiling "
               "collection. The generated data may be ineffective for improving "
               "performance\n\n";
-  printColdSamplesDiagnostic();
 }
 
 void DataAggregator::printBranchStacksDiagnostics(