Skip to content

Jump table annotations for Linux #112606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −4
Open

Jump table annotations for Linux #112606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions llvm/include/llvm/CodeGen/AsmPrinter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,10 @@ class AsmPrinter : public MachineFunctionPass {
/// function to the current output stream.
virtual void emitJumpTableInfo();

/// Emit jump table annotations correlating each table with its associated
/// indirect branch instruction.
virtual void emitJumpTableAnnotation(const MachineFunction &MF, const MachineInstr &MI);
Copy link
Member

@nickdesaulniers nickdesaulniers Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to be virtual if you're only providing one definition?

Copy link
Contributor Author

@ardbiesheuvel ardbiesheuvel Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a rhetorical question?

Copy link
Member

@nickdesaulniers nickdesaulniers Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes (sorry, I could have phrased it differently).

As an example, if you look at the below method declaration for emitGlobalVariable, you'll find a definition for it in the base class AsmPrinter, but overrides are provided in various derived classes (AMDGPUAsmPrinter, ARMAsmPrinter, NVPTXAsmPrinter, PPCAsmPrinter, and WebAssemblyAsmPrinter).

Since you're not changing the functionality for any of the derived classes (i.e. no overrides) and only providing a definition for the base class, then you should not declare this method as virtual. That should avoid needing to traverse the vtable to call this method.


/// Emit the specified global variable to the .s file.
virtual void emitGlobalVariable(const GlobalVariable *GV);

Expand Down
45 changes: 43 additions & 2 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ static cl::opt<bool> EmitJumpTableSizesSection(
cl::desc("Emit a section containing jump table addresses and sizes"),
cl::Hidden, cl::init(false));

static cl::opt<bool> AnnotateJumpTables("annotate-jump-tables",
cl::desc("Annotate jump tables"),
cl::Hidden, cl::init(false));
Comment on lines +165 to +167
Copy link
Member

@nickdesaulniers nickdesaulniers Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cl::opt is not great because it usually means that the command line flag that would trigger this needs to also be passed to the linker when performing LTO. Instead, I think a better approach (though it's more code in LLVM) is to have a module level metadata to signal this codegen behavior, and have the front end emit that when the command line flag is set.

Copy link
Contributor Author

@ardbiesheuvel ardbiesheuvel Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I'll look into that. Suggestions welcome on useful examples.

Copy link
Member

@nickdesaulniers nickdesaulniers Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fc018eb was exactly this. Perhaps we can add a new -f flag to clang for this?

(That way we have -fannotate-jump-tables/-fno-annotate-jump-tables for the compiler, rather than -mllvm -annotate-jump-tables for the compiler (and the linker for LTO)).


STATISTIC(EmittedInsts, "Number of machine instrs printed");

char AsmPrinter::ID = 0;
Expand Down Expand Up @@ -1528,6 +1532,25 @@ void AsmPrinter::emitPseudoProbe(const MachineInstr &MI) {
}
}

void AsmPrinter::emitJumpTableAnnotation(const MachineFunction &MF,
const MachineInstr &MI) {
if (!AnnotateJumpTables || !TM.getTargetTriple().isOSBinFormatELF())
return;

MCSymbol *JTISymbol = GetJTISymbol(MI.getOperand(0).getImm());
MCSymbol *ProvenanceLabel = OutContext.createTempSymbol("jtp");

const MCExpr *OffsetExpr =
MCSymbolRefExpr::create(ProvenanceLabel, OutContext);
const MCExpr *JTISymbolExpr =
MCSymbolRefExpr::create(JTISymbol, OutContext);

OutStreamer->emitRelocDirective(*OffsetExpr, "BFD_RELOC_NONE",
JTISymbolExpr, SMLoc(),
*OutContext.getSubtargetInfo());
OutStreamer->emitLabel(ProvenanceLabel);
}

void AsmPrinter::emitStackSizeSection(const MachineFunction &MF) {
if (!MF.getTarget().Options.EmitStackSizeSection)
return;
Expand Down Expand Up @@ -1849,8 +1872,7 @@ void AsmPrinter::emitFunctionBody() {
OutStreamer->emitRawComment("MEMBARRIER");
break;
case TargetOpcode::JUMP_TABLE_DEBUG_INFO:
// This instruction is only used to note jump table debug info, it's
// purely meta information.
emitJumpTableAnnotation(*MF, MI);
break;
case TargetOpcode::INIT_UNDEF:
// This is only used to influence register allocation behavior, no
Expand Down Expand Up @@ -2821,6 +2843,25 @@ void AsmPrinter::emitJumpTableInfo() {
// label differences will be evaluated at write time.
for (const MachineBasicBlock *MBB : JTBBs)
emitJumpTableEntry(MJTI, MBB, JTI);

if (AnnotateJumpTables && TM.getTargetTriple().isOSBinFormatELF()) {
// Create a temp symbol for the end of the jump table.
MCSymbol *JTIEndSymbol = createTempSymbol("jt_end");
OutStreamer->emitLabel(JTIEndSymbol);

const MCExpr *JTISymbolExpr =
MCSymbolRefExpr::create(JTISymbol, OutContext);

MCSymbol *JTISymbolForSize = OutContext.getOrCreateSymbol(
"$JTI" + Twine(MF->getFunctionNumber()) + "_" + Twine(JTI));
OutStreamer->emitAssignment(JTISymbolForSize, JTISymbolExpr);
OutStreamer->emitSymbolAttribute(JTISymbolForSize, MCSA_ELF_TypeObject);

const MCExpr *SizeExp = MCBinaryExpr::createSub(
MCSymbolRefExpr::create(JTIEndSymbol, OutContext), JTISymbolExpr,
OutContext);
OutStreamer->emitELFSize(JTISymbolForSize, SizeExp);
}
}

if (EmitJumpTableSizesSection)
Expand Down
6 changes: 4 additions & 2 deletions llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,10 @@ SDValue TargetLowering::expandIndirectJTBranch(const SDLoc &dl, SDValue Value,
SDValue Addr, int JTI,
SelectionDAG &DAG) const {
SDValue Chain = Value;
// Jump table debug info is only needed if CodeView is enabled.
if (DAG.getTarget().getTargetTriple().isOSBinFormatCOFF()) {
const auto &Triple = DAG.getTarget().getTargetTriple();
// Jump table debug info is only needed if CodeView is enabled,
// or when adding jump table annotations to ELF objects.
if (Triple.isOSBinFormatCOFF() || Triple.isOSBinFormatELF()) {
Chain = DAG.getJumpTableDebugInfo(JTI, Chain, dl);
}
return DAG.getNode(ISD::BRIND, dl, MVT::Other, Chain, Addr);
Expand Down
Loading