https://github.com/aaupov updated https://github.com/llvm/llvm-project/pull/115334
>From 9476fad1aa50282a38614a63a6a5a41f0ac42532 Mon Sep 17 00:00:00 2001 From: Amir Ayupov <aau...@fb.com> Date: Wed, 27 Nov 2024 15:59:02 +0100 Subject: [PATCH] Preserve CFG until BAT, use it to check call cont landing pads, encode them in secondary entry points table --- bolt/docs/BAT.md | 20 +--- .../bolt/Profile/BoltAddressTranslation.h | 26 +---- bolt/lib/Core/BinaryEmitter.cpp | 3 +- bolt/lib/Profile/BoltAddressTranslation.cpp | 104 +++++------------- bolt/lib/Profile/DataAggregator.cpp | 17 +-- 5 files changed, 44 insertions(+), 126 deletions(-) diff --git a/bolt/docs/BAT.md b/bolt/docs/BAT.md index 20340884621b9a..828114310e195f 100644 --- a/bolt/docs/BAT.md +++ b/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). diff --git a/bolt/include/bolt/Profile/BoltAddressTranslation.h b/bolt/include/bolt/Profile/BoltAddressTranslation.h index b04ed7a82eeefb..f956f48b8356b2 100644 --- a/bolt/include/bolt/Profile/BoltAddressTranslation.h +++ b/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 { diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp index f34a94c5779213..31b93418f53943 100644 --- a/bolt/lib/Core/BinaryEmitter.cpp +++ b/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); } }; diff --git a/bolt/lib/Profile/BoltAddressTranslation.cpp b/bolt/lib/Profile/BoltAddressTranslation.cpp index 49b272ebd35c1e..6c26ed3e26b419 100644 --- a/bolt/lib/Profile/BoltAddressTranslation.cpp +++ b/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 diff --git a/bolt/lib/Profile/DataAggregator.cpp b/bolt/lib/Profile/DataAggregator.cpp index c471e97565a576..fcd3cd90271a26 100644 --- a/bolt/lib/Profile/DataAggregator.cpp +++ b/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. _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits