Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
03ca42c
safe-icf
Nov 12, 2024
237c02e
Changed icf option
Nov 15, 2024
d794fb0
addressed comments
Nov 15, 2024
6e853ca
Added four tests for pic/no-pic mode. Tests for global const function…
Nov 16, 2024
e36ec02
switched to enum version of icf compile option
Nov 16, 2024
b83c335
simplified debug print, removed shared func
Nov 20, 2024
39c7902
updated tests
Nov 20, 2024
39586b3
addressed comments
Nov 20, 2024
debeb6f
updated icf option descriptions
Nov 20, 2024
429075e
moved getRelocation* to Relocs.cpp
Nov 20, 2024
29cc466
moved code around
Nov 20, 2024
bf3bf2b
simplified assembly
Nov 22, 2024
e867900
changed comment
Nov 22, 2024
ab51077
consolidate main/helper assembly under main. Next step move to test t…
Nov 25, 2024
8fe4b3c
move assembly into test
Nov 26, 2024
dc41c58
removed some lines from assembly
Nov 26, 2024
6287a7f
removed comments, changed demangled names for key functions
Nov 26, 2024
c6a1965
removed .text and .file
Nov 26, 2024
d393b5e
Changed schedule strategy, cleaned up tests some more, and removed so…
Nov 30, 2024
4ace9bd
moved reloc checking code, and changed elf object from assert to retu…
Nov 30, 2024
09bc3fa
fixing formatting that was messed up due to VSC format on save gettin…
Nov 30, 2024
3912b85
some cleanup after code changes to address comments
Dec 1, 2024
769cf24
clang-format
Dec 1, 2024
d4f81c9
removed some redundant tests, re-added on more targetted one that jus…
Dec 4, 2024
ec54f24
removed one more test I missed
Dec 4, 2024
13bcc22
Changed so that general data sections are processed. Moved ICF option…
Dec 5, 2024
5110f74
Added vtable filtering, and check fo X86 architecture
Dec 6, 2024
d524ed5
moved check as first one
Dec 6, 2024
77ae0a2
fix formatter
Dec 6, 2024
d03ff1d
minor nits
Dec 10, 2024
38758ee
Removed skip and moved processInstructionForFuncReferences, nits
Dec 10, 2024
bfb1dc6
refactor bit vector
Dec 11, 2024
417196c
nit
Dec 11, 2024
bbf015f
changed to using BinarySections, cleaned up code that is no longer ne…
Dec 13, 2024
1ef1d27
clang-format
Dec 13, 2024
f1813b3
more cleanup
Dec 13, 2024
eb882e5
addressed comments
Dec 13, 2024
f6dcb90
added comment for static relocs
Dec 13, 2024
0070ceb
Added deprecation warning, switched to SparseBitVector
Dec 14, 2024
0f96524
changed comments
Dec 16, 2024
a9cd467
more cleanup
Dec 16, 2024
6a89d5c
addressed comments
Dec 17, 2024
645aac2
small change to comment
Dec 17, 2024
23ea76b
changed comments for HasAddressTaken
Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions bolt/include/bolt/Passes/IdenticalCodeFolding.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,29 @@ class IdenticalCodeFolding : public BinaryFunctionPass {
All, // Aggressive ICF for code.
};
explicit IdenticalCodeFolding(const cl::opt<bool> &PrintPass)
: BinaryFunctionPass(PrintPass) {}
: BinaryFunctionPass(PrintPass) {
VtableBitVector.resize((((uint64_t)1) << 32) / 8);
}

const char *getName() const override { return "identical-code-folding"; }
Error runOnFunctions(BinaryContext &BC) override;

private:
/// Bit vector of memory addresses of vtables.
llvm::BitVector VtableBitVector;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to use SparseBitVector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding random access is O(n) for it?
Considering how much debug information processing takes, seems fine?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, random access is O(N), but sequential is not:

testing/setting bits in a SparseBitVector is O(distance away from last set bit).

Vtables should occupy a vanishingly small part of the binary, so sparse shouldn't be too inefficient. You can also drop an explicit size. Can you check the two and compare peak RSS (disabling debug info update) and pass wall time?

Or let @maksfb weigh in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BitVector
12:10.63 real, 2190.50 user, 3352.54 sys, 0 amem, 50543836 mmem
12:15.47 real, 2171.61 user, 3110.00 sys, 0 amem, 50486944 mmem
12:15.42 real, 2199.27 user, 3336.12 sys, 0 amem, 50550316 mmem
SparceBItVector
11:56.70 real, 2173.22 user, 3025.61 sys, 0 amem, 50546564 mmem
12:25.82 real, 2196.06 user, 3279.76 sys, 0 amem, 50561464 mmem
11:48.03 real, 2254.86 user, 3216.55 sys, 0 amem, 50547536 mmem

Seems slightly faster with SparseBitVector. 🤷

bool isInVTable(uint64_t Address) const {
return VtableBitVector.test(Address / 8);
}
/// Scans symbol table and creates a bit vector of memory addresses of
/// vtables.
void processSymbolTable(const BinaryContext &BC);
/// Analyze .text section and relocations and mark functions that are not
/// safe to fold.
Error markFunctionsUnsafeToFold(BinaryContext &BC);
/// Process relocations in the .data section to identify function
/// references.
Error processDataRelocations(BinaryContext &BC,
const SectionRef &SecRefRelData,
const llvm::BitVector &BitVector,
const bool HasAddressTaken);
};

Expand Down
27 changes: 12 additions & 15 deletions bolt/lib/Passes/IdenticalCodeFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,22 +355,21 @@ typedef std::unordered_map<BinaryFunction *, std::vector<BinaryFunction *>,
namespace llvm {
namespace bolt {
/// Scans symbol table and creates a bit vector of memory addresses of vtables.
static void processSymbolTable(const BinaryContext &BC,
llvm::BitVector &BitVector) {
void IdenticalCodeFolding::processSymbolTable(const BinaryContext &BC) {
for (auto &[Address, Data] : BC.getBinaryData()) {
// Filter out all symbols that are not vtables.
if (!Data->getName().starts_with("_ZTV"))
continue;
for (uint64_t I = Address / 8, End = I + (Data->getSize() / 8); I < End;
++I)
BitVector.set(I);
VtableBitVector.set(I);
}
}
Error IdenticalCodeFolding::processDataRelocations(
BinaryContext &BC, const SectionRef &SecRefRelData,
const llvm::BitVector &BitVector, const bool HasAddressTaken) {
const bool HasAddressTaken) {
for (const RelocationRef &Rel : SecRefRelData.relocations()) {
if (BitVector.test(Rel.getOffset() / 8))
if (isInVTable(Rel.getOffset()))
continue;
symbol_iterator SymbolIter = Rel.getSymbol();
const ObjectFile *OwningObj = Rel.getObject();
Expand All @@ -393,10 +392,8 @@ Error IdenticalCodeFolding::processDataRelocations(

Error IdenticalCodeFolding::markFunctionsUnsafeToFold(BinaryContext &BC) {
if (!BC.isX86())
BC.outs() << "BOLT-WARNING: Safe ICF is only supported for x86\n";
constexpr uint64_t NumBits = (((uint64_t)1) << 32) / 8;
llvm::BitVector BitVector(NumBits);
processSymbolTable(BC, BitVector);
BC.outs() << "BOLT-WARNING: safe ICF is only supported for x86\n";
processSymbolTable(BC);
for (const auto &Sec : BC.sections()) {
if (!Sec.hasSectionRef() || !Sec.isRela())
continue;
Expand All @@ -414,26 +411,26 @@ Error IdenticalCodeFolding::markFunctionsUnsafeToFold(BinaryContext &BC) {
if (!RelocatedSecRef.isData() || SkipRelocs)
continue;

Error ErrorStatus = processDataRelocations(BC, SecRef, BitVector,
/* HasAddressTaken */ true);
Error ErrorStatus =
processDataRelocations(BC, SecRef, /* HasAddressTaken */ true);
if (ErrorStatus)
return ErrorStatus;
}
ErrorOr<BinarySection &> SecRelDataIA =
BC.getUniqueSectionByName(".rela.init_array");
if (SecRelDataIA) {
const SectionRef SecRefRelData = SecRelDataIA->getSectionRef();
Error ErrorStatus = processDataRelocations(BC, SecRefRelData, BitVector,
/* !HasAddressTaken */ false);
Error ErrorStatus =
processDataRelocations(BC, SecRefRelData, /* !HasAddressTaken */ false);
if (ErrorStatus)
return ErrorStatus;
}
ErrorOr<BinarySection &> SecRelDataFIA =
BC.getUniqueSectionByName(".rela.fini_array");
if (SecRelDataFIA) {
const SectionRef SecRefRelData = SecRelDataFIA->getSectionRef();
Error ErrorStatus = processDataRelocations(BC, SecRefRelData, BitVector,
/* !HasAddressTaken */ false);
Error ErrorStatus =
processDataRelocations(BC, SecRefRelData, /* !HasAddressTaken */ false);
if (ErrorStatus)
return ErrorStatus;
}
Expand Down
Loading