Skip to content

Commit

Permalink
[BOLT][Refactoring] Isolate changes to MC layer
Browse files Browse the repository at this point in the history
Summary:
Changes that we made to MCInst, MCOperand, MCExpr, etc. are now all
moved into tools/llvm-bolt. That required a change to the way we handle
annotations and any extra operands for MCInst.

Any MCPlus information is now attached via an extra operand of type
MCInst with an opcode ANNOTATION_LABEL. Since this operand is MCInst, we
attach extra info as operands to this instruction. For first-level
annotations use functions to access the information, such as
getConditionalTailCall() or getEHInfo(), etc. For the rest, optional or
second-class annotations, use a general named-annotation interface such
as getAnnotationAs<uint64_t>(Inst, "Count").

I did a test on HHVM binary, and a memory consumption went down a little
bit while the runtime remained the same.

(cherry picked from FBD7405412)
  • Loading branch information
maksfb committed Mar 20, 2018
1 parent 0dea337 commit a62f4fd
Show file tree
Hide file tree
Showing 29 changed files with 747 additions and 546 deletions.
25 changes: 9 additions & 16 deletions bolt/BinaryContext.cpp
Expand Up @@ -686,15 +686,14 @@ void BinaryContext::printInstruction(raw_ostream &OS,
if (MIB->isTailCall(Instruction))
OS << " # TAILCALL ";
if (MIB->isInvoke(Instruction)) {
const MCSymbol *LP;
uint64_t Action;
std::tie(LP, Action) = MIB->getEHInfo(Instruction);
OS << " # handler: ";
if (LP)
OS << *LP;
else
OS << '0';
OS << "; action: " << Action;
if (const auto EHInfo = MIB->getEHInfo(Instruction)) {
OS << " # handler: ";
if (EHInfo->first)
OS << *EHInfo->first;
else
OS << '0';
OS << "; action: " << EHInfo->second;
}
auto GnuArgsSize = MIB->getGnuArgsSize(Instruction);
if (GnuArgsSize >= 0)
OS << "; GNU_args_size = " << GnuArgsSize;
Expand All @@ -706,13 +705,7 @@ void BinaryContext::printInstruction(raw_ostream &OS,
}
}

MIB->forEachAnnotation(
Instruction,
[&OS](const MCAnnotation *Annotation) {
OS << " # " << Annotation->getName() << ": ";
Annotation->print(OS);
}
);
MIB->printAnnotations(Instruction, OS);

const DWARFDebugLine::LineTable *LineTable =
Function && opts::PrintDebugInfo ? Function->getDWARFUnitLineTable().second
Expand Down
4 changes: 2 additions & 2 deletions bolt/BinaryContext.h
Expand Up @@ -223,7 +223,7 @@ class BinaryContext {

std::unique_ptr<const MCInstrAnalysis> MIA;

std::unique_ptr<const MCPlusBuilder> MIB;
std::unique_ptr<MCPlusBuilder> MIB;

std::unique_ptr<const MCRegisterInfo> MRI;

Expand Down Expand Up @@ -273,7 +273,7 @@ class BinaryContext {
std::unique_ptr<const MCSubtargetInfo> STI,
std::unique_ptr<MCInstPrinter> InstPrinter,
std::unique_ptr<const MCInstrAnalysis> MIA,
std::unique_ptr<const MCPlusBuilder> MIB,
std::unique_ptr<MCPlusBuilder> MIB,
std::unique_ptr<const MCRegisterInfo> MRI,
std::unique_ptr<MCDisassembler> DisAsm,
DataReader &DR) :
Expand Down
44 changes: 15 additions & 29 deletions bolt/BinaryFunction.cpp
Expand Up @@ -765,18 +765,19 @@ IndirectBranchType BinaryFunction::processIndirectBranch(MCInst &Instruction,
// Get or create a new label for the table.
auto LI = JT->Labels.find(JTOffset);
if (LI == JT->Labels.end()) {
auto *JTStartLabel = BC.registerNameAtAddress(generateJumpTableName(ArrayStart),
ArrayStart,
0,
JT->EntrySize);
auto *JTStartLabel =
BC.registerNameAtAddress(generateJumpTableName(ArrayStart),
ArrayStart,
0,
JT->EntrySize);
auto Result = JT->Labels.emplace(JTOffset, JTStartLabel);
assert(Result.second && "error adding jump table label");
LI = Result.first;
}

BC.MIB->replaceMemOperandDisp(const_cast<MCInst &>(*MemLocInstr),
LI->second, BC.Ctx.get());
BC.MIB->setJumpTable(BC.Ctx.get(), Instruction, ArrayStart, IndexRegNum);
BC.MIB->setJumpTable(Instruction, ArrayStart, IndexRegNum);

JTSites.emplace_back(Offset, ArrayStart);

Expand Down Expand Up @@ -873,7 +874,7 @@ IndirectBranchType BinaryFunction::processIndirectBranch(MCInst &Instruction,
JumpTables.emplace(ArrayStart, JT.release());
BC.MIB->replaceMemOperandDisp(const_cast<MCInst &>(*MemLocInstr),
JTStartLabel, BC.Ctx.get());
BC.MIB->setJumpTable(BC.Ctx.get(), Instruction, ArrayStart, IndexRegNum);
BC.MIB->setJumpTable(Instruction, ArrayStart, IndexRegNum);

JTSites.emplace_back(Offset, ArrayStart);

Expand Down Expand Up @@ -1328,11 +1329,11 @@ void BinaryFunction::disassemble(ArrayRef<uint8_t> FunctionData) {

// Record offset of the instruction for profile matching.
if (BC.keepOffsetForInstruction(Instruction)) {
MIB->addAnnotation(Ctx.get(), Instruction, "Offset", Offset);
MIB->addAnnotation(Instruction, "Offset", Offset);
}

if (MemData && !emptyRange(MemData->getMemInfoRange(Offset))) {
MIB->addAnnotation(Ctx.get(), Instruction, "MemDataOffset", Offset);
MIB->addAnnotation(Instruction, "MemDataOffset", Offset);
}

addInstruction(Offset, std::move(Instruction));
Expand Down Expand Up @@ -1514,13 +1515,11 @@ void BinaryFunction::recomputeLandingPads() {
if (!BC.MIB->isInvoke(Instr))
continue;

const MCSymbol *LPLabel;
uint64_t Action;
std::tie(LPLabel, Action) = BC.MIB->getEHInfo(Instr);
if (!LPLabel)
const auto EHInfo = BC.MIB->getEHInfo(Instr);
if (!EHInfo || !EHInfo->first)
continue;

auto *LPBlock = getBasicBlockForLabel(LPLabel);
auto *LPBlock = getBasicBlockForLabel(EHInfo->first);
if (!BBLandingPads.count(LPBlock)) {
BBLandingPads.insert(LPBlock);
BB->LandingPads.emplace_back(LPBlock);
Expand Down Expand Up @@ -1583,7 +1582,7 @@ bool BinaryFunction::buildCFG() {
assert(PrevBB && PrevBB != InsertBB && "invalid previous block");
auto *PrevInstr = PrevBB->getLastNonPseudoInstr();
if (PrevInstr && !MIB->hasAnnotation(*PrevInstr, "Offset"))
MIB->addAnnotation(BC.Ctx.get(), *PrevInstr, "Offset", Offset);
MIB->addAnnotation(*PrevInstr, "Offset", Offset);
};

for (auto I = Instructions.begin(), E = Instructions.end(); I != E; ++I) {
Expand Down Expand Up @@ -1887,10 +1886,8 @@ void BinaryFunction::removeConditionalTailCalls() {
}

// Assert that the tail call does not throw.
const MCSymbol *LP;
uint64_t Action;
std::tie(LP, Action) = BC.MIB->getEHInfo(*CTCInstr);
assert(!LP && "found tail call with associated landing pad");
assert(!BC.MIB->getEHInfo(*CTCInstr) &&
"found tail call with associated landing pad");

// Create a basic block with an unconditional tail call instruction using
// the same destination.
Expand Down Expand Up @@ -2159,7 +2156,6 @@ void BinaryFunction::emitBody(MCStreamer &Streamer, bool EmitColdPart) {
if (EmitColdPart && hasConstantIsland())
duplicateConstantIslands();

int64_t CurrentGnuArgsSize = 0;
for (auto BB : layout()) {
if (EmitColdPart != BB->isCold())
continue;
Expand Down Expand Up @@ -2187,16 +2183,6 @@ void BinaryFunction::emitBody(MCStreamer &Streamer, bool EmitColdPart) {
LastLocSeen = emitLineInfo(Instr.getLoc(), LastLocSeen);
}

// Emit GNU_args_size CFIs as necessary.
if (usesGnuArgsSize() && BC.MIB->isInvoke(Instr)) {
auto NewGnuArgsSize = BC.MIB->getGnuArgsSize(Instr);
assert(NewGnuArgsSize >= 0 && "expected non-negative GNU_args_size");
if (NewGnuArgsSize != CurrentGnuArgsSize) {
CurrentGnuArgsSize = NewGnuArgsSize;
Streamer.EmitCFIGnuArgsSize(CurrentGnuArgsSize);
}
}

Streamer.EmitInstruction(Instr, *BC.STI);
}
}
Expand Down
51 changes: 27 additions & 24 deletions bolt/BinaryFunction.h
Expand Up @@ -23,6 +23,7 @@
#include "DataReader.h"
#include "DebugData.h"
#include "JumpTable.h"
#include "MCPlus.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/ilist.h"
#include "llvm/ADT/iterator.h"
Expand Down Expand Up @@ -426,7 +427,7 @@ class BinaryFunction {
//
// * instructions with landing pads
//
// Most of the common cases should be handled by MCInst::equals()
// Most of the common cases should be handled by MCPlus::equals()
// that compares regular instruction operands.
//
// NB: there's no need to compare jump table indirect jump instructions
Expand All @@ -435,23 +436,33 @@ class BinaryFunction {
const auto EHInfoA = BC.MIB->getEHInfo(InstA);
const auto EHInfoB = BC.MIB->getEHInfo(InstB);

// Action indices should match.
if (EHInfoA.second != EHInfoB.second)
return false;
if (EHInfoA || EHInfoB) {
if (!EHInfoA && (EHInfoB->first || EHInfoB->second))
return false;

if (!EHInfoA.first != !EHInfoB.first)
return false;
if (!EHInfoB && (EHInfoA->first || EHInfoA->second))
return false;

if (EHInfoA.first && EHInfoB.first) {
const auto *LPA = BBA.getLandingPad(EHInfoA.first);
const auto *LPB = BBB.getLandingPad(EHInfoB.first);
assert(LPA && LPB && "cannot locate landing pad(s)");
if (EHInfoA && EHInfoB) {
// Action indices should match.
if (EHInfoA->second != EHInfoB->second)
return false;

if (LPA->getLayoutIndex() != LPB->getLayoutIndex())
return false;
if (!EHInfoA->first != !EHInfoB->first)
return false;

if (EHInfoA->first && EHInfoB->first) {
const auto *LPA = BBA.getLandingPad(EHInfoA->first);
const auto *LPB = BBB.getLandingPad(EHInfoB->first);
assert(LPA && LPB && "cannot locate landing pad(s)");

if (LPA->getLayoutIndex() != LPB->getLayoutIndex())
return false;
}
}
}

return InstA.equals(InstB, Comp);
return MCPlus::equals(InstA, InstB, Comp);
}

/// Recompute landing pad information for the function and all its blocks.
Expand Down Expand Up @@ -928,9 +939,9 @@ class BinaryFunction {
BinaryBasicBlock *getLandingPadBBFor(const BinaryBasicBlock &BB,
const MCInst &InvokeInst) {
assert(BC.MIB->isInvoke(InvokeInst) && "must be invoke instruction");
MCLandingPad LP = BC.MIB->getEHInfo(InvokeInst);
if (LP.first) {
auto *LBB = BB.getLandingPad(LP.first);
const auto LP = BC.MIB->getEHInfo(InvokeInst);
if (LP && LP->first) {
auto *LBB = BB.getLandingPad(LP->first);
assert (LBB && "Landing pad should be defined");
return LBB;
}
Expand Down Expand Up @@ -2374,14 +2385,6 @@ template <> struct GraphTraits<Inverse<const bolt::BinaryFunction *>> :
}
};

template <>
class MCAnnotationPrinter<bolt::IndirectCallSiteProfile> {
public:
void print(raw_ostream &OS, const bolt::IndirectCallSiteProfile &ICSP) const {
OS << ICSP;
}
};

} // namespace llvm

#endif
5 changes: 2 additions & 3 deletions bolt/BinaryFunctionProfile.cpp
Expand Up @@ -503,14 +503,13 @@ void BinaryFunction::convertBranchData() {
<< Twine::utohexstr(BI.From.Offset)
<< " in function " << *this << '\n';
}
auto &Value = BC.MIB->getOrCreateAnnotationAs<uint64_t>(BC.Ctx.get(),
*Instr, Name);
auto &Value = BC.MIB->getOrCreateAnnotationAs<uint64_t>(*Instr, Name);
Value += Count;
};

if (BC.MIB->isIndirectCall(*Instr) || BC.MIB->isIndirectBranch(*Instr)) {
IndirectCallSiteProfile &CSP =
BC.MIB->getOrCreateAnnotationAs<IndirectCallSiteProfile>(BC.Ctx.get(),
BC.MIB->getOrCreateAnnotationAs<IndirectCallSiteProfile>(
*Instr, "CallProfile");
CSP.emplace_back(BI.To.IsSymbol, BI.To.Name, BI.Branches,
BI.Mispreds);
Expand Down
2 changes: 1 addition & 1 deletion bolt/BinaryPassManager.cpp
Expand Up @@ -450,7 +450,7 @@ void BinaryFunctionPassManager::runAllPasses(
Manager.registerPass(
llvm::make_unique<InstructionLowering>(PrintAfterLowering));

Manager.registerPass(llvm::make_unique<StripAnnotations>(NeverPrint));
Manager.registerPass(llvm::make_unique<LowerAnnotations>(NeverPrint));

Manager.runPasses();
}
Expand Down
12 changes: 5 additions & 7 deletions bolt/Exceptions.cpp
Expand Up @@ -243,8 +243,7 @@ void BinaryFunction::parseLSDA(ArrayRef<uint8_t> LSDASectionData,
// Add extra operands to a call instruction making it an invoke from
// now on.
BC.MIB->addEHInfo(Instruction,
MCLandingPad(LPSymbol, ActionEntry),
BC.Ctx.get());
MCPlus::MCLandingPad(LPSymbol, ActionEntry));
}
++II;
} while (II != IE && II->first < Start + Length);
Expand Down Expand Up @@ -406,13 +405,11 @@ void BinaryFunction::updateEHRanges() {
}

for (auto II = BB->begin(); II != BB->end(); ++II) {
auto Instr = *II;

if (!BC.MIB->isCall(Instr))
if (!BC.MIB->isCall(*II))
continue;

// Instruction can throw an exception that should be handled.
const bool Throws = BC.MIB->isInvoke(Instr);
const bool Throws = BC.MIB->isInvoke(*II);

// Ignore the call if it's a continuation of a no-throw gap.
if (!Throws && !StartRange)
Expand All @@ -421,7 +418,8 @@ void BinaryFunction::updateEHRanges() {
// Extract exception handling information from the instruction.
const MCSymbol *LP = nullptr;
uint64_t Action = 0;
std::tie(LP, Action) = BC.MIB->getEHInfo(Instr);
if (const auto EHInfo = BC.MIB->getEHInfo(*II))
std::tie(LP, Action) = *EHInfo;

// No action if the exception handler has not changed.
if (Throws &&
Expand Down

0 comments on commit a62f4fd

Please sign in to comment.