Skip to content

Commit

Permalink
[BOLT] Modify MCPlus annotation internals. NFCI. (llvm#70412)
Browse files Browse the repository at this point in the history 
When annotating MCInst instructions, attach extra annotation operands
directly to the annotated instruction, instead of attaching them to an
instruction pointed to by a special kInst operand.

With this change, it's no longer necessary to allocate MCInst and most
of the first-class annotations come with free memory as currently MCInst
is declared with:

    SmallVector<MCOperand, 10> Operands;

i.e. more operands than are normally being used.

We still create a kInst operand with a nullptr instruction value to
designate the beginning of annotation operands. However, this special
operand might not be needed if we can rely on MCInstrDesc::NumOperands.
  • Loading branch information
@maksfb
maksfb authored Nov 6, 2023
1 parent cc05b47 commit 74e0a26
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 114 deletions.
26 changes: 16 additions & 10 deletions bolt/include/bolt/Core/MCPlus.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,16 @@ namespace MCPlus {
/// pad and the uint64_t represents the action.
using MCLandingPad = std::pair<const MCSymbol *, uint64_t>;

/// An extension to MCInst is provided via an extra operand of type MCInst with
/// ANNOTATION_LABEL opcode (i.e. we are tying an annotation instruction to an
/// existing one). The annotation instruction contains a list of Immediate
/// operands. Each operand either contains a value, or is a pointer to
/// an instance of class MCAnnotation.
/// An extension to MCInst is provided via extra operands, i.e. operands that
/// are not used in the instruction assembly. Any kind of metadata can be
/// attached to MCInst with this "annotation" extension using MCPlusBuilder
/// interface.
//
/// The first extra operand must be of type kInst with an empty (nullptr)
/// value. The kInst operand type is unused on most non-VLIW architectures.
/// We use it to mark the beginning of annotations operands. The rest of the
/// operands are of Immediate type with annotation info encoded into the value
/// of the immediate.
///
/// There are 2 distinct groups of annotations. The first group is a first-class
/// annotation that affects semantics of the instruction, such as an
Expand All @@ -55,7 +60,7 @@ using MCLandingPad = std::pair<const MCSymbol *, uint64_t>;
/// of their corresponding operand.
///
/// Annotations in the second group could be addressed either by name, or by
/// by and index which could be queried by providing a name.
/// by index which could be queried by providing the name.
class MCAnnotation {
public:
enum Kind {
Expand Down Expand Up @@ -106,10 +111,11 @@ template <typename ValueType> class MCSimpleAnnotation : public MCAnnotation {
/// Return a number of operands in \Inst excluding operands representing
/// annotations.
inline unsigned getNumPrimeOperands(const MCInst &Inst) {
if (Inst.getNumOperands() > 0 && std::prev(Inst.end())->isInst()) {
assert(std::prev(Inst.end())->getInst()->getOpcode() ==
TargetOpcode::ANNOTATION_LABEL);
return Inst.getNumOperands() - 1;
for (signed I = Inst.getNumOperands() - 1; I >= 0; --I) {
if (Inst.getOperand(I).isInst())
return I;
if (!Inst.getOperand(I).isImm())
return Inst.getNumOperands();
}
return Inst.getNumOperands();
}
Expand Down
113 changes: 54 additions & 59 deletions bolt/include/bolt/Core/MCPlusBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class MCPlusBuilder {
private:
/// A struct that represents a single annotation allocator
struct AnnotationAllocator {
SpecificBumpPtrAllocator<MCInst> MCInstAllocator;
BumpPtrAllocator ValueAllocator;
std::unordered_set<MCPlus::MCAnnotation *> AnnotationPool;
};
Expand Down Expand Up @@ -97,60 +96,62 @@ class MCPlusBuilder {
return SignExtend64<56>(ImmValue & 0xff'ffff'ffff'ffffULL);
}

MCInst *getAnnotationInst(const MCInst &Inst) const {
if (Inst.getNumOperands() == 0)
return nullptr;
std::optional<unsigned> getFirstAnnotationOpIndex(const MCInst &Inst) const {
const unsigned NumPrimeOperands = MCPlus::getNumPrimeOperands(Inst);
if (Inst.getNumOperands() == NumPrimeOperands)
return std::nullopt;

const MCOperand &LastOp = Inst.getOperand(Inst.getNumOperands() - 1);
if (!LastOp.isInst())
return nullptr;
assert(Inst.getOperand(NumPrimeOperands).getInst() == nullptr &&
"Empty instruction expected.");

MCInst *AnnotationInst = const_cast<MCInst *>(LastOp.getInst());
assert(AnnotationInst->getOpcode() == TargetOpcode::ANNOTATION_LABEL);
return NumPrimeOperands + 1;
}

return AnnotationInst;
MCInst::iterator getAnnotationInstOp(MCInst &Inst) const {
for (MCInst::iterator Iter = Inst.begin(); Iter != Inst.end(); ++Iter) {
if (Iter->isInst()) {
assert(Iter->getInst() == nullptr && "Empty instruction expected.");
return Iter;
}
}
return Inst.end();
}

void removeAnnotationInst(MCInst &Inst) const {
assert(getAnnotationInst(Inst) && "Expected annotation instruction.");
Inst.erase(std::prev(Inst.end()));
assert(!getAnnotationInst(Inst) &&
"More than one annotation instruction detected.");
void removeAnnotations(MCInst &Inst) const {
Inst.erase(getAnnotationInstOp(Inst), Inst.end());
}

void setAnnotationOpValue(MCInst &Inst, unsigned Index, int64_t Value,
AllocatorIdTy AllocatorId = 0) {
MCInst *AnnotationInst = getAnnotationInst(Inst);
if (!AnnotationInst) {
AnnotationAllocator &Allocator = getAnnotationAllocator(AllocatorId);
AnnotationInst = new (Allocator.MCInstAllocator.Allocate()) MCInst();
AnnotationInst->setOpcode(TargetOpcode::ANNOTATION_LABEL);
Inst.addOperand(MCOperand::createInst(AnnotationInst));
void setAnnotationOpValue(MCInst &Inst, unsigned Index, int64_t Value) const {
const int64_t AnnotationValue = encodeAnnotationImm(Index, Value);
const std::optional<unsigned> FirstAnnotationOp =
getFirstAnnotationOpIndex(Inst);
if (!FirstAnnotationOp) {
Inst.addOperand(MCOperand::createInst(nullptr));
Inst.addOperand(MCOperand::createImm(AnnotationValue));
return;
}

const int64_t AnnotationValue = encodeAnnotationImm(Index, Value);
for (int I = AnnotationInst->getNumOperands() - 1; I >= 0; --I) {
int64_t ImmValue = AnnotationInst->getOperand(I).getImm();
for (unsigned I = *FirstAnnotationOp; I < Inst.getNumOperands(); ++I) {
const int64_t ImmValue = Inst.getOperand(I).getImm();
if (extractAnnotationIndex(ImmValue) == Index) {
AnnotationInst->getOperand(I).setImm(AnnotationValue);
Inst.getOperand(I).setImm(AnnotationValue);
return;
}
}

AnnotationInst->addOperand(MCOperand::createImm(AnnotationValue));
Inst.addOperand(MCOperand::createImm(AnnotationValue));
}

std::optional<int64_t> getAnnotationOpValue(const MCInst &Inst,
unsigned Index) const {
const MCInst *AnnotationInst = getAnnotationInst(Inst);
if (!AnnotationInst)
std::optional<unsigned> FirstAnnotationOp = getFirstAnnotationOpIndex(Inst);
if (!FirstAnnotationOp)
return std::nullopt;

for (int I = AnnotationInst->getNumOperands() - 1; I >= 0; --I) {
int64_t ImmValue = AnnotationInst->getOperand(I).getImm();
if (extractAnnotationIndex(ImmValue) == Index) {
for (unsigned I = *FirstAnnotationOp; I < Inst.getNumOperands(); ++I) {
const int64_t ImmValue = Inst.getOperand(I).getImm();
if (extractAnnotationIndex(ImmValue) == Index)
return extractAnnotationValue(ImmValue);
}
}

return std::nullopt;
Expand All @@ -172,21 +173,18 @@ class MCPlusBuilder {
/// AnnotationNameIndexMap and AnnotationsNames.
mutable llvm::sys::RWMutex AnnotationNameMutex;

/// Allocate the TailCall annotation value. Clients of the target-specific
/// Set TailCall annotation value to true. Clients of the target-specific
/// MCPlusBuilder classes must use convert/lower/create* interfaces instead.
void setTailCall(MCInst &Inst);
void setTailCall(MCInst &Inst) const;

public:
/// Transfer annotations from \p SrcInst to \p DstInst.
void moveAnnotations(MCInst &&SrcInst, MCInst &DstInst) const {
assert(!getAnnotationInst(DstInst) &&
"Destination instruction should not have annotations.");
const MCInst *AnnotationInst = getAnnotationInst(SrcInst);
if (!AnnotationInst)
return;
MCInst::iterator AnnotationOp = getAnnotationInstOp(SrcInst);
for (MCInst::iterator Iter = AnnotationOp; Iter != SrcInst.end(); ++Iter)
DstInst.addOperand(*Iter);

DstInst.addOperand(MCOperand::createInst(AnnotationInst));
removeAnnotationInst(SrcInst);
SrcInst.erase(AnnotationOp, SrcInst.end());
}

/// Return iterator range covering def operands.
Expand Down Expand Up @@ -390,7 +388,6 @@ class MCPlusBuilder {

Allocator.AnnotationPool.clear();
Allocator.ValueAllocator.Reset();
Allocator.MCInstAllocator.DestroyAll();
}
}

Expand Down Expand Up @@ -1128,20 +1125,19 @@ class MCPlusBuilder {
std::optional<MCPlus::MCLandingPad> getEHInfo(const MCInst &Inst) const;

/// Add handler and action info for call instruction.
void addEHInfo(MCInst &Inst, const MCPlus::MCLandingPad &LP);
void addEHInfo(MCInst &Inst, const MCPlus::MCLandingPad &LP) const;

/// Update exception-handling info for the invoke instruction \p Inst.
/// Return true on success and false otherwise, e.g. if the instruction is
/// not an invoke.
bool updateEHInfo(MCInst &Inst, const MCPlus::MCLandingPad &LP);
bool updateEHInfo(MCInst &Inst, const MCPlus::MCLandingPad &LP) const;

/// Return non-negative GNU_args_size associated with the instruction
/// or -1 if there's no associated info.
int64_t getGnuArgsSize(const MCInst &Inst) const;

/// Add the value of GNU_args_size to Inst if it already has EH info.
void addGnuArgsSize(MCInst &Inst, int64_t GnuArgsSize,
AllocatorIdTy AllocId = 0);
void addGnuArgsSize(MCInst &Inst, int64_t GnuArgsSize) const;

/// Return jump table addressed by this instruction.
uint64_t getJumpTable(const MCInst &Inst) const;
Expand All @@ -1154,19 +1150,19 @@ class MCPlusBuilder {
AllocatorIdTy AllocId = 0);

/// Disassociate instruction with a jump table.
bool unsetJumpTable(MCInst &Inst);
bool unsetJumpTable(MCInst &Inst) const;

/// Return destination of conditional tail call instruction if \p Inst is one.
std::optional<uint64_t> getConditionalTailCall(const MCInst &Inst) const;

/// Mark the \p Instruction as a conditional tail call, and set its
/// destination address if it is known. If \p Instruction was already marked,
/// update its destination with \p Dest.
bool setConditionalTailCall(MCInst &Inst, uint64_t Dest = 0);
bool setConditionalTailCall(MCInst &Inst, uint64_t Dest = 0) const;

/// If \p Inst was marked as a conditional tail call convert it to a regular
/// branch. Return true if the instruction was converted.
bool unsetConditionalTailCall(MCInst &Inst);
bool unsetConditionalTailCall(MCInst &Inst) const;

/// Return offset of \p Inst in the original function, if available.
std::optional<uint32_t> getOffset(const MCInst &Inst) const;
Expand All @@ -1175,10 +1171,10 @@ class MCPlusBuilder {
uint32_t getOffsetWithDefault(const MCInst &Inst, uint32_t Default) const;

/// Set offset of \p Inst in the original function.
bool setOffset(MCInst &Inst, uint32_t Offset, AllocatorIdTy AllocatorId = 0);
bool setOffset(MCInst &Inst, uint32_t Offset) const;

/// Remove offset annotation.
bool clearOffset(MCInst &Inst);
bool clearOffset(MCInst &Inst) const;

/// Return the label of \p Inst, if available.
MCSymbol *getLabel(const MCInst &Inst) const;
Expand Down Expand Up @@ -1827,8 +1823,7 @@ class MCPlusBuilder {

if (!std::is_trivial<ValueType>::value)
Allocator.AnnotationPool.insert(A);
setAnnotationOpValue(Inst, Index, reinterpret_cast<int64_t>(A),
AllocatorId);
setAnnotationOpValue(Inst, Index, reinterpret_cast<int64_t>(A));
return A->getValue();
}

Expand Down Expand Up @@ -1961,21 +1956,21 @@ class MCPlusBuilder {
///
/// Return true if the annotation was removed, false if the annotation
/// was not present.
bool removeAnnotation(MCInst &Inst, unsigned Index);
bool removeAnnotation(MCInst &Inst, unsigned Index) const;

/// Remove annotation associated with \p Name.
///
/// Return true if the annotation was removed, false if the annotation
/// was not present.
bool removeAnnotation(MCInst &Inst, StringRef Name) {
bool removeAnnotation(MCInst &Inst, StringRef Name) const {
const auto Index = getAnnotationIndex(Name);
if (!Index)
return false;
return removeAnnotation(Inst, *Index);
}

/// Remove meta-data, but don't destroy it.
void stripAnnotations(MCInst &Inst, bool KeepTC = false);
/// Remove meta-data from the instruction, but don't destroy it.
void stripAnnotations(MCInst &Inst, bool KeepTC = false) const;

virtual InstructionListType
createInstrumentedIndirectCall(MCInst &&CallInst, MCSymbol *HandlerFuncAddr,
Expand Down
19 changes: 10 additions & 9 deletions bolt/lib/Core/BinaryFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1999,7 +1999,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
}
}
if (LastNonNop && !MIB->getOffset(*LastNonNop))
MIB->setOffset(*LastNonNop, static_cast<uint32_t>(Offset), AllocatorId);
MIB->setOffset(*LastNonNop, static_cast<uint32_t>(Offset));
};

for (auto I = Instructions.begin(), E = Instructions.end(); I != E; ++I) {
Expand All @@ -2022,7 +2022,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
if (MIB->isNoop(Instr) && !MIB->getOffset(Instr)) {
// If "Offset" annotation is not present, set it and mark the nop for
// deletion.
MIB->setOffset(Instr, static_cast<uint32_t>(Offset), AllocatorId);
MIB->setOffset(Instr, static_cast<uint32_t>(Offset));
// Annotate ordinary nops, so we can safely delete them if required.
MIB->addAnnotation(Instr, "NOP", static_cast<uint32_t>(1), AllocatorId);
}
Expand Down Expand Up @@ -2303,6 +2303,13 @@ void BinaryFunction::removeConditionalTailCalls() {
assert(CTCTargetLabel && "symbol expected for conditional tail call");
MCInst TailCallInstr;
BC.MIB->createTailCall(TailCallInstr, CTCTargetLabel, BC.Ctx.get());

// Move offset from CTCInstr to TailCallInstr.
if (const std::optional<uint32_t> Offset = BC.MIB->getOffset(*CTCInstr)) {
BC.MIB->setOffset(TailCallInstr, *Offset);
BC.MIB->clearOffset(*CTCInstr);
}

// Link new BBs to the original input offset of the BB where the CTC
// is, so we can map samples recorded in new BBs back to the original BB
// seem in the input binary (if using BAT)
Expand Down Expand Up @@ -2331,12 +2338,6 @@ void BinaryFunction::removeConditionalTailCalls() {

// This branch is no longer a conditional tail call.
BC.MIB->unsetConditionalTailCall(*CTCInstr);

// Move offset from CTCInstr to TailCallInstr.
if (std::optional<uint32_t> Offset = BC.MIB->getOffset(*CTCInstr)) {
BC.MIB->setOffset(TailCallInstr, *Offset);
BC.MIB->clearOffset(*CTCInstr);
}
}

insertBasicBlocks(std::prev(end()), std::move(NewBlocks),
Expand Down Expand Up @@ -3373,7 +3374,7 @@ void BinaryFunction::propagateGnuArgsSizeInfo(
}
} else if (BC.MIB->isInvoke(Instr)) {
// Add the value of GNU_args_size as an extra operand to invokes.
BC.MIB->addGnuArgsSize(Instr, CurrentGnuArgsSize, AllocId);
BC.MIB->addGnuArgsSize(Instr, CurrentGnuArgsSize);
}
++II;
}
Expand Down
Loading

0 comments on commit 74e0a26

Please sign in to comment.