More refactoring work.

Summary:
Avoid referring to BinaryFunction's by name.

Functions could be found by MCSymbol using
BinaryContext::getFunctionForSymbol().

(cherry picked from FBD3707685)
This commit is contained in:
Maksim Panchenko
2016-08-11 14:23:54 -07:00
parent 36df6057b0
commit 003d106c0b
7 changed files with 98 additions and 90 deletions

View File

@@ -39,6 +39,7 @@
#include <set>
#include <string>
#include <system_error>
#include <unordered_map>
#include <vector>
namespace llvm {
@@ -65,6 +66,9 @@ public:
/// [address] -> [name1], [name2], ...
std::multimap<uint64_t, std::string> GlobalAddresses;
/// [MCSymbol] -> [BinaryFunction]
std::unordered_map<const MCSymbol *, const BinaryFunction *> SymbolToFunctionMap;
/// Map virtual address to a section.
std::map<uint64_t, SectionRef> AllocatableSections;
@@ -173,6 +177,11 @@ public:
GlobalAddresses.emplace(std::make_pair(Address, Name));
}
const BinaryFunction *getFunctionForSymbol(const MCSymbol *Symbol) const {
auto BFI = SymbolToFunctionMap.find(Symbol);
return BFI == SymbolToFunctionMap.end() ? nullptr : BFI->second;
}
/// Populate some internal data structures with debug info.
void preprocessDebugInfo(
std::map<uint64_t, BinaryFunction> &BinaryFunctions);

View File

@@ -107,9 +107,6 @@ private:
/// A list of function names.
std::vector<std::string> Names;
/// Symbol associated with this function in the input.
SymbolRef Symbol;
/// Containing section
SectionRef Section;
@@ -278,7 +275,8 @@ private:
/// construction. Map from tail call terminated basic block to a struct with
/// information about the tail call.
struct TailCallInfo {
uint32_t Offset; // offset of the tail call from the function start
uint32_t Offset; // offset of the tail call from the function
// start
uint32_t Index; // index of the tail call in the basic block
uint64_t TargetAddress; // address of the callee
uint64_t Count{0}; // taken count from profile data
@@ -366,8 +364,24 @@ private:
Itr itr;
};
BinaryFunction& operator=(const BinaryFunction &) = delete;
BinaryFunction(const BinaryFunction &) = delete;
friend class RewriteInstance;
/// Creation should be handled by RewriteInstance::createBinaryFunction().
BinaryFunction(const std::string &Name, SectionRef Section, uint64_t Address,
uint64_t Size, BinaryContext &BC, bool IsSimple) :
Names({Name}), Section(Section), Address(Address),
IdenticalFunctionAddress(Address), Size(Size), BC(BC), IsSimple(IsSimple),
CodeSectionName(".text." + Name), FunctionNumber(++Count) {
OutputSymbol = BC.Ctx->getOrCreateSymbol(Name);
}
public:
BinaryFunction(BinaryFunction &&) = default;
typedef Iterator<BasicBlockListType::iterator, BinaryBasicBlock> iterator;
typedef Iterator<BasicBlockListType::const_iterator,
const BinaryBasicBlock> const_iterator;
@@ -446,21 +460,6 @@ public:
return iterator_range<const_cfi_iterator>(cie_begin(), cie_end());
}
BinaryFunction& operator=(const BinaryFunction &) = delete;
BinaryFunction(const BinaryFunction &) = delete;
BinaryFunction(BinaryFunction &&) = default;
BinaryFunction(const std::string &Name, SymbolRef Symbol, SectionRef Section,
uint64_t Address, uint64_t Size, BinaryContext &BC,
bool IsSimple = true) :
Names({Name}), Symbol(Symbol), Section(Section), Address(Address),
IdenticalFunctionAddress(Address), Size(Size), BC(BC), IsSimple(IsSimple),
CodeSectionName(".text." + Name), FunctionNumber(++Count)
{
OutputSymbol = BC.Ctx->getOrCreateSymbol(Name);
}
/// Modify code layout making necessary adjustments to instructions at the
/// end of basic blocks.
void modifyLayout(LayoutType Type, bool MinBranchClusters, bool Split);

View File

@@ -81,16 +81,11 @@ void OptimizeBodylessFunctions::analyze(
auto Expr = dyn_cast<MCSymbolRefExpr>(Op1.getExpr());
if (!Expr)
return;
auto AddressIt = BC.GlobalSymbols.find(Expr->getSymbol().getName());
if (AddressIt == BC.GlobalSymbols.end())
return;
auto CalleeIt = BFs.find(AddressIt->second);
if (CalleeIt == BFs.end())
const auto *Function = BC.getFunctionForSymbol(&Expr->getSymbol());
if (!Function)
return;
assert(&Expr->getSymbol() == CalleeIt->second.getSymbol());
EquivalentCallTarget[BF.getSymbol()->getName()] = &CalleeIt->second;
EquivalentCallTarget[BF.getSymbol()] = Function;
}
void OptimizeBodylessFunctions::optimizeCalls(BinaryFunction &BF,
@@ -112,9 +107,8 @@ void OptimizeBodylessFunctions::optimizeCalls(BinaryFunction &BF,
// Iteratively update target since we could have f1() calling f2()
// calling f3() calling f4() and we want to output f1() directly
// calling f4().
while (EquivalentCallTarget.count(Target->getName())) {
Target =
EquivalentCallTarget.find(Target->getName())->second->getSymbol();
while (EquivalentCallTarget.count(Target)) {
Target = EquivalentCallTarget.find(Target)->second->getSymbol();
}
if (Target == OriginalTarget)
continue;
@@ -163,7 +157,7 @@ void InlineSmallFunctions::findInliningCandidates(
BB.size() <= kMaxInstructions &&
BC.MIA->isReturn(LastInstruction) &&
!BC.MIA->isTailCall(LastInstruction)) {
InliningCandidates.insert(Function.getSymbol()->getName());
InliningCandidates.insert(&Function);
}
}
@@ -205,7 +199,7 @@ void InlineSmallFunctions::findInliningCandidatesAggressive(
}
}
if (!FoundCFI)
InliningCandidates.insert(Function.getSymbol()->getName());
InliningCandidates.insert(&Function);
}
DEBUG(errs() << "BOLT-DEBUG: " << InliningCandidates.size()
@@ -568,26 +562,26 @@ bool InlineSmallFunctions::inlineCallsInFunction(
auto Target = dyn_cast<MCSymbolRefExpr>(
Inst.getOperand(0).getExpr());
assert(Target && "Not MCSymbolRefExpr");
auto FunctionIt = FunctionByName.find(Target->getSymbol().getName());
if (FunctionIt != FunctionByName.end()) {
auto &TargetFunction = *FunctionIt->second;
const auto *TargetFunction =
BC.getFunctionForSymbol(&Target->getSymbol());
if (TargetFunction) {
bool CallToInlineableFunction =
InliningCandidates.count(TargetFunction.getSymbol()->getName());
InliningCandidates.count(TargetFunction);
totalInlineableCalls +=
CallToInlineableFunction * BB->getExecutionCount();
if (CallToInlineableFunction &&
TargetFunction.getSize() + ExtraSize
TargetFunction->getSize() + ExtraSize
+ Function.estimateHotSize() < Function.getMaxSize()) {
auto NextInstIt = std::next(InstIt);
inlineCall(BC, *BB, &Inst, *TargetFunction.begin());
inlineCall(BC, *BB, &Inst, *TargetFunction->begin());
DidInlining = true;
DEBUG(errs() << "BOLT-DEBUG: Inlining call to "
<< TargetFunction << " in "
<< *TargetFunction << " in "
<< Function << "\n");
InstIt = NextInstIt;
ExtraSize += TargetFunction.getSize();
ExtraSize += TargetFunction->getSize();
inlinedDynamicCalls += BB->getExecutionCount();
continue;
}
@@ -637,29 +631,29 @@ bool InlineSmallFunctions::inlineCallsInFunctionAggressive(
auto Target = dyn_cast<MCSymbolRefExpr>(
Inst.getOperand(0).getExpr());
assert(Target && "Not MCSymbolRefExpr");
auto FunctionIt = FunctionByName.find(Target->getSymbol().getName());
if (FunctionIt != FunctionByName.end()) {
auto &TargetFunction = *FunctionIt->second;
const auto *TargetFunction =
BC.getFunctionForSymbol(&Target->getSymbol());
if (TargetFunction) {
bool CallToInlineableFunction =
InliningCandidates.count(TargetFunction.getSymbol()->getName());
InliningCandidates.count(TargetFunction);
totalInlineableCalls +=
CallToInlineableFunction * BB->getExecutionCount();
if (CallToInlineableFunction &&
TargetFunction.getSize() + ExtraSize
TargetFunction->getSize() + ExtraSize
+ Function.estimateHotSize() < Function.getMaxSize()) {
unsigned NextInstIndex = 0;
BinaryBasicBlock *NextBB = nullptr;
std::tie(NextBB, NextInstIndex) =
inlineCall(BC, Function, BB, InstIndex, TargetFunction);
inlineCall(BC, Function, BB, InstIndex, *TargetFunction);
DidInlining = true;
DEBUG(errs() << "BOLT-DEBUG: Inlining call to "
<< TargetFunction << " in "
<< *TargetFunction << " in "
<< Function << "\n");
InstIndex = NextBB == BB ? NextInstIndex : BB->size();
InstIt = NextBB == BB ? BB->begin() + NextInstIndex : BB->end();
ExtraSize += TargetFunction.getSize();
ExtraSize += TargetFunction->getSize();
inlinedDynamicCalls += BB->getExecutionCount();
continue;
}
@@ -678,10 +672,6 @@ void InlineSmallFunctions::runOnFunctions(
BinaryContext &BC,
std::map<uint64_t, BinaryFunction> &BFs,
std::set<uint64_t> &) {
for (auto &It : BFs) {
FunctionByName[It.second.getSymbol()->getName()] = &It.second;
}
findInliningCandidates(BC, BFs);
std::vector<BinaryFunction *> ConsideredFunctions;
@@ -878,26 +868,21 @@ bool SimplifyConditionalTailCalls::fixTailCalls(BinaryContext &BC,
cast<MCSymbolRefExpr>(Instr.getOperand(0).getExpr());
auto const &TailTarget = TailTargetSymExpr->getSymbol();
// Lookup the address for the current function and
// the tail call target.
auto const FnAddress =
BC.GlobalSymbols.find(BF.getSymbol()->getName());
// Lookup the address for the tail call target.
auto const TailAddress = BC.GlobalSymbols.find(TailTarget.getName());
if (FnAddress == BC.GlobalSymbols.end() ||
TailAddress == BC.GlobalSymbols.end()) {
if (TailAddress == BC.GlobalSymbols.end())
continue;
}
// Check to make sure we would be doing a forward jump.
// This assumes the address range of the current BB and the
// tail call target address don't overlap.
if (FnAddress->second < TailAddress->second) {
if (BF.getAddress() < TailAddress->second) {
++NumTailCallsPatched;
++NumLocalPatchedTailCalls;
// Is the original jump forward or backward?
const bool isForward =
TailAddress->second > FnAddress->second + BB->getOffset();
TailAddress->second > BF.getAddress() + BB->getOffset();
if (isForward) ++NumOrigForwardBranches;
@@ -1118,25 +1103,19 @@ void IdenticalCodeFolding::discoverCallers(
}
// Find the target function for this call.
const MCExpr *TargetExpr = TargetOp.getExpr();
const auto *TargetExpr = TargetOp.getExpr();
assert(TargetExpr->getKind() == MCExpr::SymbolRef);
const MCSymbol &TargetSymbol =
const auto &TargetSymbol =
dyn_cast<MCSymbolRefExpr>(TargetExpr)->getSymbol();
auto AI = BC.GlobalSymbols.find(TargetSymbol.getName());
assert(AI != BC.GlobalSymbols.end());
uint64_t TargetAddress = AI->second;
auto FI = BFs.find(TargetAddress);
if (FI == BFs.end()) {
const auto *Function = BC.getFunctionForSymbol(&TargetSymbol);
if (!Function) {
// Call to a function without a BinaryFunction object.
++InstrIndex;
continue;
}
BinaryFunction *Callee = &FI->second;
// Insert a tuple in the Callers map.
Callers[Callee].emplace_back(
Callers[Function].emplace_back(
CallSite(&Caller, BlockIndex, InstrIndex));
++InstrIndex;
}
}

View File

@@ -19,6 +19,7 @@
#include <map>
#include <set>
#include <string>
#include <unordered_map>
namespace llvm {
namespace bolt {
@@ -38,7 +39,8 @@ class OptimizeBodylessFunctions : public BinaryFunctionPass {
private:
/// EquivalentCallTarget[F] = G ==> function F is simply a tail call to G,
/// thus calls to F can be optimized to calls to G.
std::map<std::string, const BinaryFunction *> EquivalentCallTarget;
std::unordered_map<const MCSymbol *, const BinaryFunction *>
EquivalentCallTarget;
void analyze(BinaryFunction &BF,
BinaryContext &BC,
@@ -58,9 +60,7 @@ public:
/// correctness and we may break exception handling because of this.
class InlineSmallFunctions : public BinaryFunctionPass {
private:
std::set<std::string> InliningCandidates;
/// Maps function name to BinaryFunction.
std::map<std::string, const BinaryFunction *> FunctionByName;
std::set<const BinaryFunction *> InliningCandidates;
/// Maximum number of instructions in an inlined function.
static const unsigned kMaxInstructions = 8;
@@ -226,7 +226,7 @@ class IdenticalCodeFolding : public BinaryFunctionPass {
CallSite(BinaryFunction *Caller, unsigned BlockIndex, unsigned InstrIndex) :
Caller(Caller), BlockIndex(BlockIndex), InstrIndex(InstrIndex) { }
};
using CallerMap = std::map<BinaryFunction *, std::vector<CallSite>>;
using CallerMap = std::map<const BinaryFunction *, std::vector<CallSite>>;
CallerMap Callers;
/// Replaces all calls to BFTOFold with calls to BFToReplaceWith and merges

View File

@@ -119,6 +119,8 @@ private:
class AddressRangesOwner {
public:
virtual void setAddressRangesOffset(uint32_t Offset) = 0;
virtual ~AddressRangesOwner() {}
};
/// Represents DWARF entities that have generic address ranges, maintaining

View File

@@ -342,9 +342,11 @@ ExecutableFileMemoryManager::~ExecutableFileMemoryManager() {
}
}
namespace {
/// Create BinaryContext for a given architecture \p ArchName and
/// triple \p TripleName.
static std::unique_ptr<BinaryContext> CreateBinaryContext(
std::unique_ptr<BinaryContext> createBinaryContext(
std::string ArchName,
std::string TripleName,
const DataReader &DR,
@@ -451,10 +453,12 @@ static std::unique_ptr<BinaryContext> CreateBinaryContext(
return BC;
}
} // namespace
RewriteInstance::RewriteInstance(ELFObjectFileBase *File,
const DataReader &DR)
: InputFile(File),
BC(CreateBinaryContext("x86-64", "x86_64-unknown-linux", DR,
BC(createBinaryContext("x86-64", "x86_64-unknown-linux", DR,
std::unique_ptr<DWARFContext>(new DWARFContextInMemory(*InputFile)))) {
}
@@ -464,7 +468,7 @@ void RewriteInstance::reset() {
BinaryFunctions.clear();
FileSymRefs.clear();
auto &DR = BC->DR;
BC = CreateBinaryContext("x86-64", "x86_64-unknown-linux", DR,
BC = createBinaryContext("x86-64", "x86_64-unknown-linux", DR,
std::unique_ptr<DWARFContext>(new DWARFContextInMemory(*InputFile)));
CFIRdWrt.reset(nullptr);
SectionMM.reset(nullptr);
@@ -780,26 +784,24 @@ void RewriteInstance::discoverFileObjects() {
}
}
BinaryFunction *BF{nullptr};
auto BFI = BinaryFunctions.find(Address);
if (BFI != BinaryFunctions.end()) {
BF = &BFI->second;
// Duplicate function name. Make sure everything matches before we add
// an alternative name.
if (SymbolSize != BFI->second.getSize()) {
if (SymbolSize != BF->getSize()) {
errs() << "BOLT-WARNING: size mismatch for duplicate entries "
<< UniqueName << ':' << SymbolSize << " and "
<< BFI->second << ':' << BFI->second.getSize() << '\n';
<< *BF << ':' << BF->getSize() << '\n';
}
BFI->second.addAlternativeName(UniqueName);
BF->addAlternativeName(UniqueName);
} else {
// Create the function and add it to the map.
auto Result = BinaryFunctions.emplace(
Address,
BinaryFunction(UniqueName, Symbol, *Section, Address, SymbolSize,
*BC, IsSimple));
BFI = Result.first;
BF = createBinaryFunction(UniqueName, *Section, Address, SymbolSize,
IsSimple);
}
if (!AlternativeName.empty())
BFI->second.addAlternativeName(AlternativeName);
BF->addAlternativeName(AlternativeName);
}
if (!SeenFileName && BC->DR.hasLocalsWithFileName() && !opts::AllowStripped) {
@@ -812,6 +814,17 @@ void RewriteInstance::discoverFileObjects() {
}
}
BinaryFunction *RewriteInstance::createBinaryFunction(
const std::string &Name, SectionRef Section, uint64_t Address,
uint64_t Size, bool IsSimple) {
auto Result = BinaryFunctions.emplace(
Address, BinaryFunction(Name, Section, Address, Size, *BC, IsSimple));
assert(Result.second == true && "unexpected duplicate function");
auto *BF = &Result.first->second;
BC->SymbolToFunctionMap[BF->getSymbol()] = BF;
return BF;
}
void RewriteInstance::readSpecialSections() {
// Process special sections.
StringRef FrameHdrContents;

View File

@@ -332,6 +332,12 @@ private:
/// Total hotness score according to profiling data for this binary.
uint64_t TotalScore{0};
/// Construct BinaryFunction object and add it to internal maps.
BinaryFunction *createBinaryFunction(const std::string &Name,
object::SectionRef Section,
uint64_t Address,
uint64_t Size,
bool IsSimple);
};
} // namespace bolt