[BOLT-AArch64] Create cold symbols on demand

Summary:
Rework the logic we use for managing references to constant
islands. Defer the creation of the cold versions to when we split the
function and will need them.

(cherry picked from FBD8228803)
This commit is contained in:
Rafael Auler 2018-05-31 10:33:53 -07:00 committed by Maksim Panchenko
parent 44a36937f8
commit 7aee0adbf9
2 changed files with 68 additions and 76 deletions

View File

@ -942,7 +942,7 @@ MCSymbol *BinaryFunction::getOrCreateLocalLabel(uint64_t Address,
return LI->second; return LI->second;
// For AArch64, check if this address is part of a constant island. // For AArch64, check if this address is part of a constant island.
if (MCSymbol *IslandSym = getOrCreateIslandAccess(Address).first) { if (MCSymbol *IslandSym = getOrCreateIslandAccess(Address)) {
return IslandSym; return IslandSym;
} }
@ -976,7 +976,7 @@ void BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
if (BC.isAArch64()) { if (BC.isAArch64()) {
// Check if this is an access to a constant island and create bookkeeping // Check if this is an access to a constant island and create bookkeeping
// to keep track of it and emit it later as part of this function // to keep track of it and emit it later as part of this function
if (MCSymbol *IslandSym = getOrCreateIslandAccess(TargetAddress).first) { if (MCSymbol *IslandSym = getOrCreateIslandAccess(TargetAddress)) {
return IslandSym; return IslandSym;
} else { } else {
// Detect custom code written in assembly that refers to arbitrary // Detect custom code written in assembly that refers to arbitrary
@ -986,13 +986,15 @@ void BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {
auto IslandIter = auto IslandIter =
BC.AddressToConstantIslandMap.lower_bound(TargetAddress); BC.AddressToConstantIslandMap.lower_bound(TargetAddress);
if (IslandIter != BC.AddressToConstantIslandMap.end()) { if (IslandIter != BC.AddressToConstantIslandMap.end()) {
MCSymbol *IslandSym, *ColdIslandSym; if (MCSymbol *IslandSym =
std::tie(IslandSym, ColdIslandSym) = IslandIter->second->getOrCreateProxyIslandAccess(
IslandIter->second->getOrCreateProxyIslandAccess(TargetAddress, TargetAddress, this)) {
this); /// Make this function depend on IslandIter->second because we have
if (IslandSym) { /// a reference to its constant island. When emitting this function,
addConstantIslandDependency(IslandIter->second, IslandSym, /// we will also emit IslandIter->second's constants. This only
ColdIslandSym); /// happens in custom AArch64 assembly code.
IslandDependency.insert(IslandIter->second);
ProxyIslandSymbols[IslandSym] = IslandIter->second;
return IslandSym; return IslandSym;
} }
} }
@ -2434,18 +2436,6 @@ void BinaryFunction::setTrapOnEntry() {
TrapsOnEntry = true; TrapsOnEntry = true;
} }
void BinaryFunction::addConstantIslandDependency(BinaryFunction *OtherBF,
MCSymbol *HotSymbol,
MCSymbol *ColdSymbol) {
IslandDependency.insert(OtherBF);
if (!ColdIslandSymbols.count(HotSymbol)) {
ColdIslandSymbols[HotSymbol] = ColdSymbol;
}
DEBUG(dbgs() << "BOLT-DEBUG: Constant island dependency added! "
<< getPrintName() << " refers to " << OtherBF->getPrintName()
<< "\n");
}
void BinaryFunction::emitConstantIslands( void BinaryFunction::emitConstantIslands(
MCStreamer &Streamer, bool EmitColdPart, MCStreamer &Streamer, bool EmitColdPart,
BinaryFunction *OnBehalfOf) { BinaryFunction *OnBehalfOf) {
@ -2474,7 +2464,7 @@ void BinaryFunction::emitConstantIslands(
<< "\n"; << "\n";
// We split the island into smaller blocks and output labels between them. // We split the island into smaller blocks and output labels between them.
auto IS = IslandSymbols.begin(); auto IS = IslandOffsets.begin();
for (auto DataIter = DataOffsets.begin(); DataIter != DataOffsets.end(); for (auto DataIter = DataOffsets.begin(); DataIter != DataOffsets.end();
++DataIter) { ++DataIter) {
uint64_t FunctionOffset = *DataIter; uint64_t FunctionOffset = *DataIter;
@ -2498,9 +2488,9 @@ void BinaryFunction::emitConstantIslands(
// Emit labels, relocs and data // Emit labels, relocs and data
auto RI = MoveRelocations.lower_bound(FunctionOffset); auto RI = MoveRelocations.lower_bound(FunctionOffset);
while ((IS != IslandSymbols.end() && IS->first < EndOffset) || while ((IS != IslandOffsets.end() && IS->first < EndOffset) ||
(RI != MoveRelocations.end() && RI->first < EndOffset)) { (RI != MoveRelocations.end() && RI->first < EndOffset)) {
auto NextLabelOffset = IS == IslandSymbols.end() ? EndOffset : IS->first; auto NextLabelOffset = IS == IslandOffsets.end() ? EndOffset : IS->first;
auto NextRelOffset = RI == MoveRelocations.end() ? EndOffset : RI->first; auto NextRelOffset = RI == MoveRelocations.end() ? EndOffset : RI->first;
auto NextStop = std::min(NextLabelOffset, NextRelOffset); auto NextStop = std::min(NextLabelOffset, NextRelOffset);
assert(NextStop <= EndOffset && "internal overflow error"); assert(NextStop <= EndOffset && "internal overflow error");
@ -2508,7 +2498,7 @@ void BinaryFunction::emitConstantIslands(
Streamer.EmitBytes(FunctionContents.slice(FunctionOffset, NextStop)); Streamer.EmitBytes(FunctionContents.slice(FunctionOffset, NextStop));
FunctionOffset = NextStop; FunctionOffset = NextStop;
} }
if (IS != IslandSymbols.end() && FunctionOffset == IS->first) { if (IS != IslandOffsets.end() && FunctionOffset == IS->first) {
// This is a slightly complex code to decide which label to emit. We // This is a slightly complex code to decide which label to emit. We
// have 4 cases to handle: regular symbol, cold symbol, regular or cold // have 4 cases to handle: regular symbol, cold symbol, regular or cold
// symbol being emitted on behalf of an external function. // symbol being emitted on behalf of an external function.
@ -2521,7 +2511,7 @@ void BinaryFunction::emitConstantIslands(
Streamer.EmitLabel(IS->second); Streamer.EmitLabel(IS->second);
else else
assert(hasName(IS->second->getName())); assert(hasName(IS->second->getName()));
} else { } else if (ColdIslandSymbols.count(IS->second) != 0) {
DEBUG(dbgs() << "BOLT-DEBUG: emitted label " DEBUG(dbgs() << "BOLT-DEBUG: emitted label "
<< ColdIslandSymbols[IS->second]->getName() << '\n'); << ColdIslandSymbols[IS->second]->getName() << '\n');
if (ColdIslandSymbols[IS->second]->isUndefined()) if (ColdIslandSymbols[IS->second]->isUndefined())
@ -2534,15 +2524,13 @@ void BinaryFunction::emitConstantIslands(
<< '\n'); << '\n');
Streamer.EmitLabel(Sym); Streamer.EmitLabel(Sym);
} }
} else { } else if (MCSymbol *Sym =
if (MCSymbol *Sym = ColdIslandProxies[OnBehalfOf][IS->second]) {
IslandProxies[OnBehalfOf][ColdIslandSymbols[IS->second]]) {
DEBUG(dbgs() << "BOLT-DEBUG: emitted label " << Sym->getName() DEBUG(dbgs() << "BOLT-DEBUG: emitted label " << Sym->getName()
<< '\n'); << '\n');
Streamer.EmitLabel(Sym); Streamer.EmitLabel(Sym);
} }
} }
}
++IS; ++IS;
} }
if (RI != MoveRelocations.end() && FunctionOffset == RI->first) { if (RI != MoveRelocations.end() && FunctionOffset == RI->first) {
@ -2560,7 +2548,7 @@ void BinaryFunction::emitConstantIslands(
Streamer.EmitBytes(FunctionContents.slice(FunctionOffset, EndOffset)); Streamer.EmitBytes(FunctionContents.slice(FunctionOffset, EndOffset));
} }
} }
assert(IS == IslandSymbols.end() && "some symbols were not emitted!"); assert(IS == IslandOffsets.end() && "some symbols were not emitted!");
if (OnBehalfOf) if (OnBehalfOf)
return; return;
@ -2584,13 +2572,31 @@ void BinaryFunction::duplicateConstantIslands() {
++OpNum; ++OpNum;
continue; continue;
} }
const auto *Symbol = BC.MIB->getTargetSymbol(Inst, OpNum); auto *Symbol = BC.MIB->getTargetSymbol(Inst, OpNum);
auto ISym = ColdIslandSymbols.find(Symbol); // Check if this is an island symbol
if (ISym == ColdIslandSymbols.end()) if (!IslandSymbols.count(Symbol) && !ProxyIslandSymbols.count(Symbol))
continue; continue;
// Create cold symbol, if missing
auto ISym = ColdIslandSymbols.find(Symbol);
MCSymbol *ColdSymbol;
if (ISym != ColdIslandSymbols.end()) {
ColdSymbol = ISym->second;
} else {
ColdSymbol = BC.Ctx->getOrCreateSymbol(Symbol->getName() + ".cold");
ColdIslandSymbols[Symbol] = ColdSymbol;
// Check if this is a proxy island symbol and update owner proxy map
if (ProxyIslandSymbols.count(Symbol)) {
BinaryFunction *Owner = ProxyIslandSymbols[Symbol];
auto IProxiedSym = Owner->IslandProxies[this].find(Symbol);
Owner->ColdIslandProxies[this][IProxiedSym->second] = ColdSymbol;
}
}
// Update instruction reference
Operand = MCOperand::createExpr(BC.MIB->getTargetExprFor( Operand = MCOperand::createExpr(BC.MIB->getTargetExprFor(
Inst, Inst,
MCSymbolRefExpr::create(ISym->second, MCSymbolRefExpr::VK_None, MCSymbolRefExpr::create(ColdSymbol, MCSymbolRefExpr::VK_None,
*BC.Ctx), *BC.Ctx),
*BC.Ctx, 0)); *BC.Ctx, 0));
++OpNum; ++OpNum;

View File

@ -556,12 +556,14 @@ private:
/// Offsets in function that are data values in a constant island identified /// Offsets in function that are data values in a constant island identified
/// after disassembling /// after disassembling
std::map<uint64_t, MCSymbol *> IslandSymbols; std::map<uint64_t, MCSymbol *> IslandOffsets;
SmallPtrSet<MCSymbol *, 4> IslandSymbols;
std::map<const MCSymbol *, BinaryFunction *> ProxyIslandSymbols;
std::map<const MCSymbol *, MCSymbol *> ColdIslandSymbols; std::map<const MCSymbol *, MCSymbol *> ColdIslandSymbols;
/// Keeps track of other functions we depend on because there is a reference /// Keeps track of other functions we depend on because there is a reference
/// to the constant islands in them. /// to the constant islands in them.
std::map<BinaryFunction *, std::map<const MCSymbol *, MCSymbol *>> std::map<BinaryFunction *, std::map<const MCSymbol *, MCSymbol *>>
IslandProxies; IslandProxies, ColdIslandProxies;
std::set<BinaryFunction *> IslandDependency; // The other way around std::set<BinaryFunction *> IslandDependency; // The other way around
// Blocks are kept sorted in the layout order. If we need to change the // Blocks are kept sorted in the layout order. If we need to change the
@ -1730,62 +1732,46 @@ public:
/// hot code area while the second return value is the symbol for reference /// hot code area while the second return value is the symbol for reference
/// in the cold code area, as when the function is split the islands are /// in the cold code area, as when the function is split the islands are
/// duplicated. /// duplicated.
std::pair<MCSymbol *, MCSymbol *> getOrCreateIslandAccess(uint64_t Address) { MCSymbol *getOrCreateIslandAccess(uint64_t Address) {
MCSymbol *Symbol, *ColdSymbol; MCSymbol *Symbol;
if (!isInConstantIsland(Address)) if (!isInConstantIsland(Address))
return std::make_pair(nullptr, nullptr); return nullptr;
// Register our island at global namespace // Register our island at global namespace
Symbol = BC.getOrCreateGlobalSymbol(Address, 0, 0, "ISLANDat"); Symbol = BC.getOrCreateGlobalSymbol(Address, 0, 0, "ISLANDat");
// Internal bookkeeping // Internal bookkeeping
const auto Offset = Address - getAddress(); const auto Offset = Address - getAddress();
assert((!IslandSymbols.count(Offset) || IslandSymbols[Offset] == Symbol) && assert((!IslandOffsets.count(Offset) || IslandOffsets[Offset] == Symbol) &&
"Inconsistent island symbol management"); "Inconsistent island symbol management");
if (!IslandSymbols.count(Offset)) { if (!IslandOffsets.count(Offset)) {
IslandSymbols[Offset] = Symbol; IslandOffsets[Offset] = Symbol;
IslandSymbols.insert(Symbol);
} }
if (!ColdIslandSymbols.count(Symbol)) { return Symbol;
ColdSymbol = BC.Ctx->getOrCreateSymbol(Symbol->getName() + ".cold");
ColdIslandSymbols[Symbol] = ColdSymbol;
} else {
ColdSymbol = ColdIslandSymbols[Symbol];
}
return std::make_pair(Symbol, ColdSymbol);
} }
/// Called by an external function which wishes to emit references to constant /// Called by an external function which wishes to emit references to constant
/// island symbols of this function. We create a proxy for it, so we emit /// island symbols of this function. We create a proxy for it, so we emit
/// separate symbols when emitting our constant island on behalf of this other /// separate symbols when emitting our constant island on behalf of this other
/// function. /// function.
std::pair<MCSymbol *, MCSymbol *> MCSymbol *
getOrCreateProxyIslandAccess(uint64_t Address, BinaryFunction *Referrer) { getOrCreateProxyIslandAccess(uint64_t Address, BinaryFunction *Referrer) {
auto HotColdSymbols = getOrCreateIslandAccess(Address); auto Symbol = getOrCreateIslandAccess(Address);
if (!HotColdSymbols.first) if (!Symbol)
return HotColdSymbols; return nullptr;
MCSymbol *ProxyHot, *ProxyCold; MCSymbol *Proxy;
if (!IslandProxies[Referrer].count(HotColdSymbols.first)) { if (!IslandProxies[Referrer].count(Symbol)) {
ProxyHot = Proxy =
BC.Ctx->getOrCreateSymbol(HotColdSymbols.first->getName() + BC.Ctx->getOrCreateSymbol(Symbol->getName() +
".proxy.for." + Referrer->getPrintName()); ".proxy.for." + Referrer->getPrintName());
ProxyCold = IslandProxies[Referrer][Symbol] = Proxy;
BC.Ctx->getOrCreateSymbol(HotColdSymbols.second->getName() + IslandProxies[Referrer][Proxy] = Symbol;
".proxy.for." + Referrer->getPrintName());
IslandProxies[Referrer][HotColdSymbols.first] = ProxyHot;
IslandProxies[Referrer][HotColdSymbols.second] = ProxyCold;
} }
ProxyHot = IslandProxies[Referrer][HotColdSymbols.first]; Proxy = IslandProxies[Referrer][Symbol];
ProxyCold = IslandProxies[Referrer][HotColdSymbols.second]; return Proxy;
return std::make_pair(ProxyHot, ProxyCold);
} }
/// Make this function depend on \p OtherBF because we have a reference to its
/// constant island. When emitting this function, we will also emit OtherBF's
/// constants. This only happens in custom AArch64 assembly code (either
/// poorly written code or over-optimized).
void addConstantIslandDependency(BinaryFunction *OtherBF, MCSymbol *HotSymbol,
MCSymbol *ColdSymbol);
/// Detects whether \p Address is inside a data region in this function /// Detects whether \p Address is inside a data region in this function
/// (constant islands). /// (constant islands).
bool isInConstantIsland(uint64_t Address) const { bool isInConstantIsland(uint64_t Address) const {