Skip to content

Commit 2db9b6a

Browse files
authored
[BOLT] Make instruction size a first-class annotation (llvm#72167)
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.
1 parent b288b41 commit 2db9b6a

File tree

8 files changed

+30
-9
lines changed

8 files changed

+30
-9
lines changed

bolt/include/bolt/Core/BinaryContext.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,8 +1239,8 @@ class BinaryContext {
12391239
uint64_t
12401240
computeInstructionSize(const MCInst &Inst,
12411241
const MCCodeEmitter *Emitter = nullptr) const {
1242-
if (auto Size = MIB->getAnnotationWithDefault<uint32_t>(Inst, "Size"))
1243-
return Size;
1242+
if (std::optional<uint32_t> Size = MIB->getSize(Inst))
1243+
return *Size;
12441244

12451245
if (!Emitter)
12461246
Emitter = this->MCE.get();

bolt/include/bolt/Core/MCPlus.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class MCAnnotation {
7272
kConditionalTailCall, /// CTC.
7373
kOffset, /// Offset in the function.
7474
kLabel, /// MCSymbol pointing to this instruction.
75+
kSize, /// Size of the instruction.
7576
kGeneric /// First generic annotation.
7677
};
7778

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,12 @@ class MCPlusBuilder {
11791179
/// is emitted to MCStreamer.
11801180
bool setLabel(MCInst &Inst, MCSymbol *Label) const;
11811181

1182+
/// Get instruction size specified via annotation.
1183+
std::optional<uint32_t> getSize(const MCInst &Inst) const;
1184+
1185+
/// Set instruction size.
1186+
void setSize(MCInst &Inst, uint32_t Size) const;
1187+
11821188
/// Return MCSymbol that represents a target of this instruction at a given
11831189
/// operand number \p OpNum. If there's no symbol associated with
11841190
/// the operand - return nullptr.

bolt/lib/Core/BinaryContext.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,6 +1897,8 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction,
18971897
}
18981898
if (std::optional<uint32_t> Offset = MIB->getOffset(Instruction))
18991899
OS << " # Offset: " << *Offset;
1900+
if (std::optional<uint32_t> Size = MIB->getSize(Instruction))
1901+
OS << " # Size: " << *Size;
19001902
if (MCSymbol *Label = MIB->getLabel(Instruction))
19011903
OS << " # Label: " << *Label;
19021904

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,7 +1380,7 @@ bool BinaryFunction::disassemble() {
13801380
// NOTE: disassembly loses the correct size information for noops on x86.
13811381
// E.g. nopw 0x0(%rax,%rax,1) is 9 bytes, but re-encoded it's only
13821382
// 5 bytes. Preserve the size info using annotations.
1383-
MIB->addAnnotation(Instruction, "Size", static_cast<uint32_t>(Size));
1383+
MIB->setSize(Instruction, Size);
13841384
}
13851385

13861386
addInstruction(Offset, std::move(Instruction));
@@ -4353,10 +4353,11 @@ MCInst *BinaryFunction::getInstructionAtOffset(uint64_t Offset) {
43534353
}
43544354

43554355
if (MCInst *LastInstr = BB->getLastNonPseudoInstr()) {
4356-
const uint32_t Size =
4357-
BC.MIB->getAnnotationWithDefault<uint32_t>(*LastInstr, "Size");
4358-
if (BB->getEndOffset() - Offset == Size)
4359-
return LastInstr;
4356+
if (std::optional<uint32_t> Size = BC.MIB->getSize(*LastInstr)) {
4357+
if (BB->getEndOffset() - Offset == Size) {
4358+
return LastInstr;
4359+
}
4360+
}
43604361
}
43614362

43624363
return nullptr;

bolt/lib/Core/MCPlusBuilder.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,17 @@ bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label) const {
279279
return true;
280280
}
281281

282+
std::optional<uint32_t> MCPlusBuilder::getSize(const MCInst &Inst) const {
283+
if (std::optional<int64_t> Value =
284+
getAnnotationOpValue(Inst, MCAnnotation::kSize))
285+
return static_cast<uint32_t>(*Value);
286+
return std::nullopt;
287+
}
288+
289+
void MCPlusBuilder::setSize(MCInst &Inst, uint32_t Size) const {
290+
setAnnotationOpValue(Inst, MCAnnotation::kSize, Size);
291+
}
292+
282293
bool MCPlusBuilder::hasAnnotation(const MCInst &Inst, unsigned Index) const {
283294
return (bool)getAnnotationOpValue(Inst, Index);
284295
}

bolt/lib/Profile/DataReader.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,8 @@ bool DataReader::recordBranch(BinaryFunction &BF, uint64_t From, uint64_t To,
698698
if (!BC.MIB->isNoop(Instr))
699699
break;
700700

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

704705
if (To == Offset)

bolt/lib/Rewrite/RewriteInstance.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3212,7 +3212,6 @@ void RewriteInstance::buildFunctionsCFG() {
32123212
// Create annotation indices to allow lock-free execution
32133213
BC->MIB->getOrCreateAnnotationIndex("JTIndexReg");
32143214
BC->MIB->getOrCreateAnnotationIndex("NOP");
3215-
BC->MIB->getOrCreateAnnotationIndex("Size");
32163215

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

0 commit comments

Comments
 (0)