Page MenuHomePhabricator

[X86] Support -fno-plt __tls_get_addr calls
ClosedPublic

Authored by MaskRay on May 19 2019, 2:10 AM.

Details

Summary

In general dynamic/local dynamic TLS models, with -fno-plt,

  • x86: emit calll *___tls_get_addr@GOT(%ebx) instead of calll ___tls_get_addr@PLT Note, on x86, if we can get rid of %ebx as the PIC register, it may be better to use a register not preserved across function calls.
  • x86_64: emit callq *__tls_get_addr@GOTPCREL(%rip) instead of callq __tls_get_addr@PLT

Reorganize the code by separating 32-bit and 64-bit.

Diff Detail

Repository
rL LLVM

Event Timeline

MaskRay created this revision.May 19 2019, 2:10 AM
Herald added a project: Restricted Project. ยท View Herald TranscriptMay 19 2019, 2:10 AM
Herald added a subscriber: llvm-commits. ยท View Herald Transcript
MaskRay updated this revision to Diff 200174.May 19 2019, 2:11 AM
MaskRay edited the summary of this revision. (Show Details)

Fix markdown in the description...

Harbormaster completed remote builds in B32117: Diff 200174.
dalias added a subscriber: dalias.May 19 2019, 11:36 PM

Hardcoding ebx defeats most of the purpose of no-plt, which was motivated by: https://ewontfix.com/18/

https://ewontfix.com/18/ was enlightening. I read it two or three years ago for the first time but apparently I lacked background knowledge to understand these stuff then :) Now I understand how -fno-plt benefits people who don't care about lazy binding...

call    __x86.get_pc_thunk.cx
jmp *bar@GOT+_GLOBAL_OFFSET_TABLE_(%ecx)

I think the relocation type is unavailable. This requires something like G+GOT+A-* (think R_X86_64_GOTPCREL G+GOT+A-P) but that is not in the psABI.

We can still do better by switching to %eax, %ecx or %edx, but I think this function does not have information to make the decision. We should probably propagate the -fno-plt (RtLibUseGot) information to several levels above but I know really little about the x86 codegen to achieve that...

I have to add a TODO item in the description to note down this future improvement.

MaskRay updated this revision to Diff 200222.May 20 2019, 2:15 AM
MaskRay edited the summary of this revision. (Show Details)

Add a TODO

I'm not sure how LLVM internals work, but if it's anything like GCC's, I think you just need to make it take a register operand for the GOT address as an input/operand. Then the higher-level code should ensure it's available, and then you just use it in the code emitted. Touching higher-level code shouldn't be needed.

rnk accepted this revision.Wed, May 22, 2:33 PM

Looks good to me.

I'm not 100% following the discussion of using EBX as the PIC register, but I think it's a pre-existing issue for 32-bit code. I think it's unlikely that anyone will ever care enough to improve our 32-bit codegen, so it seems reasonable to leave it alone for now (or forever).

lib/Target/X86/X86MCInstLower.cpp
690 โ†—(On Diff #200222)

I think it would be more readable to retain this NeedsPadding variable, but make it evaluate to SRVK == MCSymbolRefExpr::VK_TLSGD.

This revision is now accepted and ready to land.Wed, May 22, 2:33 PM
MaskRay updated this revision to Diff 200836.Wed, May 22, 4:40 PM
MaskRay edited the summary of this revision. (Show Details)

Address review comment

This revision was automatically updated to reflect the committed changes.