Skip to content

Commit 7c206c7

Browse files
authored
[BOLT] Refactor interface for instruction labels. NFCI (#83209)
To avoid accidentally setting the label twice for the same instruction, which can lead to a "lost" label, introduce getOrSetInstLabel() function. Rename existing functions to getInstLabel()/setInstLabel() to make it explicit that they operate on instruction labels. Add an assertion in setInstLabel() that the instruction did not have a prior label set.
1 parent 1f2a1a7 commit 7c206c7

File tree

7 files changed

+32
-21
lines changed

7 files changed

+32
-21
lines changed

bolt/include/bolt/Core/MCPlusBuilder.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,11 +1183,16 @@ class MCPlusBuilder {
11831183
bool clearOffset(MCInst &Inst) const;
11841184

11851185
/// Return the label of \p Inst, if available.
1186-
MCSymbol *getLabel(const MCInst &Inst) const;
1186+
MCSymbol *getInstLabel(const MCInst &Inst) const;
1187+
1188+
/// Set the label of \p Inst or return the existing label for the instruction.
1189+
/// This label will be emitted right before \p Inst is emitted to MCStreamer.
1190+
MCSymbol *getOrCreateInstLabel(MCInst &Inst, const Twine &Name,
1191+
MCContext *Ctx) const;
11871192

11881193
/// Set the label of \p Inst. This label will be emitted right before \p Inst
11891194
/// is emitted to MCStreamer.
1190-
bool setLabel(MCInst &Inst, MCSymbol *Label) const;
1195+
void setInstLabel(MCInst &Inst, MCSymbol *Label) const;
11911196

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

bolt/lib/Core/BinaryContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1967,7 +1967,7 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction,
19671967
OS << " # Offset: " << *Offset;
19681968
if (std::optional<uint32_t> Size = MIB->getSize(Instruction))
19691969
OS << " # Size: " << *Size;
1970-
if (MCSymbol *Label = MIB->getLabel(Instruction))
1970+
if (MCSymbol *Label = MIB->getInstLabel(Instruction))
19711971
OS << " # Label: " << *Label;
19721972

19731973
MIB->printAnnotations(Instruction, OS);

bolt/lib/Core/BinaryEmitter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF,
489489

490490
if (!EmitCodeOnly) {
491491
// A symbol to be emitted before the instruction to mark its location.
492-
MCSymbol *InstrLabel = BC.MIB->getLabel(Instr);
492+
MCSymbol *InstrLabel = BC.MIB->getInstLabel(Instr);
493493

494494
if (opts::UpdateDebugSections && BF.getDWARFUnit()) {
495495
LastLocSeen = emitLineInfo(BF, Instr.getLoc(), LastLocSeen,

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1424,7 +1424,7 @@ Error BinaryFunction::disassemble() {
14241424
InstrMapType::iterator II = Instructions.find(Offset);
14251425
assert(II != Instructions.end() && "reference to non-existing instruction");
14261426

1427-
BC.MIB->setLabel(II->second, Label);
1427+
BC.MIB->setInstLabel(II->second, Label);
14281428
}
14291429

14301430
// Reset symbolizer for the disassembler.

bolt/lib/Core/Exceptions.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,12 +408,11 @@ void BinaryFunction::updateEHRanges() {
408408

409409
// Same symbol is used for the beginning and the end of the range.
410410
MCSymbol *EHSymbol;
411-
if (MCSymbol *InstrLabel = BC.MIB->getLabel(Instr)) {
411+
if (MCSymbol *InstrLabel = BC.MIB->getInstLabel(Instr)) {
412412
EHSymbol = InstrLabel;
413413
} else {
414414
std::unique_lock<llvm::sys::RWMutex> Lock(BC.CtxMutex);
415-
EHSymbol = BC.Ctx->createNamedTempSymbol("EH");
416-
BC.MIB->setLabel(Instr, EHSymbol);
415+
EHSymbol = BC.MIB->getOrCreateInstLabel(Instr, "EH", BC.Ctx.get());
417416
}
418417

419418
// At this point we could be in one of the following states:

bolt/lib/Core/MCPlusBuilder.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "bolt/Core/MCPlusBuilder.h"
1414
#include "bolt/Core/MCPlus.h"
15+
#include "llvm/MC/MCContext.h"
1516
#include "llvm/MC/MCInst.h"
1617
#include "llvm/MC/MCInstrAnalysis.h"
1718
#include "llvm/MC/MCInstrDesc.h"
@@ -266,17 +267,29 @@ bool MCPlusBuilder::clearOffset(MCInst &Inst) const {
266267
return true;
267268
}
268269

269-
MCSymbol *MCPlusBuilder::getLabel(const MCInst &Inst) const {
270+
MCSymbol *MCPlusBuilder::getInstLabel(const MCInst &Inst) const {
270271
if (std::optional<int64_t> Label =
271272
getAnnotationOpValue(Inst, MCAnnotation::kLabel))
272273
return reinterpret_cast<MCSymbol *>(*Label);
273274
return nullptr;
274275
}
275276

276-
bool MCPlusBuilder::setLabel(MCInst &Inst, MCSymbol *Label) const {
277+
MCSymbol *MCPlusBuilder::getOrCreateInstLabel(MCInst &Inst, const Twine &Name,
278+
MCContext *Ctx) const {
279+
MCSymbol *Label = getInstLabel(Inst);
280+
if (Label)
281+
return Label;
282+
283+
Label = Ctx->createNamedTempSymbol(Name);
284+
setAnnotationOpValue(Inst, MCAnnotation::kLabel,
285+
reinterpret_cast<int64_t>(Label));
286+
return Label;
287+
}
288+
289+
void MCPlusBuilder::setInstLabel(MCInst &Inst, MCSymbol *Label) const {
290+
assert(!getInstLabel(Inst) && "Instruction already has assigned label.");
277291
setAnnotationOpValue(Inst, MCAnnotation::kLabel,
278292
reinterpret_cast<int64_t>(Label));
279-
return true;
280293
}
281294

282295
std::optional<uint32_t> MCPlusBuilder::getSize(const MCInst &Inst) const {

bolt/lib/Rewrite/LinuxKernelRewriter.cpp

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -770,11 +770,8 @@ Error LinuxKernelRewriter::rewriteORCTables() {
770770
continue;
771771

772772
// Issue label for the instruction.
773-
MCSymbol *Label = BC.MIB->getLabel(Inst);
774-
if (!Label) {
775-
Label = BC.Ctx->createTempSymbol("__ORC_");
776-
BC.MIB->setLabel(Inst, Label);
777-
}
773+
MCSymbol *Label =
774+
BC.MIB->getOrCreateInstLabel(Inst, "__ORC_", BC.Ctx.get());
778775

779776
if (Error E = emitORCEntry(0, *ErrorOrState, Label))
780777
return E;
@@ -908,11 +905,8 @@ Error LinuxKernelRewriter::readStaticCalls() {
908905

909906
BC.MIB->addAnnotation(*Inst, "StaticCall", EntryID);
910907

911-
MCSymbol *Label = BC.MIB->getLabel(*Inst);
912-
if (!Label) {
913-
Label = BC.Ctx->createTempSymbol("__SC_");
914-
BC.MIB->setLabel(*Inst, Label);
915-
}
908+
MCSymbol *Label =
909+
BC.MIB->getOrCreateInstLabel(*Inst, "__SC_", BC.Ctx.get());
916910

917911
StaticCallEntries.push_back({EntryID, BF, Label});
918912
}

0 commit comments

Comments
 (0)