Preserve CFG until BAT, use it to check call cont landing pads, encode them in secondary entry points table

Author: aaupov

bolt/docs/BAT.md

@@ -115,21 +115,13 @@ Deleted basic blocks are emitted as having `OutputOffset` equal to the size of
 the function. They don't affect address translation and only participate in
 input basic block mapping.
 
-### Secondary Entry Points table
+### Secondary Entry Points and Call Continuation Landing Pads table
 The table is emitted for hot fragments only. It contains `NumSecEntryPoints`
-offsets denoting secondary entry points, delta encoded, implicitly starting at zero.
+offsets, delta encoded, implicitly starting at zero.
 | Entry | Encoding | Description |
 | ----- | -------- | ----------- |
-| `SecEntryPoint` | Delta, ULEB128 | Secondary entry point offset |
+| `OutputOffset` | Delta, ULEB128 | An offset of secondary entry point or a call continuation landing pad\*|
 
-### Call continuation landing pads table
-This table contains the addresses of call continuation blocks that are also
-landing pads, to aid pre-aggregated profile conversion. The table is optional
-for backwards compatibility, but new versions of BOLT will always emit it.
-
-| Entry | Encoding | Description |
-| ----- | -------- | ----------- |
-| `NumEntries` | ULEB128 | Number of addresses |
-| `InputAddress` | Delta, ULEB128 | `NumEntries` input addresses of call continuation landing pad blocks |
-
-Addresses are delta encoded, implicitly starting at zero.
+Call continuation landing pads offsets are shifted by the size of the function
+for backwards compatibility (treated as entry points past the end of the
+function).

bolt/include/bolt/Profile/BoltAddressTranslation.h

@@ -143,20 +143,12 @@ class BoltAddressTranslation {
   /// Write the serialized address translation table for a function.
   template <bool Cold> void writeMaps(uint64_t &PrevAddress, raw_ostream &OS);
 
-  /// Write call continuation landing pad addresses.
-  void writeCallContLandingPads(raw_ostream &OS);
-
   /// Read the serialized address translation table for a function.
   /// Return a parse error if failed.
   template <bool Cold>
   void parseMaps(uint64_t &PrevAddress, DataExtractor &DE, uint64_t &Offset,
                  Error &Err);
 
-
-  /// Read the table with call continuation landing pad offsets.
-  void parseCallContLandingPads(DataExtractor &DE, uint64_t &Offset,
-                                Error &Err);
-
   /// Returns the bitmask with set bits corresponding to indices of BRANCHENTRY
   /// entries in function address translation map.
   APInt calculateBranchEntriesBitMask(MapTy &Map, size_t EqualElems) const;
@@ -176,14 +168,6 @@ class BoltAddressTranslation {
   /// Map a function to its secondary entry points vector
   std::unordered_map<uint64_t, std::vector<uint32_t>> SecondaryEntryPointsMap;
 
-  /// Vector with call continuation landing pads input addresses (pre-BOLT
-  /// binary).
-  std::vector<uint64_t> CallContLandingPadAddrs;
-
-  /// Return a secondary entry point ID for a function located at \p Address and
-  /// \p Offset within that function.
-  unsigned getSecondaryEntryPointId(uint64_t Address, uint32_t Offset) const;
-
   /// Links outlined cold bocks to their original function
   std::map<uint64_t, uint64_t> ColdPartSource;
 
@@ -195,13 +179,9 @@ class BoltAddressTranslation {
   const static uint32_t BRANCHENTRY = 0x1;
 
 public:
-  /// Returns whether a given \p Offset is a secondary entry point in function
-  /// with address \p Address.
-  bool isSecondaryEntry(uint64_t Address, uint32_t Offset) const;
-
-  /// Returns whether a given \p Offset is a call continuation landing pad in
-  /// function with address \p Address.
-  bool isCallContinuationLandingPad(uint64_t Address, uint32_t Offset) const;
+  /// Return a secondary entry point ID for a function located at \p Address and
+  /// \p Offset within that function.
+  unsigned getSecondaryEntryPointId(uint64_t Address, uint32_t Offset) const;
 
   /// Map basic block input offset to a basic block index and hash pair.
   class BBHashMapTy {

bolt/lib/Core/BinaryEmitter.cpp

@@ -257,7 +257,8 @@ void BinaryEmitter::emitFunctions() {
       Streamer.setAllowAutoPadding(OriginalAllowAutoPadding);
 
       if (Emitted)
-        Function->setEmitted(/*KeepCFG=*/opts::PrintCacheMetrics);
+        Function->setEmitted(/*KeepCFG=*/opts::PrintCacheMetrics ||
+                             opts::EnableBAT);
     }
   };
 

bolt/lib/Profile/BoltAddressTranslation.cpp

@@ -87,13 +87,28 @@ void BoltAddressTranslation::write(const BinaryContext &BC, raw_ostream &OS) {
       continue;
 
     uint32_t NumSecondaryEntryPoints = 0;
-    Function.forEachEntryPoint([&](uint64_t Offset, const MCSymbol *) {
-      if (!Offset)
-        return true;
+    for (const BinaryBasicBlock &BB : llvm::drop_begin(Function)) {
+      if (BB.isEntryPoint()) {
+        ++NumSecondaryEntryPoints;
+        SecondaryEntryPointsMap[OutputAddress].push_back(BB.getOffset());
+        continue;
+      }
+      // Add call continuation landing pads, offset by function size
+      if (!BB.isLandingPad())
+        continue;
+      const BinaryBasicBlock *PrevBB =
+          Function.getLayout().getBlock(BB.getIndex() - 1);
+      if (!PrevBB->isSuccessor(&BB))
+        continue;
+      const MCInst *Instr = PrevBB->getLastNonPseudoInstr();
+      if (!Instr || !BC.MIB->isCall(*Instr))
+        continue;
       ++NumSecondaryEntryPoints;
-      SecondaryEntryPointsMap[OutputAddress].push_back(Offset);
-      return true;
-    });
+      SecondaryEntryPointsMap[OutputAddress].push_back(
+          Function.getOutputSize() + BB.getOffset());
+    }
+    if (NumSecondaryEntryPoints)
+      llvm::sort(SecondaryEntryPointsMap[OutputAddress]);
 
     LLVM_DEBUG(dbgs() << "Function name: " << Function.getPrintName() << "\n");
     LLVM_DEBUG(dbgs() << " Address reference: 0x"
@@ -145,7 +160,6 @@ void BoltAddressTranslation::write(const BinaryContext &BC, raw_ostream &OS) {
   uint64_t PrevAddress = 0;
   writeMaps</*Cold=*/false>(PrevAddress, OS);
   writeMaps</*Cold=*/true>(PrevAddress, OS);
-  writeCallContLandingPads(OS);
 
   BC.outs() << "BOLT-INFO: Wrote " << Maps.size() << " BAT maps\n";
   BC.outs() << "BOLT-INFO: Wrote " << FuncHashes.getNumFunctions()
@@ -292,15 +306,6 @@ void BoltAddressTranslation::writeMaps(uint64_t &PrevAddress, raw_ostream &OS) {
   }
 }
 
-void BoltAddressTranslation::writeCallContLandingPads(raw_ostream &OS) {
-  encodeULEB128(CallContLandingPadAddrs.size(), OS);
-  uint64_t PrevAddress = 0;
-  for (const uint64_t Address : CallContLandingPadAddrs) {
-    encodeULEB128(Address - PrevAddress, OS);
-    PrevAddress = Address;
-  }
-}
-
 std::error_code BoltAddressTranslation::parse(raw_ostream &OS, StringRef Buf) {
   DataExtractor DE = DataExtractor(Buf, true, 8);
   uint64_t Offset = 0;
@@ -325,8 +330,6 @@ std::error_code BoltAddressTranslation::parse(raw_ostream &OS, StringRef Buf) {
   parseMaps</*Cold=*/false>(PrevAddress, DE, Offset, Err);
   parseMaps</*Cold=*/true>(PrevAddress, DE, Offset, Err);
   OS << "BOLT-INFO: Parsed " << Maps.size() << " BAT entries\n";
-  if (Offset < Buf.size())
-    parseCallContLandingPads(DE, Offset, Err);
   return errorToErrorCode(std::move(Err));
 }
 
@@ -446,21 +449,6 @@ void BoltAddressTranslation::parseMaps(uint64_t &PrevAddress, DataExtractor &DE,
   }
 }
 
-void BoltAddressTranslation::parseCallContLandingPads(DataExtractor &DE,
-                                                      uint64_t &Offset,
-                                                      Error &Err) {
-  const uint32_t NumEntries = DE.getULEB128(&Offset, &Err);
-  LLVM_DEBUG(dbgs() << "Parsing " << NumEntries
-                    << " call continuation landing pad entries\n");
-  CallContLandingPadAddrs.reserve(NumEntries);
-  uint64_t PrevAddress = 0;
-  for (uint32_t I = 0; I < NumEntries; ++I) {
-    const uint64_t Address = PrevAddress + DE.getULEB128(&Offset, &Err);
-    CallContLandingPadAddrs.emplace_back(Address);
-    PrevAddress = Address;
-  }
-}
-
 void BoltAddressTranslation::dump(raw_ostream &OS) const {
   const size_t NumTables = Maps.size();
   OS << "BAT tables for " << NumTables << " functions:\n";
@@ -609,25 +597,14 @@ void BoltAddressTranslation::saveMetadata(BinaryContext &BC) {
     // changed
     if (BF.isIgnored() || (!BC.HasRelocations && !BF.isSimple()))
       continue;
-    const uint64_t FuncAddress = BF.getAddress();
     // Prepare function and block hashes
-    FuncHashes.addEntry(FuncAddress, BF.computeHash());
+    FuncHashes.addEntry(BF.getAddress(), BF.computeHash());
     BF.computeBlockHashes();
-    BBHashMapTy &BBHashMap = getBBHashMap(FuncAddress);
+    BBHashMapTy &BBHashMap = getBBHashMap(BF.getAddress());
     // Set BF/BB metadata
-    for (const BinaryBasicBlock &BB : BF) {
-      const uint32_t BlockOffset = BB.getInputOffset();
-      BBHashMap.addEntry(BlockOffset, BB.getIndex(), BB.getHash());
-      // Set CallContLandingPads
-      if (!BB.isEntryPoint() && BB.isLandingPad()) {
-        const BinaryBasicBlock *PrevBB =
-            BF.getLayout().getBlock(BB.getIndex() - 1);
-        const MCInst *Instr = PrevBB->getLastNonPseudoInstr();
-        if (Instr && BC.MIB->isCall(*Instr))
-          CallContLandingPadAddrs.emplace_back(FuncAddress + BlockOffset);
-      }
-    }
-    NumBasicBlocksMap.emplace(FuncAddress, BF.size());
+    for (const BinaryBasicBlock &BB : BF)
+      BBHashMap.addEntry(BB.getInputOffset(), BB.getIndex(), BB.getHash());
+    NumBasicBlocksMap.emplace(BF.getAddress(), BF.size());
   }
 }
 
@@ -638,8 +615,8 @@ BoltAddressTranslation::getSecondaryEntryPointId(uint64_t Address,
   if (FunctionIt == SecondaryEntryPointsMap.end())
     return 0;
   const std::vector<uint32_t> &Offsets = FunctionIt->second;
-  auto OffsetIt = std::find(Offsets.begin(), Offsets.end(), Offset);
-  if (OffsetIt == Offsets.end())
+  auto OffsetIt = llvm::lower_bound(FunctionIt->second, Offset);
+  if (OffsetIt == Offsets.end() || *OffsetIt != Offset)
     return 0;
   // Adding one here because main entry point is not stored in BAT, and
   // enumeration for secondary entry points starts with 1.
@@ -675,30 +652,5 @@ BoltAddressTranslation::translateSymbol(const BinaryContext &BC,
   return std::pair(ParentBF, SecondaryEntryId);
 }
 
-bool BoltAddressTranslation::isSecondaryEntry(uint64_t OutputAddress,
-                                              uint32_t Offset) const {
-  const uint64_t InputOffset =
-      translate(OutputAddress, Offset, /*IsBranchSrc*/ false);
-
-  const uint64_t HotAddress = fetchParentAddress(OutputAddress);
-  auto MapsIter = Maps.find(HotAddress ? HotAddress : OutputAddress);
-  if (MapsIter == Maps.end())
-    return false;
-
-  const uint64_t InputAddress = MapsIter->second.begin()->second;
-
-  auto FunctionIt = SecondaryEntryPointsMap.find(Address);
-  if (FunctionIt == SecondaryEntryPointsMap.end())
-    return false;
-  const std::vector<uint32_t> &Offsets = FunctionIt->second;
-  uint64_t InputOffset = translate(OutputAddress, Offset, /*IsBranchSrc*/ false);
-  auto OffsetIt = llvm::lower_bound(Offsets, InputOffset << 1);
-  return OffsetIt != Offsets.end() && *OffsetIt >> 1 == InputOffset;
-}
-
-bool BoltAddressTranslation::isCallContinuationLandingPad(
-    uint64_t Address, uint32_t Offset) const {
-}
-
 } // namespace bolt
 } // namespace llvm

bolt/lib/Profile/DataAggregator.cpp

@@ -809,18 +809,11 @@ bool DataAggregator::doBranch(uint64_t From, uint64_t To, uint64_t Count,
       return false;
 
     if (!Func.hasCFG()) {
-      if (!BAT)
-        return false;
-      const uint64_t FuncAddress = Func.getAddress();
-      const BinaryData *BD =
-        BC->getBinaryDataAtAddress(FuncAddress + Offset);
-      unsigned EntryID = 0;
-      if (BD && BD->getSymbol())
-        EntryID = BAT->translateSymbol(*BC, *BD->getSymbol(), 0).second;
-      const MCSymbol &Symbol =
-      const auto [Function, EntryID] = BAT->translateSymbol(BC,
-      return BAT && !(BAT->isSecondaryEntry(FuncAddress, Offset) ||
-                      BAT->isCallContinuationLandingPad(FuncAddress, Offset));
+      const uint64_t Address = Func.getAddress();
+      // Check if offset is a secondary entry point or a call continuation
+      // landing pad (offset shifted by function size).
+      return BAT && !BAT->getSecondaryEntryPointId(Address, Offset) &&
+             !BAT->getSecondaryEntryPointId(Address, Func.getSize() + Offset);
     }
 
     // The offset should not be an entry point or a landing pad.