Reduce CHECKs

Created using spr 1.3.6-beta.1

Author: wangpc-pp

.github/workflows/containers/github-action-ci/stage1.Dockerfile

@@ -2,7 +2,7 @@ FROM docker.io/library/ubuntu:22.04 as base
 ENV LLVM_SYSROOT=/opt/llvm
 
 FROM base as stage1-toolchain
-ENV LLVM_VERSION=18.1.8
+ENV LLVM_VERSION=19.1.2
 
 RUN apt-get update && \
     apt-get install -y \

bolt/include/bolt/Core/BinaryFunction.h

@@ -386,6 +386,9 @@ class BinaryFunction {
   /// Raw branch count for this function in the profile.
   uint64_t RawBranchCount{0};
 
+  /// Dynamically executed function bytes, used for density computation.
+  uint64_t SampleCountInBytes{0};
+
   /// Indicates the type of profile the function is using.
   uint16_t ProfileFlags{PF_NONE};
 
@@ -1844,6 +1847,9 @@ class BinaryFunction {
   /// to this function.
   void setRawBranchCount(uint64_t Count) { RawBranchCount = Count; }
 
+  /// Return the number of dynamically executed bytes, from raw perf data.
+  uint64_t getSampleCountInBytes() const { return SampleCountInBytes; }
+
   /// Return the execution count for functions with known profile.
   /// Return 0 if the function has no profile.
   uint64_t getKnownExecutionCount() const {

bolt/include/bolt/Core/DIEBuilder.h

@@ -314,7 +314,7 @@ class DIEBuilder {
 
     BC.errs()
         << "BOLT-ERROR: unable to find TypeUnit for Type Unit at offset 0x"
-        << DU.getOffset() << "\n";
+        << Twine::utohexstr(DU.getOffset()) << "\n";
     return nullptr;
   }
 

bolt/include/bolt/Profile/BoltAddressTranslation.h

@@ -141,15 +141,13 @@ class BoltAddressTranslation {
                          uint64_t FuncOutputAddress) const;
 
   /// Write the serialized address translation table for a function.
-  template <bool Cold>
-  void writeMaps(std::map<uint64_t, MapTy> &Maps, uint64_t &PrevAddress,
-                 raw_ostream &OS);
+  template <bool Cold> void writeMaps(uint64_t &PrevAddress, raw_ostream &OS);
 
   /// Read the serialized address translation table for a function.
   /// Return a parse error if failed.
   template <bool Cold>
-  void parseMaps(std::vector<uint64_t> &HotFuncs, uint64_t &PrevAddress,
-                 DataExtractor &DE, uint64_t &Offset, Error &Err);
+  void parseMaps(uint64_t &PrevAddress, DataExtractor &DE, uint64_t &Offset,
+                 Error &Err);
 
   /// Returns the bitmask with set bits corresponding to indices of BRANCHENTRY
   /// entries in function address translation map.
@@ -161,6 +159,9 @@ class BoltAddressTranslation {
 
   std::map<uint64_t, MapTy> Maps;
 
+  /// Ordered vector with addresses of hot functions.
+  std::vector<uint64_t> HotFuncs;
+
   /// Map a function to its basic blocks count
   std::unordered_map<uint64_t, size_t> NumBasicBlocksMap;
 

bolt/include/bolt/Utils/CommandLineOpts.h

@@ -55,6 +55,7 @@ extern llvm::cl::opt<bool> PrintSections;
 enum ProfileFormatKind { PF_Fdata, PF_YAML };
 
 extern llvm::cl::opt<ProfileFormatKind> ProfileFormat;
+extern llvm::cl::opt<bool> ShowDensity;
 extern llvm::cl::opt<bool> SplitEH;
 extern llvm::cl::opt<bool> StrictMode;
 extern llvm::cl::opt<bool> TimeOpts;

bolt/lib/Core/BinaryContext.cpp

@@ -1294,8 +1294,8 @@ bool BinaryContext::handleAArch64Veneer(uint64_t Address, bool MatchOnly) {
     Veneer->getOrCreateLocalLabel(Address);
     Veneer->setMaxSize(TotalSize);
     Veneer->updateState(BinaryFunction::State::Disassembled);
-    LLVM_DEBUG(dbgs() << "BOLT-DEBUG: handling veneer function at 0x" << Address
-                      << "\n");
+    LLVM_DEBUG(dbgs() << "BOLT-DEBUG: handling veneer function at 0x"
+                      << Twine::utohexstr(Address) << "\n");
     return true;
   };
 

bolt/lib/Core/BinaryFunction.cpp

@@ -2577,6 +2577,7 @@ struct CFISnapshot {
     case MCCFIInstruction::OpAdjustCfaOffset:
     case MCCFIInstruction::OpWindowSave:
     case MCCFIInstruction::OpNegateRAState:
+    case MCCFIInstruction::OpNegateRAStateWithPC:
     case MCCFIInstruction::OpLLVMDefAspaceCfa:
     case MCCFIInstruction::OpLabel:
       llvm_unreachable("unsupported CFI opcode");
@@ -2715,6 +2716,7 @@ struct CFISnapshotDiff : public CFISnapshot {
     case MCCFIInstruction::OpAdjustCfaOffset:
     case MCCFIInstruction::OpWindowSave:
     case MCCFIInstruction::OpNegateRAState:
+    case MCCFIInstruction::OpNegateRAStateWithPC:
     case MCCFIInstruction::OpLLVMDefAspaceCfa:
     case MCCFIInstruction::OpLabel:
       llvm_unreachable("unsupported CFI opcode");
@@ -2864,6 +2866,7 @@ BinaryFunction::unwindCFIState(int32_t FromState, int32_t ToState,
     case MCCFIInstruction::OpAdjustCfaOffset:
     case MCCFIInstruction::OpWindowSave:
     case MCCFIInstruction::OpNegateRAState:
+    case MCCFIInstruction::OpNegateRAStateWithPC:
     case MCCFIInstruction::OpLLVMDefAspaceCfa:
     case MCCFIInstruction::OpLabel:
       llvm_unreachable("unsupported CFI opcode");
@@ -3684,9 +3687,8 @@ BinaryFunction::BasicBlockListType BinaryFunction::dfs() const {
     BinaryBasicBlock *BB = Stack.top();
     Stack.pop();
 
-    if (Visited.find(BB) != Visited.end())
+    if (!Visited.insert(BB).second)
       continue;
-    Visited.insert(BB);
     DFS.push_back(BB);
 
     for (BinaryBasicBlock *SuccBB : BB->landing_pads()) {
@@ -3879,11 +3881,8 @@ void BinaryFunction::disambiguateJumpTables(
       JumpTable *JT = getJumpTable(Inst);
       if (!JT)
         continue;
-      auto Iter = JumpTables.find(JT);
-      if (Iter == JumpTables.end()) {
-        JumpTables.insert(JT);
+      if (JumpTables.insert(JT).second)
         continue;
-      }
       // This instruction is an indirect jump using a jump table, but it is
       // using the same jump table of another jump. Try all our tricks to
       // extract the jump table symbol and make it point to a new, duplicated JT

bolt/lib/Core/DIEBuilder.cpp

@@ -57,11 +57,9 @@ getDWOName(llvm::DWARFUnit &CU,
          "DW_AT_dwo_name/DW_AT_GNU_dwo_name does not exist.");
   if (DwarfOutputPath) {
     DWOName = std::string(sys::path::filename(DWOName));
-    auto Iter = NameToIndexMap.find(DWOName);
-    if (Iter == NameToIndexMap.end())
-      Iter = NameToIndexMap.insert({DWOName, 0}).first;
-    DWOName.append(std::to_string(Iter->second));
-    ++Iter->second;
+    uint32_t &Index = NameToIndexMap[DWOName];
+    DWOName.append(std::to_string(Index));
+    ++Index;
   }
   DWOName.append(".dwo");
   return DWOName;
@@ -283,8 +281,7 @@ void DIEBuilder::buildTypeUnits(DebugStrOffsetsWriter *StrOffsetWriter,
     for (auto &Row : TUIndex.getRows()) {
       uint64_t Signature = Row.getSignature();
       // manually populate TypeUnit to UnitVector
-      DwarfContext->getTypeUnitForHash(DwarfContext->getMaxVersion(), Signature,
-                                       true);
+      DwarfContext->getTypeUnitForHash(Signature, true);
     }
   }
   const unsigned int CUNum = getCUNum(DwarfContext, isDWO());

bolt/lib/Passes/BinaryPasses.cpp

@@ -15,6 +15,7 @@
 #include "bolt/Core/ParallelUtilities.h"
 #include "bolt/Passes/ReorderAlgorithm.h"
 #include "bolt/Passes/ReorderFunctions.h"
+#include "bolt/Utils/CommandLineOpts.h"
 #include "llvm/Support/CommandLine.h"
 #include <atomic>
 #include <mutex>
@@ -223,6 +224,18 @@ static cl::opt<unsigned> TopCalledLimit(
              "functions section"),
     cl::init(100), cl::Hidden, cl::cat(BoltCategory));
 
+// Profile density options, synced with llvm-profgen/ProfileGenerator.cpp
+static cl::opt<int> ProfileDensityCutOffHot(
+    "profile-density-cutoff-hot", cl::init(990000),
+    cl::desc("Total samples cutoff for functions used to calculate "
+             "profile density."));
+
+static cl::opt<double> ProfileDensityThreshold(
+    "profile-density-threshold", cl::init(60),
+    cl::desc("If the profile density is below the given threshold, it "
+             "will be suggested to increase the sampling rate."),
+    cl::Optional);
+
 } // namespace opts
 
 namespace llvm {
@@ -1383,6 +1396,7 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
   uint64_t StaleSampleCount = 0;
   uint64_t InferredSampleCount = 0;
   std::vector<const BinaryFunction *> ProfiledFunctions;
+  std::vector<std::pair<double, uint64_t>> FuncDensityList;
   const char *StaleFuncsHeader = "BOLT-INFO: Functions with stale profile:\n";
   for (auto &BFI : BC.getBinaryFunctions()) {
     const BinaryFunction &Function = BFI.second;
@@ -1441,6 +1455,22 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
       StaleSampleCount += SampleCount;
       ++NumAllStaleFunctions;
     }
+
+    if (opts::ShowDensity) {
+      uint64_t Size = Function.getSize();
+      // In case of BOLT split functions registered in BAT, executed traces are
+      // automatically attributed to the main fragment. Add up function sizes
+      // for all fragments.
+      if (IsHotParentOfBOLTSplitFunction)
+        for (const BinaryFunction *Fragment : Function.getFragments())
+          Size += Fragment->getSize();
+      double Density = (double)1.0 * Function.getSampleCountInBytes() / Size;
+      FuncDensityList.emplace_back(Density, SampleCount);
+      LLVM_DEBUG(BC.outs() << Function << ": executed bytes "
+                           << Function.getSampleCountInBytes() << ", size (b) "
+                           << Size << ", density " << Density
+                           << ", sample count " << SampleCount << '\n');
+    }
   }
   BC.NumProfiledFuncs = ProfiledFunctions.size();
   BC.NumStaleProfileFuncs = NumStaleProfileFunctions;
@@ -1684,6 +1714,50 @@ Error PrintProgramStats::runOnFunctions(BinaryContext &BC) {
       BC.outs() << ". Use -print-unknown to see the list.";
     BC.outs() << '\n';
   }
+
+  if (opts::ShowDensity) {
+    double Density = 0.0;
+    // Sorted by the density in descending order.
+    llvm::stable_sort(FuncDensityList,
+                      [&](const std::pair<double, uint64_t> &A,
+                          const std::pair<double, uint64_t> &B) {
+                        if (A.first != B.first)
+                          return A.first > B.first;
+                        return A.second < B.second;
+                      });
+
+    uint64_t AccumulatedSamples = 0;
+    uint32_t I = 0;
+    assert(opts::ProfileDensityCutOffHot <= 1000000 &&
+           "The cutoff value is greater than 1000000(100%)");
+    while (AccumulatedSamples <
+               TotalSampleCount *
+                   static_cast<float>(opts::ProfileDensityCutOffHot) /
+                   1000000 &&
+           I < FuncDensityList.size()) {
+      AccumulatedSamples += FuncDensityList[I].second;
+      Density = FuncDensityList[I].first;
+      I++;
+    }
+    if (Density == 0.0) {
+      BC.errs() << "BOLT-WARNING: the output profile is empty or the "
+                   "--profile-density-cutoff-hot option is "
+                   "set too low. Please check your command.\n";
+    } else if (Density < opts::ProfileDensityThreshold) {
+      BC.errs()
+          << "BOLT-WARNING: BOLT is estimated to optimize better with "
+          << format("%.1f", opts::ProfileDensityThreshold / Density)
+          << "x more samples. Please consider increasing sampling rate or "
+             "profiling for longer duration to get more samples.\n";
+    }
+
+    BC.outs() << "BOLT-INFO: Functions with density >= "
+              << format("%.1f", Density) << " account for "
+              << format("%.2f",
+                        static_cast<double>(opts::ProfileDensityCutOffHot) /
+                            10000)
+              << "% total sample counts.\n";
+  }
   return Error::success();
 }
 

bolt/lib/Passes/Instrumentation.cpp

@@ -109,9 +109,8 @@ static bool hasAArch64ExclusiveMemop(
     BinaryBasicBlock *BB = BBQueue.front().first;
     bool IsLoad = BBQueue.front().second;
     BBQueue.pop();
-    if (Visited.find(BB) != Visited.end())
+    if (!Visited.insert(BB).second)
       continue;
-    Visited.insert(BB);
 
     for (const MCInst &Inst : *BB) {
       // Two loads one after another - skip whole function
@@ -126,8 +125,7 @@ static bool hasAArch64ExclusiveMemop(
       if (BC.MIB->isAArch64ExclusiveLoad(Inst))
         IsLoad = true;
 
-      if (IsLoad && BBToSkip.find(BB) == BBToSkip.end()) {
-        BBToSkip.insert(BB);
+      if (IsLoad && BBToSkip.insert(BB).second) {
         if (opts::Verbosity >= 2) {
           outs() << "BOLT-INSTRUMENTER: skip BB " << BB->getName()
                  << " due to exclusive instruction in function "