This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Implemented @tlsgd optimization (GD->IE case, x64).
ClosedPublic

Authored by grimar on Nov 25 2015, 3:15 PM.

Details

Summary

"Ulrich Drepper, ELF Handling For Thread-Local Storage" (5.5 x86-x64 linker optimizations, http://www.akkadia.org/drepper/tls.pdf) shows how GD can be optimized to IE.
This patch implements this optimization.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 41192.Nov 25 2015, 3:15 PM
grimar retitled this revision from to [ELF] - Implemented @tlsgd optimization (GD->IE case)..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
ruiu added inline comments.Nov 25 2015, 3:18 PM
ELF/OutputSections.cpp
226–231 ↗(On Diff #41192)

There is really a concerning trend that this function gets longer and longer over time. Probably it's time to split up into multiple small functions with comments.

grimar updated this revision to Diff 41412.Nov 30 2015, 10:19 AM
grimar retitled this revision from [ELF] - Implemented @tlsgd optimization (GD->IE case). to [ELF] - Implemented @tlsgd optimization (GD->IE case, x64)..

Review comments addressed.

ELF/OutputSections.cpp
226–231 ↗(On Diff #41192)

Done.

grimar updated this revision to Diff 41622.Dec 2 2015, 7:29 AM

Refactored, rebased.

ruiu added inline comments.Dec 2 2015, 3:10 PM
ELF/OutputSections.cpp
221–226 ↗(On Diff #41622)

Can you reduce the nest level by moving this if outside of the outer `if? I mean

if (Body && Target->isTlsOptimized(Type, Body)) {
  P->setSymbolAndType(Body->getDynamicSymbolTableIndex(),
                      Target->getTlsGotReloc(), Config->Mips64EL);
  P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
  return true;
}

if (Body && Target->isTlsGlobalDynamicReloc(Type)) {
  ...
}
ELF/Target.cpp
494–495 ↗(On Diff #41622)

I don't think these magic numbers are good. In fact it seems that you don't need them. Please see the comment below.

624 ↗(On Diff #41622)

This can be

if (canBePreempted(&S, true))
grimar updated this revision to Diff 41769.Dec 3 2015, 9:38 AM

Addressed review comments.

ELF/OutputSections.cpp
221–226 ↗(On Diff #41622)

Refactored, what about this one ?

ELF/Target.cpp
494–495 ↗(On Diff #41622)

Cool ! Fixed.

624 ↗(On Diff #41622)

Done.

ruiu accepted this revision.Dec 3 2015, 10:26 AM
ruiu edited edge metadata.

LGTM with a few nits.

ELF/Target.cpp
499 ↗(On Diff #41769)

Remove the outermost ().

548 ↗(On Diff #41769)

Remove newline at end of this line.

This revision is now accepted and ready to land.Dec 3 2015, 10:26 AM
This revision was automatically updated to reflect the committed changes.