This is an archive of the discontinued LLVM Phabricator instance.

[PPC64] Add support for local-exec TLS model
ClosedPublic

Authored by syzaara on May 31 2018, 11:37 AM.

Details

Summary

This patch adds the relocations needed support the initial-exec TLS model:
R_PPC64_TPREL16
R_PPC64_TPREL16_HA
R_PPC64_TPREL16_LO
R_PPC64_TPREL16_HI
R_PPC64_TPREL16_DS
R_PPC64_TPREL16_LO_DS
R_PPC64_TPREL16_HIGHER
R_PPC64_TPREL16_HIGHERA
R_PPC64_TPREL16_HIGHEST
R_PPC64_TPREL16_HIGHESTA

Missing R_PPC64_TPREL16_HIGH and R_PPC64_TPREL16_HIGHA because llvm-mc doesn't support them in assembly yet. Will add these as a follow-up patch.

Diff Detail

Repository
rL LLVM

Event Timeline

syzaara created this revision.May 31 2018, 11:37 AM

This LGTM, but I think someone with more lld experience should Ok it as well.

lld/test/ELF/ppc64-local-exec-tls.s
7 ↗(On Diff #149327)

minor nit: this should line up with the other directives.

137 ↗(On Diff #149327)

should this be b@tprel?

ruiu added inline comments.May 31 2018, 2:12 PM
lld/ELF/InputSection.cpp
621 ↗(On Diff #149327)

nit: please (almost) always put a blank line before multi-line comment so that the code doesn't look too dense.

621–625 ↗(On Diff #149327)

Do you know why only PPC64v2 is doing something different?

syzaara added inline comments.Jun 1 2018, 8:43 AM
lld/ELF/InputSection.cpp
621–625 ↗(On Diff #149327)

Looking at other targets ABI, for example ARM, the thread pointer points to the thread control block, which is followed by the TLS storage area. So the current behavior was to add the size of the TCB to get to the beginning of the TLS storage.
The Power V2 ABI, shows the thread pointer does not point to the TCB. Instead it points within the TLS storage area, offset by 0x7000 bytes. The ABI mentions that the TP offset allows for efficient addressing of the TCB and up to 4 KB – 8 B of other thread library information (placed before the TCB).

ruiu added inline comments.Jun 1 2018, 9:56 AM
lld/ELF/InputSection.cpp
621–625 ↗(On Diff #149327)

Is that the only PPCv2 thing? I'm not worried about the code, but I want to improve the comment to answer questions that the reader of this code would have.

sfertile added inline comments.Jun 1 2018, 10:35 AM
lld/ELF/InputSection.cpp
621–625 ↗(On Diff #149327)

The code already assumes we know the differences between variant 1 and variant 2, would it help if it was explained here as variant 1 tls, but with the thread pointer offset past the TCB by TlsTpOffset? Then we could expand the comment around TcbSize and TlsTpOffset in the Target class to briefly explain the difference between the 2 variants as well as why these values being set essentially picks which variant to use.

ruiu added a comment.Jun 1 2018, 10:55 AM

That sounds good.

syzaara updated this revision to Diff 149668.Jun 3 2018, 9:16 PM
ruiu accepted this revision.Jun 5 2018, 8:52 AM

LGTM

lld/ELF/InputSection.cpp
627 ↗(On Diff #149668)

I often find that describing not only how it works but how it was designed that way is very useful, so let's write that too in this comment.

... by TlpTpOffset for efficient addressing TCB and up to 4 KB – 8 B of other thread library information (placed before the TCB).

This revision is now accepted and ready to land.Jun 5 2018, 8:52 AM
This revision was automatically updated to reflect the committed changes.