[llvm] [BOLT] Never emit "large" functions (PR #115974)
Maksim Panchenko via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 12 20:41:02 PST 2024
https://github.com/maksfb updated https://github.com/llvm/llvm-project/pull/115974
>From b31c1ca73965dc16c03e902e52a6542f5391d695 Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Tue, 12 Nov 2024 15:36:00 -0800
Subject: [PATCH 1/2] [BOLT] Never emit "large" functions
"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.
---
bolt/include/bolt/Core/Exceptions.h | 11 +++-----
bolt/include/bolt/Rewrite/RewriteInstance.h | 8 ------
bolt/lib/Core/BinaryFunction.cpp | 2 ++
bolt/lib/Core/Exceptions.cpp | 19 ++++---------
bolt/lib/Passes/BinaryPasses.cpp | 14 ++++++++--
bolt/lib/Rewrite/RewriteInstance.cpp | 31 ++++++---------------
6 files changed, 30 insertions(+), 55 deletions(-)
diff --git a/bolt/include/bolt/Core/Exceptions.h b/bolt/include/bolt/Core/Exceptions.h
index 422b86f6ddb7a3..f10cf776f09437 100644
--- a/bolt/include/bolt/Core/Exceptions.h
+++ b/bolt/include/bolt/Core/Exceptions.h
@@ -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 *>;
diff --git a/bolt/include/bolt/Rewrite/RewriteInstance.h b/bolt/include/bolt/Rewrite/RewriteInstance.h
index e5b7ad63007cab..73d2857f946cce 100644
--- a/bolt/include/bolt/Rewrite/RewriteInstance.h
+++ b/bolt/include/bolt/Rewrite/RewriteInstance.h
@@ -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;
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 5da777411ba7a1..f208da7df43d7d 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -485,6 +485,8 @@ void BinaryFunction::print(raw_ostream &OS, std::string Annotation) {
OS << "\n Branch Count: " << RawBranchCount;
OS << "\n Profile Acc : " << format("%.1f%%", ProfileMatchRatio * 100.0f);
}
+ if (hasConstantIsland())
+ OS << "\n Has constant island";
if (opts::PrintDynoStats && !getLayout().block_empty()) {
OS << '\n';
diff --git a/bolt/lib/Core/Exceptions.cpp b/bolt/lib/Core/Exceptions.cpp
index 8c55d18bc1e115..0b2e63b8ca6a79 100644
--- a/bolt/lib/Core/Exceptions.cpp
+++ b/bolt/lib/Core/Exceptions.cpp
@@ -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);
@@ -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 "
diff --git a/bolt/lib/Passes/BinaryPasses.cpp b/bolt/lib/Passes/BinaryPasses.cpp
index 8e46c22a4e0247..03d3dd75a03368 100644
--- a/bolt/lib/Passes/BinaryPasses.cpp
+++ b/bolt/lib/Passes/BinaryPasses.cpp
@@ -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);
}
};
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index a4c50cbc3e2bbf..1fcf2bb959bbbb 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -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());
@@ -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) {
@@ -3852,19 +3848,11 @@ 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(
@@ -3872,9 +3860,6 @@ void RewriteInstance::mapCodeSections(BOLTLinker::SectionMapper MapSection) {
FF.getImageAddress(), FF.getAddress(), FF.getImageSize()));
MapSection(*ColdSection, FF.getAddress());
- if (TooLarge)
- BC->deregisterSection(*ColdSection);
-
NextAvailableAddress += FF.getImageSize();
}
@@ -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());
>From d475c0361804b4d2954a5a750ab60c4669b961bd Mon Sep 17 00:00:00 2001
From: Maksim Panchenko <maks at fb.com>
Date: Tue, 12 Nov 2024 20:40:52 -0800
Subject: [PATCH 2/2] fixup! [BOLT] Never emit "large" functions
---
bolt/lib/Core/BinaryFunction.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index f208da7df43d7d..5da777411ba7a1 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -485,8 +485,6 @@ void BinaryFunction::print(raw_ostream &OS, std::string Annotation) {
OS << "\n Branch Count: " << RawBranchCount;
OS << "\n Profile Acc : " << format("%.1f%%", ProfileMatchRatio * 100.0f);
}
- if (hasConstantIsland())
- OS << "\n Has constant island";
if (opts::PrintDynoStats && !getLayout().block_empty()) {
OS << '\n';
More information about the llvm-commits
mailing list