Merge remote-tracking branch 'origin/main' into laa-nonconst-dist-forward

Author: fhahn

.github/workflows/pr-code-format.yml

@@ -1,4 +1,8 @@
 name: "Check code formatting"
+
+permissions:
+  contents: read
+
 on:
   pull_request:
     branches:

bolt/include/bolt/Core/BinaryFunction.h

@@ -1402,7 +1402,8 @@ class BinaryFunction {
 
   /// Return true if the function has CFI instructions
   bool hasCFI() const {
-    return !FrameInstructions.empty() || !CIEFrameInstructions.empty();
+    return !FrameInstructions.empty() || !CIEFrameInstructions.empty() ||
+           IsInjected;
   }
 
   /// Return unique number associated with the function.

bolt/include/bolt/Profile/BoltAddressTranslation.h

@@ -119,11 +119,6 @@ class BoltAddressTranslation {
   /// True if a given \p Address is a function with translation table entry.
   bool isBATFunction(uint64_t Address) const { return Maps.count(Address); }
 
-  /// Returns branch offsets grouped by containing basic block in a given
-  /// function.
-  std::unordered_map<uint32_t, std::vector<uint32_t>>
-  getBFBranches(uint64_t FuncOutputAddress) const;
-
   /// For a given \p Symbol in the output binary and known \p InputOffset
   /// return a corresponding pair of parent BinaryFunction and secondary entry
   /// point in it.
@@ -193,7 +188,7 @@ class BoltAddressTranslation {
       EntryTy(unsigned Index, size_t Hash) : Index(Index), Hash(Hash) {}
     };
 
-    std::unordered_map<uint32_t, EntryTy> Map;
+    std::map<uint32_t, EntryTy> Map;
     const EntryTy &getEntry(uint32_t BBInputOffset) const {
       auto It = Map.find(BBInputOffset);
       assert(It != Map.end());
@@ -218,6 +213,10 @@ class BoltAddressTranslation {
     }
 
     size_t getNumBasicBlocks() const { return Map.size(); }
+
+    auto begin() const { return Map.begin(); }
+    auto end() const { return Map.end(); }
+    auto upper_bound(uint32_t Offset) const { return Map.upper_bound(Offset); }
   };
 
   /// Map function output address to its hash and basic blocks hash map.

bolt/include/bolt/Profile/DataAggregator.h

@@ -225,6 +225,10 @@ class DataAggregator : public DataReader {
   /// Aggregation statistics
   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};
 
   /// Looks into system PATH for Linux Perf and set up the aggregator to use it
@@ -245,14 +249,12 @@ class DataAggregator : public DataReader {
   /// disassembled BinaryFunctions
   BinaryFunction *getBinaryFunctionContainingAddress(uint64_t Address) const;
 
+  /// Perform BAT translation for a given \p Func and return the parent
+  /// BinaryFunction or nullptr.
+  BinaryFunction *getBATParentFunction(const BinaryFunction &Func) const;
+
   /// Retrieve the location name to be used for samples recorded in \p Func.
-  /// If doing BAT translation, link cold parts to the hot part  names (used by
-  /// the original binary).  \p Count specifies how many samples were recorded
-  /// at that location, so we can tally total activity in cold areas if we are
-  /// dealing with profiling data collected in a bolted binary. For LBRs,
-  /// \p Count should only be used for the source of the branch to avoid
-  /// counting cold activity twice (one for source and another for destination).
-  StringRef getLocationName(BinaryFunction &Func, uint64_t Count);
+  StringRef getLocationName(const BinaryFunction &Func) const;
 
   /// Semantic actions - parser hooks to interpret parsed perf samples
   /// Register a sample (non-LBR mode), i.e. a new hit at \p Address
@@ -467,9 +469,6 @@ class DataAggregator : public DataReader {
   std::error_code writeBATYAML(BinaryContext &BC,
                                StringRef OutputFilename) const;
 
-  /// Fixup profile collected on BOLTed binary, namely handle split functions.
-  void fixupBATProfile(BinaryContext &BC);
-
   /// Filter out binaries based on PID
   void filterBinaryMMapInfo();
 

bolt/lib/Core/BinaryContext.cpp

@@ -555,6 +555,9 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
                                      const uint64_t NextJTAddress,
                                      JumpTable::AddressesType *EntriesAsAddress,
                                      bool *HasEntryInFragment) const {
+  // Target address of __builtin_unreachable.
+  const uint64_t UnreachableAddress = BF.getAddress() + BF.getSize();
+
   // Is one of the targets __builtin_unreachable?
   bool HasUnreachable = false;
 
@@ -564,9 +567,15 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
   // Number of targets other than __builtin_unreachable.
   uint64_t NumRealEntries = 0;
 
-  auto addEntryAddress = [&](uint64_t EntryAddress) {
-    if (EntriesAsAddress)
-      EntriesAsAddress->emplace_back(EntryAddress);
+  // Size of the jump table without trailing __builtin_unreachable entries.
+  size_t TrimmedSize = 0;
+
+  auto addEntryAddress = [&](uint64_t EntryAddress, bool Unreachable = false) {
+    if (!EntriesAsAddress)
+      return;
+    EntriesAsAddress->emplace_back(EntryAddress);
+    if (!Unreachable)
+      TrimmedSize = EntriesAsAddress->size();
   };
 
   ErrorOr<const BinarySection &> Section = getSectionForAddress(Address);
@@ -618,8 +627,8 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
             : *getPointerAtAddress(EntryAddress);
 
     // __builtin_unreachable() case.
-    if (Value == BF.getAddress() + BF.getSize()) {
-      addEntryAddress(Value);
+    if (Value == UnreachableAddress) {
+      addEntryAddress(Value, /*Unreachable*/ true);
       HasUnreachable = true;
       LLVM_DEBUG(dbgs() << formatv("OK: {0:x} __builtin_unreachable\n", Value));
       continue;
@@ -673,6 +682,13 @@ bool BinaryContext::analyzeJumpTable(const uint64_t Address,
     addEntryAddress(Value);
   }
 
+  // Trim direct/normal jump table to exclude trailing unreachable entries that
+  // can collide with a function address.
+  if (Type == JumpTable::JTT_NORMAL && EntriesAsAddress &&
+      TrimmedSize != EntriesAsAddress->size() &&
+      getBinaryFunctionAtAddress(UnreachableAddress))
+    EntriesAsAddress->resize(TrimmedSize);
+
   // It's a jump table if the number of real entries is more than 1, or there's
   // one real entry and one or more special targets. If there are only multiple
   // special targets, then it's not a jump table.

bolt/lib/Profile/BoltAddressTranslation.cpp

@@ -582,26 +582,6 @@ void BoltAddressTranslation::saveMetadata(BinaryContext &BC) {
   }
 }
 
-std::unordered_map<uint32_t, std::vector<uint32_t>>
-BoltAddressTranslation::getBFBranches(uint64_t OutputAddress) const {
-  std::unordered_map<uint32_t, std::vector<uint32_t>> Branches;
-  auto FuncIt = Maps.find(OutputAddress);
-  assert(FuncIt != Maps.end());
-  std::vector<uint32_t> InputOffsets;
-  for (const auto &KV : FuncIt->second)
-    InputOffsets.emplace_back(KV.second);
-  // Sort with LSB BRANCHENTRY bit.
-  llvm::sort(InputOffsets);
-  uint32_t BBOffset{0};
-  for (uint32_t InOffset : InputOffsets) {
-    if (InOffset & BRANCHENTRY)
-      Branches[BBOffset].push_back(InOffset >> 1);
-    else
-      BBOffset = InOffset >> 1;
-  }
-  return Branches;
-}
-
 unsigned
 BoltAddressTranslation::getSecondaryEntryPointId(uint64_t Address,
                                                  uint32_t Offset) const {