Skip to content

Commit

Permalink
[BOLT] Make instruction size a first-class annotation (llvm#72167)
Browse files Browse the repository at this point in the history
When NOP instructions are used to reserve space in the code, e.g. for
patching, it becomes critical to preserve their original size while
emitting the code. On x86, we rely on "Size" annotation for NOP
instructions size, as the original instruction size is lost in the
disassembly/assembly process.

This change makes instruction size a first-class annotation and is
affectively NFCI. A follow-up diff will use the annotation for code
emission.
  • Loading branch information
maksfb authored Nov 13, 2023
1 parent b288b41 commit 2db9b6a
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 9 deletions.
4 changes: 2 additions & 2 deletions bolt/include/bolt/Core/BinaryContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -1239,8 +1239,8 @@ class BinaryContext {
uint64_t
computeInstructionSize(const MCInst &Inst,
const MCCodeEmitter *Emitter = nullptr) const {
if (auto Size = MIB->getAnnotationWithDefault<uint32_t>(Inst, "Size"))
return Size;
if (std::optional<uint32_t> Size = MIB->getSize(Inst))
return *Size;

if (!Emitter)
Emitter = this->MCE.get();
Expand Down
1 change: 1 addition & 0 deletions bolt/include/bolt/Core/MCPlus.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class MCAnnotation {
kConditionalTailCall, /// CTC.
kOffset, /// Offset in the function.
kLabel, /// MCSymbol pointing to this instruction.
kSize, /// Size of the instruction.
kGeneric /// First generic annotation.
};

Expand Down
6 changes: 6 additions & 0 deletions bolt/include/bolt/Core/MCPlusBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1179,6 +1179,12 @@ class MCPlusBuilder {
/// is emitted to MCStreamer.
bool setLabel(MCInst &Inst, MCSymbol *Label) const;

/// Get instruction size specified via annotation.
std::optional<uint32_t> getSize(const MCInst &Inst) const;

/// Set instruction size.
void setSize(MCInst &Inst, uint32_t Size) const;

/// Return MCSymbol that represents a target of this instruction at a given
/// operand number \p OpNum. If there's no symbol associated with
/// the operand - return nullptr.
Expand Down
2 changes: 2 additions & 0 deletions bolt/lib/Core/BinaryContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1897,6 +1897,8 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction,
}
if (std::optional<uint32_t> Offset = MIB->getOffset(Instruction))
OS << " # Offset: " << *Offset;
if (std::optional<uint32_t> Size = MIB->getSize(Instruction))
OS << " # Size: " << *Size;
if (MCSymbol *Label = MIB->getLabel(Instruction))
OS << " # Label: " << *Label;

Expand Down
11 changes: 6 additions & 5 deletions bolt/lib/Core/BinaryFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,7 @@ bool BinaryFunction::disassemble() {
// NOTE: disassembly loses the correct size information for noops on x86.
// E.g. nopw 0x0(%rax,%rax,1) is 9 bytes, but re-encoded it's only
// 5 bytes. Preserve the size info using annotations.
MIB->addAnnotation(Instruction, "Size", static_cast<uint32_t>(Size));
MIB->setSize(Instruction, Size);
}

addInstruction(Offset, std::move(Instruction));
Expand Down Expand Up @@ -4353,10 +4353,11 @@ MCInst *BinaryFunction::getInstructionAtOffset(uint64_t Offset) {
}

if (MCInst *LastInstr = BB->getLastNonPseudoInstr()) {
const uint32_t Size =
BC.MIB->getAnnotationWithDefault<uint32_t>(*LastInstr, "Size");
if (BB->getEndOffset() - Offset == Size)
return LastInstr;
if (std::optional<uint32_t> Size = BC.MIB->getSize(*LastInstr)) {
if (BB->getEndOffset() - Offset == Size) {
return LastInstr;
}
}
}

return nullptr;
Expand Down
11 changes: 11 additions & 0 deletions bolt/lib/Core/MCPlusBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,17 @@ bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label) const {
return true;
}

std::optional<uint32_t> MCPlusBuilder::getSize(const MCInst &Inst) const {
if (std::optional<int64_t> Value =
getAnnotationOpValue(Inst, MCAnnotation::kSize))
return static_cast<uint32_t>(*Value);
return std::nullopt;
}

void MCPlusBuilder::setSize(MCInst &Inst, uint32_t Size) const {
setAnnotationOpValue(Inst, MCAnnotation::kSize, Size);
}

bool MCPlusBuilder::hasAnnotation(const MCInst &Inst, unsigned Index) const {
return (bool)getAnnotationOpValue(Inst, Index);
}
Expand Down
3 changes: 2 additions & 1 deletion bolt/lib/Profile/DataReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,8 @@ bool DataReader::recordBranch(BinaryFunction &BF, uint64_t From, uint64_t To,
if (!BC.MIB->isNoop(Instr))
break;

Offset += BC.MIB->getAnnotationWithDefault<uint32_t>(Instr, "Size");
if (std::optional<uint32_t> Size = BC.MIB->getSize(Instr))
Offset += *Size;
}

if (To == Offset)
Expand Down
1 change: 0 additions & 1 deletion bolt/lib/Rewrite/RewriteInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3212,7 +3212,6 @@ void RewriteInstance::buildFunctionsCFG() {
// Create annotation indices to allow lock-free execution
BC->MIB->getOrCreateAnnotationIndex("JTIndexReg");
BC->MIB->getOrCreateAnnotationIndex("NOP");
BC->MIB->getOrCreateAnnotationIndex("Size");

ParallelUtilities::WorkFuncWithAllocTy WorkFun =
[&](BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId) {
Expand Down

0 comments on commit 2db9b6a

Please sign in to comment.