[PATCH] D114416: [ELF] Support non-RAX/non-adjacent R_X86_64_GOTPC32_TLSDESC/R_X86_64_TLSDESC_CALL

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
Herald added subscribers: arichardson, emaste.
MaskRay updated this revision to Diff 389105.
MaskRay added a comment.
MaskRay updated this revision to Diff 389250.
MaskRay retitled this revision from "[ELF] Support non-RAX R_X86_64_GOTPC32_TLSDESC" to "[ELF] Support non-RAX/non-adjacent R_X86_64_GOTPC32_TLSDESC/R_X86_64_TLSDESC_CALL".
MaskRay edited the summary of this revision.
Herald added a subscriber: pengfei.
MaskRay updated this revision to Diff 389256.
MaskRay edited the summary of this revision.
This revision was not accepted when it landed; it landed in state "Draft".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG38ed1db7e874: [ELF] Support non-RAX/non-adjacent R_X86_64_GOTPC32_TLSDESC/R_X86_64_TLSDESC_CA… (authored by MaskRay).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

address comments


MaskRay added a comment.

Change description


MaskRay added a comment.

update description



================
Comment at: lld/ELF/Arch/X86_64.cpp:452
+    // Convert leaq x at tlsdesc(%rip), %REG to movq $x at tpoff, %REG.
+    if ((loc[-3] & 0xfb) != 0x48 || loc[-2] != 0x8d ||
+        (loc[-1] & 0xc7) != 0x05) {
----------------
REX.X and REX.B does not affect RIP-relative addressing. I believe we should use mask
`0xf8` for `loc[-3]`.


================
Comment at: lld/ELF/Arch/X86_64.cpp:452
+    // Convert leaq x at tlsdesc(%rip), %REG to movq $x at tpoff, %REG.
+    if ((loc[-3] & 0xfb) != 0x48 || loc[-2] != 0x8d ||
+        (loc[-1] & 0xc7) != 0x05) {
----------------
skan wrote:
> REX.X and REX.B does not affect RIP-relative addressing. I believe we should use mask
> `0xf8` for `loc[-3]`.
I think we should still use 0xfb. I cannot get an assembler to use the REX.X and REX.B bits.


================
Comment at: lld/ELF/Arch/X86_64.cpp:459
     }
-    // movq $x at tpoff(%rip),%rax
+    loc[-3] = (loc[-3] & 0x48) | ((loc[-3] >> 2) & 1);
     loc[-2] = 0xc7;
----------------
Here we'd like to keep the fixed prefix `0100` and set REX.B to the value of REX.R. We already checked REX.W =1 before. I think code can be simplified to

`loc[-3] = 0x48 | ((loc[-3] >>2) & 1);`


================
Comment at: lld/ELF/Arch/X86_64.cpp:461
     loc[-2] = 0xc7;
-    loc[-1] = 0xc0;
+    loc[-1] = 0xc0 | ((loc[-1] >> 3) & 7);
     write32le(loc, val + 4);
----------------
Here we set mod=11, reg=000, and set the rm to the value of reg. LGTM.


The current TLSDESC optimization code assumes:

  leaq x at tlsdesc(%rip), %rax
  call *x at tlscall(%rax)       # adjacent

>From https://gitlab.freedesktop.org/mesa/mesa/-/issues/5665 , it seems that the
two instructions may not be adjacent in GCC 10's output:

  leaq x at tlsdesc(%rip), %rax
  something else
  call *x at tlscall(%rax)

This patch supports the case. While here, support non-RAX registers for
R_X86_64_GOTPC32_TLSDESC, in case the compiler generates inefficient:

  leaq x at tlsdesc(%rip), %rcx  # or %rdx, %rbx, %rdi, ...
  movq %rcx, %rax
  call *x at tlscall(%rax)       # GNU ld/gold error for non-RAX


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114416

Files:
  lld/ELF/Arch/X86_64.cpp
  lld/docs/ReleaseNotes.rst
  lld/test/ELF/invalid/x86-64-tlsdesc-gd.s
  lld/test/ELF/x86-64-tlsdesc-gd.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114416.389257.patch
Type: text/x-patch
Size: 8958 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211123/87d7b588/attachment.bin>