This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Move section factory out from writer to make it reusable.
ClosedPublic

Authored by grimar on May 5 2016, 8:23 AM.

Details

Summary

Since I think we are all agree that linkerscript should create sections by itself
(if SECTIONS command is present),
then we might want to reuse the OutputSectionFactory (D19976 already do that now),
so this patch moves it out from writer cpp file for that purpose.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 56292.May 5 2016, 8:23 AM
grimar retitled this revision from to [ELF] - Move section factory out from writer to make it reusable..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar updated this object.
grimar added subscribers: llvm-commits, grimar.
ruiu edited edge metadata.May 5 2016, 6:34 PM

This is probably towards the right direction, but let's hold on until D19976 is settled.

ELF/OutputSections.h
617–621 ↗(On Diff #56292)

This class (actually a struct) doesn't know how to create output sections. You have moved the comment to a wrong place.

622 ↗(On Diff #56292)

I'd type it to ELFT rather than Is64Bits.

grimar updated this revision to Diff 56391.May 6 2016, 2:48 AM
grimar edited edge metadata.
  • Addressed review comment
grimar added a comment.May 6 2016, 2:50 AM
In D19977#423160, @ruiu wrote:

This is probably towards the right direction, but let's hold on until D19976 is settled.

Just in case:
If we land this one and D19981, then D19976 patch after rebase will be significantly reduced in size,
making it easier to review (and support).

ELF/OutputSections.h
617–621 ↗(On Diff #56292)

Fixed.

622 ↗(On Diff #56292)

This patch just moves existent code without changes (at least that is what I expect from it). Improving factory relative code is another story and should be done separately I believe.

grimar updated this revision to Diff 63512.Jul 11 2016, 7:37 AM
  • Rebased.
  • Split declaration and definition to cpp/h files for OutputSectionFactory and DenseMapInfo<lld::elf::SectionKey<Is64Bits>>.
ruiu accepted this revision.Jul 11 2016, 2:03 PM
ruiu edited edge metadata.

LGTM

ELF/OutputSections.cpp
1812–1824 ↗(On Diff #63512)

Add blank lines between function definitions.

ELF/OutputSections.h
735 ↗(On Diff #63512)

Remove comment.

This revision is now accepted and ready to land.Jul 11 2016, 2:03 PM
This revision was automatically updated to reflect the committed changes.