[X86] Emit RIP-relative access to local function in PIC medium code model

[tkoeppe]

Summary

Currently, the medium code model for x86_64 emits position-dependent relocations (R_X86_64_64) for local functions, regardless of PIC or no-PIC mode. (This means generically that code compiled with the medium model cannot be linked into a position-independent executable.)

Example:

static int g(int n) {
  return 2 * n + 3;
}

void f(int(**p)(int)) {
  *p = g;
}

This results in:

Disassembly of section .text:

0000000000000000 <f>:
       0: 48 b8 00 00 00 00 00 00 00 00	movabs	rax, 0x0
       a: 48 89 07                     	mov	qword ptr [rdi], rax
       d: c3                           	ret

Relocation section '.rela.text' at offset 0xf0 contains 1 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000002  0000000200000001 R_X86_64_64            0000000000000000 .text + 10

This patch changes the behaviour to unconditionally emit a RIP-relative access, both in PIC and non-PIC mode. This fixes PIC mode, and is perhaps an improvement in non-PIC mode, too, since it results in a shorter instruction. A 32-bit relocation should suffice since the medium memory model demands that all code fit within 2GiB.

[MaskRay]

It's common to tag such changes as [X86] (git log llvm/lib/Target/X86 to get a taste)

I may a title like [X86] Emit RIP-relative access to local function in PIC medium code model. I'd avoid a godbolt link and paste the code directly into the summary.

-U99999 from https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

[tkoeppe]

Updated diff to use -U9999.

[lebedev.ri]

This could use a test.

[tkoeppe]

In D140593#4014516, @lebedev.ri wrote:

This could use a test.

Yes, on it! I'll also need to update the existing tests.

But could anyone confirm in principle that the lack of PIC support of the medium model currently is indeed a bug, and that this solution is appropriate?

[tkoeppe]

Added a comment in the code explaining the choice for "medium code model, function", and updated the tests.

[MaskRay]

LGTM. I was also playing with how to hack small code model to behave like medium code model and found this piece of WrapperRIP code.

% curl -L 'https://reviews.llvm.org/D140593?download=1'
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
...

This uses patch -p0. It you use arc or git format-patch -1 --stdout, the patch file will require patch -p1, which is more common.

llvm/lib/Target/X86/X86ISelLowering.cpp

20500

This is the common path. So moving the medium code model code below this if will remove a redundant test for the common path.

[tkoeppe]

Moved new condition further down, so it appears after the more common checks. Also changed the diff header to work with -p1, not -p0.

[MaskRay]

llvm/lib/Target/X86/X86ISelLowering.cpp

20492

Delete this blank line.

[tkoeppe]

llvm/lib/Target/X86/X86ISelLowering.cpp

20492

Are you sure? I thought since M is used in multiple conditions below now, it'd be better to not "stick" to any one of them, so it's easier to spot, and so it doesn't look like it just appertains to one of the if statements. I can change it if you like, but I thought it looks clearer with this blank line.

[MaskRay]

llvm/lib/Target/X86/X86ISelLowering.cpp

20492

I'm sure. The prevailing style is compact, and the argument that it's used twice isn't really a strong one.

[tkoeppe]

llvm/lib/Target/X86/X86ISelLowering.cpp

20492

OK, sure, done!

llvm/lib/Target/X86/X86ISelLowering.cpp

 // Returns the appropriate wrapper opcode for a global reference.
 unsigned X86TargetLowering::getGlobalWrapperKind(
     const GlobalValue *GV, const unsigned char OpFlags) const {
   // References to absolute symbols are never PC-relative.
   if (GV && GV->isAbsoluteSymbolRef())
     return X86ISD::Wrapper;
 
   CodeModel::Model M = getTargetMachine().getCodeModel();
   if (Subtarget.isPICStyleRIPRel() &&
       (M == CodeModel::Small || M == CodeModel::Kernel))
     return X86ISD::WrapperRIP;
 
+  // In the medium model, functions can always be referenced
+  // RIP-relatively, since they must be within 2GiB. This is
+  // also possible in non-PIC mode, and shorter than the 64-bit
+  // absolute immediate that would otherwise be emitted.
+  if (M == CodeModel::Medium && isa_and_nonnull<Function>(GV))
+    return X86ISD::WrapperRIP;
+
   // GOTPCREL references must always use RIP.
   if (OpFlags == X86II::MO_GOTPCREL || OpFlags == X86II::MO_GOTPCREL_NORELAX)
     return X86ISD::WrapperRIP;
 
   return X86ISD::Wrapper;
 }
 
 // ConstantPool, JumpTable, GlobalAddress, and ExternalSymbol are lowered as

llvm/test/CodeGen/X86/code-model-elf.ll

 define dso_local ptr @lea_static_fn() #0 {
 ; SMALL-STATIC-LABEL: lea_static_fn:
 ; SMALL-STATIC:       # %bb.0:
 ; SMALL-STATIC-NEXT:    movl $static_fn, %eax
 ; SMALL-STATIC-NEXT:    retq
 ;
 ; MEDIUM-STATIC-LABEL: lea_static_fn:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movabsq $static_fn, %rax
+; MEDIUM-STATIC-NEXT:    leaq static_fn(%rip), %rax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: lea_static_fn:
 ; LARGE-STATIC:       # %bb.0:
 ; LARGE-STATIC-NEXT:    movabsq $static_fn, %rax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: lea_static_fn:
 ; SMALL-PIC:       # %bb.0:
 ; SMALL-PIC-NEXT:    leaq static_fn(%rip), %rax
 ; SMALL-PIC-NEXT:    retq
 ;
 ; MEDIUM-PIC-LABEL: lea_static_fn:
 ; MEDIUM-PIC:       # %bb.0:
-; MEDIUM-PIC-NEXT:    movabsq $static_fn, %rax
+; MEDIUM-PIC-NEXT:    leaq static_fn(%rip), %rax
 ; MEDIUM-PIC-NEXT:    retq
 ;
 ; LARGE-PIC-LABEL: lea_static_fn:
 ; LARGE-PIC:       # %bb.0:
 ; LARGE-PIC-NEXT:  .L7$pb:
 ; LARGE-PIC-NEXT:    leaq .L7$pb(%rip), %rax
 ; LARGE-PIC-NEXT:    movabsq $_GLOBAL_OFFSET_TABLE_-.L7$pb, %rcx
 ; LARGE-PIC-NEXT:    addq %rax, %rcx
 ; LARGE-PIC-NEXT:    movabsq $static_fn@GOTOFF, %rax
 ; LARGE-PIC-NEXT:    addq %rcx, %rax
 ; LARGE-PIC-NEXT:    retq
   ret ptr @static_fn
 }
 
 define dso_local ptr @lea_global_fn() #0 {
 ; SMALL-STATIC-LABEL: lea_global_fn:
 ; SMALL-STATIC:       # %bb.0:
 ; SMALL-STATIC-NEXT:    movl $global_fn, %eax
 ; SMALL-STATIC-NEXT:    retq
 ;
 ; MEDIUM-STATIC-LABEL: lea_global_fn:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movabsq $global_fn, %rax
+; MEDIUM-STATIC-NEXT:    leaq global_fn(%rip), %rax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: lea_global_fn:
 ; LARGE-STATIC:       # %bb.0:
 ; LARGE-STATIC-NEXT:    movabsq $global_fn, %rax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: lea_global_fn:
 ; SMALL-PIC:       # %bb.0:
 ; SMALL-PIC-NEXT:    leaq global_fn(%rip), %rax
 ; SMALL-PIC-NEXT:    retq
 ;
 ; MEDIUM-PIC-LABEL: lea_global_fn:
 ; MEDIUM-PIC:       # %bb.0:
-; MEDIUM-PIC-NEXT:    movabsq $global_fn, %rax
+; MEDIUM-PIC-NEXT:    leaq global_fn(%rip), %rax
 ; MEDIUM-PIC-NEXT:    retq
 ;
 ; LARGE-PIC-LABEL: lea_global_fn:
 ; LARGE-PIC:       # %bb.0:
 ; LARGE-PIC-NEXT:  .L8$pb:
 ; LARGE-PIC-NEXT:    leaq .L8$pb(%rip), %rax
 ; LARGE-PIC-NEXT:    movabsq $_GLOBAL_OFFSET_TABLE_-.L8$pb, %rcx
 ; LARGE-PIC-NEXT:    addq %rax, %rcx