This is an archive of the discontinued LLVM Phabricator instance.

[ORC][ORC_RT][AArch64] Implement TLS descriptor in ELFNixPlatform.
ClosedPublic

Authored by sunho on Jun 26 2022, 12:33 AM.

Details

Summary

Implements TLS descriptor relocations in JITLink ELF/AARCH64 backend and support the relevant runtime functions in ELFNixPlatform.

Unlike traditional TLS model, TLS descriptor model requires linker to return the "offset" from thread pointer via relocaiton not the actual pointer to thread local variable. There is no public libc api for adding new allocations to TLS block dynamically which thread pointer points to. So, we support this by taking delta from thread base pointer to the actual thread local variable in our allocated section.

Diff Detail

Event Timeline

sunho created this revision.Jun 26 2022, 12:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 26 2022, 12:33 AM
sunho requested review of this revision.Jun 26 2022, 12:33 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 26 2022, 12:33 AM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
sunho edited the summary of this revision. (Show Details)Jun 26 2022, 12:37 AM
sunho updated this revision to Diff 440044.Jun 26 2022, 12:57 AM
sunho edited the summary of this revision. (Show Details)Jun 26 2022, 4:36 AM
sunho updated this revision to Diff 442130.Jul 4 2022, 11:46 AM
sunho edited the summary of this revision. (Show Details)

Use delta.

lhames added a comment.Jul 4 2022, 6:46 PM

Amazing -- TLV was supposed to be a stretch goal. ;)

compiler-rt/lib/orc/elfnix_tls.aarch64.S
20

Could this re-entry code have a fast-path for instances once they've been allocated (as in https://github.com/llvm/llvm-project/issues/51162, though obviously the specifics would be different for TLSDESC). If so it's probably worth a comment here "// TODO: add fast-path for repeat access".

llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h
25

Is Nop actually needed here, or could we just not introduce the edge?

34–35

I think that these (TLVPage21 and TLVPageOffset12) should stay in: The idea is to have the edge kinds in the initially parsed graph be very descriptive (e.g. "I am a page21-expr offset _to a TLV pointer_") so that early passes have a clear picture of what the graph represents.

226–228

Braces aren't needed here, since no new decls are introduced.

sunho updated this revision to Diff 442325.Jul 5 2022, 9:10 AM

Addres comments.

sunho marked 3 inline comments as done.Jul 5 2022, 9:11 AM
lhames accepted this revision.Jul 5 2022, 3:25 PM

Otherwise LGTM. Thank you again! :)

llvm/lib/ExecutionEngine/JITLink/ELF_aarch64.cpp
320

Can we just return Error::success(); here, rather than setting a skip variable?

This revision is now accepted and ready to land.Jul 5 2022, 3:25 PM
sunho updated this revision to Diff 442491.Jul 6 2022, 3:02 AM
This revision was landed with ongoing or failed builds.Jul 6 2022, 4:13 AM
This revision was automatically updated to reflect the committed changes.