intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-13 19:08:21 +08:00
Code Issues Packages Projects Releases Wiki Activity

[BOLT] Improve handling of secondary function entry points

Browse Source 
Summary:
"Fix symbol table entries for secondary entries" diff broke the inliner.

Fix the breakage and make the discovery of secondary entry points more
accurate.

Add ability to BinaryContext::getFunctionForSymbol() to return an entry
point discriminator and use it instead of calling getEntryForSymbol()
and isSecondaryEntry(). This is the preferred way since
getFunctionForSymbol() is thread-safe.

(cherry picked from FBD19295983)
This commit is contained in:
Maksim Panchenko
2020-01-06 14:57:15 -08:00
parent 8c7f524afb
commit 088e3c032a
8 changed files with 84 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
bolt/src/BinaryContext.cpp
View File

@@ -983,7 +983,7 @@ void BinaryContext::postProcessSymbolTable() {
}
void BinaryContext::foldFunction(BinaryFunction &ChildBF,
BinaryFunction &ParentBF) {
BinaryFunction &ParentBF) {
std::shared_lock<std::shared_timed_mutex> ReadCtxLock(CtxMutex,
std::defer_lock);
std::unique_lock<std::shared_timed_mutex> WriteCtxLock(CtxMutex,
@@ -1761,6 +1761,20 @@ void BinaryContext::markAmbiguousRelocations(BinaryData &BD,
}
}
BinaryFunction *BinaryContext::getFunctionForSymbol(const MCSymbol *Symbol,
uint64_t *EntryDesc) {
std::shared_lock<std::shared_timed_mutex> Lock(SymbolToFunctionMapMutex);
auto BFI = SymbolToFunctionMap.find(Symbol);
if (BFI == SymbolToFunctionMap.end())
return nullptr;
auto *BF = BFI->second;
if (EntryDesc)
*EntryDesc = BF->getEntryForSymbol(Symbol);
return BF;
}
void BinaryContext::exitWithBugReport(StringRef Message,
const BinaryFunction &Function) const {
errs() << "=======================================\n";

19
bolt/src/BinaryContext.h
View File

@@ -922,17 +922,16 @@ public:
/// boundary between two symbols then those symbols are marked as immovable.
void markAmbiguousRelocations(BinaryData &BD, const uint64_t Address);
/// This function is thread safe.
const BinaryFunction *getFunctionForSymbol(const MCSymbol *Symbol) const {
std::shared_lock<std::shared_timed_mutex> Lock(SymbolToFunctionMapMutex);
auto BFI = SymbolToFunctionMap.find(Symbol);
return BFI == SymbolToFunctionMap.end() ? nullptr : BFI->second;
}
/// Return BinaryFunction corresponding to \p Symbol. If \p EntryDesc is not
/// nullptr, set it to entry descriminator corresponding to \p Symbol
/// (0 for single-entry functions). This function is thread safe.
BinaryFunction *getFunctionForSymbol(const MCSymbol *Symbol,
uint64_t *EntryDesc = nullptr);
BinaryFunction *getFunctionForSymbol(const MCSymbol *Symbol) {
std::shared_lock<std::shared_timed_mutex> Lock(SymbolToFunctionMapMutex);
auto BFI = SymbolToFunctionMap.find(Symbol);
return BFI == SymbolToFunctionMap.end() ? nullptr : BFI->second;
const BinaryFunction *getFunctionForSymbol(
const MCSymbol *Symbol, uint64_t *EntryDesc = nullptr) const {
return const_cast<BinaryContext *>(this)->
getFunctionForSymbol(Symbol, EntryDesc);
}
/// Associate the symbol \p Sym with the function \p BF for lookups with

49
bolt/src/BinaryFunction.cpp
View File

@@ -421,6 +421,7 @@ void BinaryFunction::print(raw_ostream &OS, std::string Annotation,
<< "\n Orc Section : " << getCodeSectionName()
<< "\n LSDA : 0x" << Twine::utohexstr(getLSDAAddress())
<< "\n IsSimple : " << IsSimple
<< "\n IsMultiEntry: " << IsMultiEntry
<< "\n IsSplit : " << isSplit()
<< "\n BB Count : " << size();
@@ -3367,31 +3368,53 @@ const MCSymbol *BinaryFunction::getSymbolForEntry(uint64_t EntryNum) const {
return nullptr;
uint64_t NumEntries = 0;
for (auto *BB : BasicBlocks) {
if (!BB->isEntryPoint())
continue;
if (NumEntries == EntryNum)
return BB->getLabel();
++NumEntries;
if (hasCFG()) {
for (auto *BB : BasicBlocks) {
if (!BB->isEntryPoint())
continue;
if (NumEntries == EntryNum)
return BB->getLabel();
++NumEntries;
}
} else {
for (auto &KV : Labels) {
if (NumEntries == EntryNum)
return KV.second;
++NumEntries;
}
}
return nullptr;
}
uint64_t BinaryFunction::getEntryForSymbol(const MCSymbol *EntrySymbol) const {
if (!isMultiEntry())
return 0;
if (getSymbol() == EntrySymbol)
return 0;
// The function might have multiple symbols associated with its main entry
// point. Check EntrySymbol against all secondary entry points and return 0
// corresponding to the main entry point if no match is found.
uint64_t NumEntries = 0;
for (const auto *BB : BasicBlocks) {
if (!BB->isEntryPoint())
continue;
if (BB->getLabel() == EntrySymbol)
return NumEntries;
++NumEntries;
if (hasCFG()) {
for (const auto *BB : BasicBlocks) {
if (!BB->isEntryPoint())
continue;
if (BB->getLabel() == EntrySymbol)
return NumEntries;
++NumEntries;
}
} else {
for (auto &KV : Labels) {
if (KV.second == EntrySymbol)
return NumEntries;
++NumEntries;
}
}
return std::numeric_limits<uint64_t>::max();
return 0;
}
BinaryFunction::BasicBlockOrderType BinaryFunction::dfs() const {

20
bolt/src/BinaryFunction.h
View File

@@ -594,6 +594,13 @@ private:
/// This is called in disassembled state.
void addEntryPoint(uint64_t Address);
/// Return an entry ID corresponding to a symbol known to belong to
/// the function.
///
/// Prefer to use BinaryContext::getFunctionForSymbol(EntrySymbol, &ID)
/// instead of calling this function directly.
uint64_t getEntryForSymbol(const MCSymbol *EntrySymbol) const;
void setParentFunction(BinaryFunction *BF) {
assert((!ParentFunction || ParentFunction == BF) &&
"cannot have more than one parent function");
@@ -1061,19 +1068,6 @@ public:
/// functions.
const MCSymbol *getSymbolForEntry(uint64_t EntryNum) const;
/// Return an entry ID corresponding to a symbol. Return
/// numeric_limits<T>::max() if entry does not exist.
uint64_t getEntryForSymbol(const MCSymbol *EntrySymbol) const;
/// Return true if \p is a valid secondary entry point in this function, false
/// otherwise.
bool isSecondaryEntryPoint(const MCSymbol *Label) const {
uint64_t EntryNo = getEntryForSymbol(Label);
if (EntryNo == std::numeric_limits<uint64_t>::max())
return false;
return EntryNo != 0;
}
MCSymbol *getColdSymbol() {
if (ColdSymbol)
return ColdSymbol;

10
bolt/src/Passes/IdenticalCodeFolding.cpp
View File

@@ -223,11 +223,13 @@ bool isIdenticalWith(const BinaryFunction &A, const BinaryFunction &B,
return true;
// Compare symbols as functions.
const auto *FunctionA = BC.getFunctionForSymbol(SymbolA);
const auto *FunctionB = BC.getFunctionForSymbol(SymbolB);
if (FunctionA && FunctionA->isSecondaryEntryPoint(SymbolA))
uint64_t EntryIDA{0};
uint64_t EntryIDB{0};
const auto *FunctionA = BC.getFunctionForSymbol(SymbolA, &EntryIDA);
const auto *FunctionB = BC.getFunctionForSymbol(SymbolB, &EntryIDB);
if (FunctionA && EntryIDA)
FunctionA = nullptr;
if (FunctionB && FunctionB->isSecondaryEntryPoint(SymbolB))
if (FunctionB && EntryIDB)
FunctionB = nullptr;
if (FunctionA && FunctionB) {
// Self-referencing functions and recursive calls.

11
bolt/src/Passes/Inliner.cpp
View File

@@ -455,16 +455,15 @@ bool Inliner::inlineCallsInFunction(BinaryFunction &Function) {
const auto *TargetSymbol = BC.MIB->getTargetSymbol(Inst);
assert(TargetSymbol && "target symbol expected for direct call");
auto *TargetFunction = BC.getFunctionForSymbol(TargetSymbol);
if (!TargetFunction) {
// Don't inline calls to a secondary entry point in a target function.
uint64_t EntryID{0};
auto *TargetFunction = BC.getFunctionForSymbol(TargetSymbol, &EntryID);
if (!TargetFunction || EntryID != 0) {
++InstIt;
continue;
}
// Don't inline calls to a secondary entry point in a target function
if (TargetFunction->isSecondaryEntryPoint(TargetSymbol))
continue;
// Don't do recursive inlining.
if (TargetFunction == &Function) {
++InstIt;

6
bolt/src/Passes/LongJmp.cpp
View File

@@ -422,10 +422,10 @@ uint64_t LongJmpPass::getSymbolAddress(const BinaryContext &BC,
assert (Iter != BBAddresses.end() && "Unrecognized BB");
return Iter->second;
}
auto *TargetFunc = BC.getFunctionForSymbol(Target);
uint64_t EntryID{0};
auto *TargetFunc = BC.getFunctionForSymbol(Target, &EntryID);
auto Iter = HotAddresses.find(TargetFunc);
if (Iter == HotAddresses.end() ||
(TargetFunc && TargetFunc->isSecondaryEntryPoint(Target))) {
if (Iter == HotAddresses.end() || (TargetFunc && EntryID)) {
// Look at BinaryContext's resolution for this symbol - this is a symbol not
// mapped to a BinaryFunction
auto ValueOrError = BC.getSymbolValue(*Target);

5
bolt/src/ProfileWriter.cpp
View File

@@ -96,11 +96,12 @@ convert(const BinaryFunction &BF, yaml::bolt::BinaryFunctionProfile &YamlBF) {
YamlBB.CallSites.push_back(CSI);
}
} else { // direct call or a tail call
uint64_t EntryID{0};
const auto *CalleeSymbol = BC.MIB->getTargetSymbol(Instr);
const auto Callee = BC.getFunctionForSymbol(CalleeSymbol);
const auto Callee = BC.getFunctionForSymbol(CalleeSymbol, &EntryID);
if (Callee) {
CSI.DestId = Callee->getFunctionNumber();;
CSI.EntryDiscriminator = Callee->getEntryForSymbol(CalleeSymbol);
CSI.EntryDiscriminator = EntryID;
}
if (BC.MIB->getConditionalTailCall(Instr)) {