Skip to content

Use COFF image-base-relative jump tables on AMD64 #147625

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use COFF image-base-relative jump tables on AMD64 #147625

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
2 changes: 2 additions & 0 deletions llvm/include/llvm/CodeGen/MIRYamlMapping.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ template <> struct ScalarEnumerationTraits<MachineJumpTableInfo::JTEntryKind> {
MachineJumpTableInfo::EK_LabelDifference64);
IO.enumCase(EntryKind, "inline", MachineJumpTableInfo::EK_Inline);
IO.enumCase(EntryKind, "custom32", MachineJumpTableInfo::EK_Custom32);
IO.enumCase(EntryKind, "coff-imgrel32",
MachineJumpTableInfo::EK_CoffImgRel32);
Comment on lines +144 to +145
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs mir support tests in test/CodeGen/MIR

}
};

Expand Down
10 changes: 9 additions & 1 deletion llvm/include/llvm/CodeGen/MachineJumpTableInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ class MachineJumpTableInfo {

/// EK_Custom32 - Each entry is a 32-bit value that is custom lowered by the
/// TargetLowering::LowerCustomJumpTableEntry hook.
EK_Custom32
EK_Custom32,

// EK_CoffImgRel32 - In PE/COFF (Windows) images, each entry is a 32-bit
// unsigned offset that is added to the image base.
// .word LBB123@IMGREL
EK_CoffImgRel32,
};

private:
Expand All @@ -100,6 +105,9 @@ class MachineJumpTableInfo {
LLVM_ABI unsigned getEntrySize(const DataLayout &TD) const;
/// getEntryAlignment - Return the alignment of each entry in the jump table.
LLVM_ABI unsigned getEntryAlignment(const DataLayout &TD) const;
/// getEntryIsSigned - Return true if the load for the jump table index
/// should use signed extension, false if zero extension (unsigned)
LLVM_ABI bool getEntryIsSigned() const;

/// createJumpTableIndex - Create a new jump table.
///
Expand Down
8 changes: 8 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3136,6 +3136,14 @@ void AsmPrinter::emitJumpTableEntry(const MachineJumpTableInfo &MJTI,
Value = MCBinaryExpr::createSub(Value, Base, OutContext);
break;
}

case MachineJumpTableInfo::EK_CoffImgRel32: {
// This generates an unsigned 32-bit offset, which is MBB's address minus
// the COFF image base.
Value = MCSymbolRefExpr::create(
MBB->getSymbol(), MCSymbolRefExpr::VK_COFF_IMGREL32, OutContext);
break;
}
}

assert(Value && "Unknown entry kind!");
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3564,6 +3564,10 @@ void CodeViewDebug::collectDebugInfoForJumpTables(const MachineFunction *MF,
std::tie(Base, BaseOffset, Branch, EntrySize) =
Asm->getCodeViewJumpTableInfo(JumpTableIndex, &BranchMI, Branch);
break;
case MachineJumpTableInfo::EK_CoffImgRel32:
EntrySize = JumpTableEntrySize::UInt32;
Base = nullptr;
break;
}

const MachineJumpTableEntry &JTE = JTI.getJumpTables()[JumpTableIndex];
Expand Down
21 changes: 21 additions & 0 deletions llvm/lib/CodeGen/MachineFunction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1328,6 +1328,7 @@ unsigned MachineJumpTableInfo::getEntrySize(const DataLayout &TD) const {
case MachineJumpTableInfo::EK_GPRel32BlockAddress:
case MachineJumpTableInfo::EK_LabelDifference32:
case MachineJumpTableInfo::EK_Custom32:
case MachineJumpTableInfo::EK_CoffImgRel32:
return 4;
case MachineJumpTableInfo::EK_Inline:
return 0;
Expand All @@ -1348,6 +1349,7 @@ unsigned MachineJumpTableInfo::getEntryAlignment(const DataLayout &TD) const {
return TD.getABIIntegerTypeAlignment(64).value();
case MachineJumpTableInfo::EK_GPRel32BlockAddress:
case MachineJumpTableInfo::EK_LabelDifference32:
case MachineJumpTableInfo::EK_CoffImgRel32:
case MachineJumpTableInfo::EK_Custom32:
return TD.getABIIntegerTypeAlignment(32).value();
case MachineJumpTableInfo::EK_Inline:
Expand All @@ -1356,6 +1358,25 @@ unsigned MachineJumpTableInfo::getEntryAlignment(const DataLayout &TD) const {
llvm_unreachable("Unknown jump table encoding!");
}

/// getEntryIsSigned - Return true if the load for the jump table index
/// should use signed extension, false if zero extension (unsigned)
bool MachineJumpTableInfo::getEntryIsSigned() const {
switch (getEntryKind()) {
case MachineJumpTableInfo::EK_BlockAddress:
case MachineJumpTableInfo::EK_GPRel64BlockAddress:
case MachineJumpTableInfo::EK_GPRel32BlockAddress:
case MachineJumpTableInfo::EK_LabelDifference32:
case MachineJumpTableInfo::EK_LabelDifference64:
case MachineJumpTableInfo::EK_Inline:
case MachineJumpTableInfo::EK_Custom32:
return true;

case MachineJumpTableInfo::EK_CoffImgRel32:
return false;
}
llvm_unreachable("Unknown jump table encoding!");
}

/// Create a new jump table entry in the jump table info.
unsigned MachineJumpTableInfo::createJumpTableIndex(
const std::vector<MachineBasicBlock*> &DestBBs) {
Expand Down
15 changes: 10 additions & 5 deletions llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4133,8 +4133,8 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {
const DataLayout &TD = DAG.getDataLayout();
EVT PTy = TLI.getPointerTy(TD);

unsigned EntrySize =
DAG.getMachineFunction().getJumpTableInfo()->getEntrySize(TD);
MachineJumpTableInfo *MJTI = DAG.getMachineFunction().getJumpTableInfo();
unsigned EntrySize = MJTI->getEntrySize(TD);

// For power-of-two jumptable entry sizes convert multiplication to a shift.
// This transformation needs to be done here since otherwise the MIPS
Expand All @@ -4151,10 +4151,15 @@ bool SelectionDAGLegalize::ExpandNode(SDNode *Node) {

EVT MemVT = EVT::getIntegerVT(*DAG.getContext(), EntrySize * 8);
SDValue LD = DAG.getExtLoad(
ISD::SEXTLOAD, dl, PTy, Chain, Addr,
MachinePointerInfo::getJumpTable(DAG.getMachineFunction()), MemVT);
MJTI->getEntryIsSigned() ? ISD::SEXTLOAD : ISD::ZEXTLOAD, dl, PTy,
Chain, Addr, MachinePointerInfo::getJumpTable(DAG.getMachineFunction()),
MemVT);
Addr = LD;
if (TLI.isJumpTableRelative()) {
if (MJTI->getEntryKind() == MachineJumpTableInfo::EK_CoffImgRel32) {
SDValue ImageBase = DAG.getExternalSymbol(
"__ImageBase", TLI.getPointerTy(DAG.getDataLayout()));
Addr = DAG.getMemBasePlusOffset(ImageBase, Addr, dl);
} else if (TLI.isJumpTableRelative()) {
// For PIC, the sequence is:
// BRIND(RelocBase + load(Jumptable + index))
// RelocBase can be JumpTable, GOT or some sort of global base.
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,9 @@ enum TOF {
/// reference is actually to the ".refptr.FOO" symbol. This is used for
/// stub symbols on windows.
MO_COFFSTUB,
/// MO_COFF_IMGREL32: Indicates that the operand value is unsigned 32-bit
/// offset from ImageBase to a symbol (basically .imgrel32).
MO_COFF_IMGREL32,
};

enum : uint64_t {
Expand Down
31 changes: 31 additions & 0 deletions llvm/lib/Target/X86/X86ISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37617,6 +37617,37 @@ X86TargetLowering::EmitSjLjDispatchBlock(MachineInstr &MI,
BuildMI(DispContBB, MIMD, TII->get(X86::JMP64r)).addReg(TReg);
break;
}
case MachineJumpTableInfo::EK_CoffImgRel32: {
Register ImageBaseReg = MRI->createVirtualRegister(&X86::GR64RegClass);
Register OReg64 = MRI->createVirtualRegister(&X86::GR64RegClass);
Register TReg = MRI->createVirtualRegister(&X86::GR64RegClass);

// movl (BReg,IReg64,4), OReg
// This implicitly zero-extends from uint32 to uint64.
BuildMI(DispContBB, MIMD, TII->get(X86::MOV32rm), OReg64)
.addReg(BReg)
.addImm(4)
.addReg(IReg64)
.addImm(0)
.addReg(0);

// leaq (__ImageBase,OReg64), ImageBaseReg
BuildMI(DispContBB, MIMD, TII->get(X86::LEA64r), ImageBaseReg)
.addReg(X86::RIP)
.addImm(0)
.addReg(0)
.addExternalSymbol("__ImageBase", X86II::MO_COFF_IMGREL32)
.addReg(0);

// addq ImageBaseReg, OReg64
BuildMI(DispContBB, MIMD, TII->get(X86::ADD64rr), TReg)
.addReg(ImageBaseReg)
.addReg(OReg64);

// jmpq *TReg
BuildMI(DispContBB, MIMD, TII->get(X86::JMP64r)).addReg(TReg);
break;
}
default:
llvm_unreachable("Unexpected jump table encoding");
}
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/Target/X86/X86ISelLoweringCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ bool X86TargetLowering::allowsMemoryAccess(LLVMContext &Context,
/// current function. The returned value is a member of the
/// MachineJumpTableInfo::JTEntryKind enum.
unsigned X86TargetLowering::getJumpTableEncoding() const {
// Always use EK_CoffImgRel32 for 64-bit Windows targets.
if (Subtarget.isTargetWin64()) {
return MachineJumpTableInfo::EK_CoffImgRel32;
}

// In GOT pic mode, each entry in the jump table is emitted as a @GOTOFF
// symbol.
if (isPositionIndependent() && Subtarget.isPICStyleGOT())
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Target/X86/X86MCInstLower.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,9 @@ MCOperand X86MCInstLower::LowerSymbolOperand(const MachineOperand &MO,
Expr = MCSymbolRefExpr::create(Label, Ctx);
}
break;
case X86II::MO_COFF_IMGREL32:
Expr = MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_COFF_IMGREL32, Ctx);
break;
}

if (!Expr)
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/CodeGen/X86/sjlj-eh.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,10 @@ try.cont:
; CHECK-X64: [[CONT]]:
; *Handlers[UFC.__callsite]
; CHECK-X64: leaq .[[TABLE:LJTI[0-9]+_[0-9]+]](%rip), %rcx
; CHECK-X64: movl (%rcx,%rax,4), %eax
; CHECK-X64: cltq
; CHECK-X64: addq %rcx, %rax
; CHECK-X64: jmpq *%rax
; CHECK-X64: movl (%rcx,%rax,4), %rax
; CHECK-X64: leaq __ImageBase@IMGREL(%rip), %rcx
; CHECK-X64: addq %rax, %rcx
; CHECK-X64: jmpq *%rcx

; CHECK-X64-LINUX: .[[RESUME:LBB[0-9]+_[0-9]+]]:
; assert(UFC.__callsite < 1);
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/X86/win-import-call-optimization-jumptable.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

; CHECK-LABEL: uses_rax:
; CHECK: .Limpcall0:
; CHECK-NEXT: jmpq *%rax
; CHECK-NEXT: jmpq *%rcx

define void @uses_rax(i32 %x) {
entry:
Expand Down Expand Up @@ -74,7 +74,7 @@ declare void @g(i32)
; CHECK-NEXT: .asciz "RetpolineV1"
; CHECK-NEXT: .long 24
; CHECK-NEXT: .secnum .text
; CHECK-NEXT: .long 16
; CHECK-NEXT: .long 17
; CHECK-NEXT: .secoffset .Limpcall0
; CHECK-NEXT: .long 17
; CHECK-NEXT: .secoffset .Limpcall1
Expand Down
44 changes: 29 additions & 15 deletions llvm/test/CodeGen/X86/win64-jumptable.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
; RUN: llc < %s -relocation-model=static | FileCheck %s
; RUN: llc < %s -relocation-model=pic | FileCheck %s --check-prefix=PIC
; RUN: llc < %s -relocation-model=pic -code-model=large | FileCheck %s --check-prefix=PIC
; RUN: llc < %s -relocation-model=pic | FileCheck %s --check-prefixes=CHECK,PIC
; RUN: llc < %s -relocation-model=pic -code-model=large | FileCheck %s --check-prefixes=CHECK,LARGE

; FIXME: Remove '-relocation-model=static' when it is no longer necessary to
; trigger the separate .rdata section.
Expand Down Expand Up @@ -43,25 +43,39 @@ declare void @g(i32)
; CHECK: .text
; CHECK: f:
; CHECK: .seh_proc f
; CHECK: jmpq *.LJTI0_0
; CHECK: .seh_endprologue

; STATIC: movl .LJTI0_0(,%rax,4), %eax
; STATIC: leaq __ImageBase(%rax), %rax
; STATIC: jmpq *%rax

; PIC: movl %ecx, %eax
; PIC: leaq .LJTI0_0(%rip), %rcx
; PIC: movl (%rcx,%rax,4), %eax
; PIC: leaq __ImageBase(%rip), %rcx
; PIC: addq %rax, %rcx
; PIC: jmpq *%rcx
Copy link
Collaborator

Choose a reason for hiding this comment

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

The existing code is:

# %bb.1:                                # %entry
        movl    %ecx, %eax
        leaq    .LJTI0_0(%rip), %rcx
        movslq  (%rcx,%rax,4), %rax
        addq    %rcx, %rax
        jmpq    *%rax

The proposed sequence is 5 bytes longer, and the benefit is... compatibility with Microsoft-internal tools that don't understand the jump-table debug info format defined by Microsoft? That seems a little dubious; why can't these tools understand the existing debug info?

The difference on AArch64 is likely even larger because you're essentially proposing we disable jump table compression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I agree; this isn't worth it for AMD64, unless I can get the code sequence down to the same idiom that MSVC uses. MSVC uses a single RIP-relative load for COFF image base, then a single register-relative load that has an SIB that handles the offset from the image base to the lookup table start, as well as scaling by the table offset. I tried several different ways to generate the same code and could not find an exact equivalent, i.e. one that generated the right SIB idiom.

Ignore my comments are ARM64. I see now that S_ARMSWITCHTABLE covers the layouts that LLVM already uses for switch tables.

Copy link
Collaborator

Choose a reason for hiding this comment

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

llvm-mc gives the following representation for the load from the jump table; does this help?

        movl    .LJTI0_0@IMGREL(%rcx,%rax,4), %eax # <MCInst #2562 MOV32rm
                                        #  <MCOperand Reg:22>
                                        #  <MCOperand Reg:54>
                                        #  <MCOperand Imm:4>
                                        #  <MCOperand Reg:51>
                                        #  <MCOperand Expr:.LJTI0_0@<variant 3>>
                                        #  <MCOperand Reg:0>>

In any case, I computed the actual numbers... the sequence in the current patch is actually 6 bytes longer than what LLVM currently generates. The MSVC-equivalent sequence is 3 bytes shorter... but still 3 bytes longer longer than LLVM's current sequence. Which is slightly better than what I expected, but still not great. (The immediate is 4 bytes. Zero-extending instead of sign-extending the table entry saves a byte.)


; LARGE: movl %ecx, %eax
; LARGE-NEXT: movabsq $.LJTI0_0, %rcx
; LARGE-NEXT: movl (%rcx,%rax,4), %eax
; LARGE-NEXT: movabsq $__ImageBase, %rcx
; LARGE-NEXT: addq %rax, %rcx
; LARGE-NEXT: jmpq *%rcx

; CHECK: .LBB0_{{.*}}: # %sw.bb
; CHECK: .LBB0_{{.*}}: # %sw.bb2
; CHECK: .LBB0_{{.*}}: # %sw.bb3
; CHECK: .LBB0_{{.*}}: # %sw.bb1
; CHECK: callq g
; CHECK: jmp g # TAILCALL
; STATIC: callq g
; STATIC: jmp g # TAILCALL
; CHECK: .section .rdata,"dr"
; CHECK: .quad .LBB0_
; CHECK: .quad .LBB0_
; CHECK: .quad .LBB0_
; CHECK: .quad .LBB0_
; CHECK: .LJTI0_0:
; CHECK: .long .LBB0_{{[0-9]+}}@IMGREL
; CHECK: .long .LBB0_{{[0-9]+}}@IMGREL
; CHECK: .long .LBB0_{{[0-9]+}}@IMGREL
; CHECK: .long .LBB0_{{[0-9]+}}@IMGREL

; It's important that we switch back to .text here, not .rdata.
; CHECK: .text
; CHECK: .seh_endproc

; Windows PIC code should use 32-bit entries
; PIC: .long .LBB0_2-.LJTI0_0
; PIC: .long .LBB0_3-.LJTI0_0
; PIC: .long .LBB0_4-.LJTI0_0
; PIC: .long .LBB0_5-.LJTI0_0
Loading