Skip to content

Commit

Permalink
[BOLT] Never emit "large" functions (#115974)
Browse files Browse the repository at this point in the history
"Large" functions are functions that are too big to fit into their
original slots after code modifications. CheckLargeFunctions pass is
designed to prevent such functions from emission. Extend this pass to
work with functions with constant islands.

Now that CheckLargeFunctions covers all functions, it guarantees that we
will never see such functions after code emission on all platforms
(previously it was guaranteed on x86 only). Hence, we can get rid of
RewriteInstance extensions that were meant to support "large" functions.
  • Loading branch information
maksfb authored Nov 13, 2024
1 parent 6b2de10 commit 1b8e0cf
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 55 deletions.
11 changes: 4 additions & 7 deletions bolt/include/bolt/Core/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,14 @@ class CFIReaderWriter {

/// Generate .eh_frame_hdr from old and new .eh_frame sections.
///
/// Take FDEs from the \p NewEHFrame unless their initial_pc is listed
/// in \p FailedAddresses. All other entries are taken from the
/// Take FDEs from the \p NewEHFrame. All other entries are taken from the
/// \p OldEHFrame.
///
/// \p EHFrameHeaderAddress specifies location of .eh_frame_hdr,
/// and is required for relative addressing used in the section.
std::vector<char>
generateEHFrameHeader(const DWARFDebugFrame &OldEHFrame,
const DWARFDebugFrame &NewEHFrame,
uint64_t EHFrameHeaderAddress,
std::vector<uint64_t> &FailedAddresses) const;
std::vector<char> generateEHFrameHeader(const DWARFDebugFrame &OldEHFrame,
const DWARFDebugFrame &NewEHFrame,
uint64_t EHFrameHeaderAddress) const;

using FDEsMap = std::map<uint64_t, const dwarf::FDE *>;

Expand Down
8 changes: 0 additions & 8 deletions bolt/include/bolt/Rewrite/RewriteInstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -556,14 +556,6 @@ class RewriteInstance {
return ErrOrSection ? &ErrOrSection.get() : nullptr;
}

/// Keep track of functions we fail to write in the binary. We need to avoid
/// rewriting CFI info for these functions.
std::vector<uint64_t> FailedAddresses;

/// Keep track of which functions didn't fit in their original space in the
/// last emission, so that we may either decide to split or not optimize them.
std::set<uint64_t> LargeFunctions;

/// Section header string table.
StringTableBuilder SHStrTab;

Expand Down
19 changes: 5 additions & 14 deletions bolt/lib/Core/Exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,16 +665,13 @@ bool CFIReaderWriter::fillCFIInfoFor(BinaryFunction &Function) const {
return true;
}

std::vector<char> CFIReaderWriter::generateEHFrameHeader(
const DWARFDebugFrame &OldEHFrame, const DWARFDebugFrame &NewEHFrame,
uint64_t EHFrameHeaderAddress,
std::vector<uint64_t> &FailedAddresses) const {
std::vector<char>
CFIReaderWriter::generateEHFrameHeader(const DWARFDebugFrame &OldEHFrame,
const DWARFDebugFrame &NewEHFrame,
uint64_t EHFrameHeaderAddress) const {
// Common PC -> FDE map to be written into .eh_frame_hdr.
std::map<uint64_t, uint64_t> PCToFDE;

// Presort array for binary search.
llvm::sort(FailedAddresses);

// Initialize PCToFDE using NewEHFrame.
for (dwarf::FrameEntry &Entry : NewEHFrame.entries()) {
const dwarf::FDE *FDE = dyn_cast<dwarf::FDE>(&Entry);
Expand All @@ -689,13 +686,7 @@ std::vector<char> CFIReaderWriter::generateEHFrameHeader(
continue;

// Add the address to the map unless we failed to write it.
if (!std::binary_search(FailedAddresses.begin(), FailedAddresses.end(),
FuncAddress)) {
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: FDE for function at 0x"
<< Twine::utohexstr(FuncAddress) << " is at 0x"
<< Twine::utohexstr(FDEAddress) << '\n');
PCToFDE[FuncAddress] = FDEAddress;
}
PCToFDE[FuncAddress] = FDEAddress;
};

LLVM_DEBUG(dbgs() << "BOLT-DEBUG: new .eh_frame contains "
Expand Down
14 changes: 11 additions & 3 deletions bolt/lib/Passes/BinaryPasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,10 +588,18 @@ Error CheckLargeFunctions::runOnFunctions(BinaryContext &BC) {
uint64_t HotSize, ColdSize;
std::tie(HotSize, ColdSize) =
BC.calculateEmittedSize(BF, /*FixBranches=*/false);
if (HotSize > BF.getMaxSize()) {
uint64_t MainFragmentSize = HotSize;
if (BF.hasIslandsInfo()) {
MainFragmentSize +=
offsetToAlignment(BF.getAddress() + MainFragmentSize,
Align(BF.getConstantIslandAlignment()));
MainFragmentSize += BF.estimateConstantIslandSize();
}
if (MainFragmentSize > BF.getMaxSize()) {
if (opts::PrintLargeFunctions)
BC.outs() << "BOLT-INFO: " << BF << " size exceeds allocated space by "
<< (HotSize - BF.getMaxSize()) << " bytes\n";
BC.outs() << "BOLT-INFO: " << BF << " size of " << MainFragmentSize
<< " bytes exceeds allocated space by "
<< (MainFragmentSize - BF.getMaxSize()) << " bytes\n";
BF.setSimple(false);
}
};
Expand Down
31 changes: 8 additions & 23 deletions bolt/lib/Rewrite/RewriteInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3807,7 +3807,6 @@ void RewriteInstance::mapCodeSections(BOLTLinker::SectionMapper MapSection) {
if (!Function.isEmitted())
continue;

bool TooLarge = false;
ErrorOr<BinarySection &> FuncSection = Function.getCodeSection();
assert(FuncSection && "cannot find section for function");
FuncSection->setOutputAddress(Function.getAddress());
Expand All @@ -3818,11 +3817,8 @@ void RewriteInstance::mapCodeSections(BOLTLinker::SectionMapper MapSection) {
MapSection(*FuncSection, Function.getAddress());
Function.setImageAddress(FuncSection->getAllocAddress());
Function.setImageSize(FuncSection->getOutputSize());
if (Function.getImageSize() > Function.getMaxSize()) {
assert(!BC->isX86() && "Unexpected large function.");
TooLarge = true;
FailedAddresses.emplace_back(Function.getAddress());
}
assert(Function.getImageSize() <= Function.getMaxSize() &&
"Unexpected large function");

// Map jump tables if updating in-place.
if (opts::JumpTables == JTS_BASIC) {
Expand Down Expand Up @@ -3852,29 +3848,18 @@ void RewriteInstance::mapCodeSections(BOLTLinker::SectionMapper MapSection) {
assert(ColdSection && "cannot find section for cold part");
// Cold fragments are aligned at 16 bytes.
NextAvailableAddress = alignTo(NextAvailableAddress, 16);
if (TooLarge) {
// The corresponding FDE will refer to address 0.
FF.setAddress(0);
FF.setImageAddress(0);
FF.setImageSize(0);
FF.setFileOffset(0);
} else {
FF.setAddress(NextAvailableAddress);
FF.setImageAddress(ColdSection->getAllocAddress());
FF.setImageSize(ColdSection->getOutputSize());
FF.setFileOffset(getFileOffsetForAddress(NextAvailableAddress));
ColdSection->setOutputAddress(FF.getAddress());
}
FF.setAddress(NextAvailableAddress);
FF.setImageAddress(ColdSection->getAllocAddress());
FF.setImageSize(ColdSection->getOutputSize());
FF.setFileOffset(getFileOffsetForAddress(NextAvailableAddress));
ColdSection->setOutputAddress(FF.getAddress());

LLVM_DEBUG(
dbgs() << formatv(
"BOLT: mapping cold fragment {0:x+} to {1:x+} with size {2:x+}\n",
FF.getImageAddress(), FF.getAddress(), FF.getImageSize()));
MapSection(*ColdSection, FF.getAddress());

if (TooLarge)
BC->deregisterSection(*ColdSection);

NextAvailableAddress += FF.getImageSize();
}

Expand Down Expand Up @@ -5814,7 +5799,7 @@ void RewriteInstance::writeEHFrameHeader() {
getFileOffsetForAddress(NextAvailableAddress);

std::vector<char> NewEHFrameHdr = CFIRdWrt->generateEHFrameHeader(
RelocatedEHFrame, NewEHFrame, EHFrameHdrOutputAddress, FailedAddresses);
RelocatedEHFrame, NewEHFrame, EHFrameHdrOutputAddress);

Out->os().seek(EHFrameHdrFileOffset);
Out->os().write(NewEHFrameHdr.data(), NewEHFrameHdr.size());
Expand Down

0 comments on commit 1b8e0cf

Please sign in to comment.