This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Introduce std::vector<InputFile *> global arrays.
ClosedPublic

Authored by grimar on Jul 28 2017, 3:52 AM.

Details

Summary

This patch removes lot of static Instances arrays from different input file
classes and introduces global arrays for access instead. Similar to arrays we
have for InputSections/OutputSectionCommands.

It allows to iterate over input files in a non-templated code.

Intented to be used together with
D35936 ("[ELF] - Move getSymbols() method to InputFile")
in D35843 ("[ELF] - Fix "--symbol-ordering-file doesn't work with linker scripts") patch,
more details is in description for D35936 patch.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Jul 28 2017, 3:52 AM
ruiu edited edge metadata.Jul 28 2017, 10:37 AM

Hmm, this also looks odd to me. At least it's not straightforward. You put everything in one bag and then search from the bag, while you could separate them from beginning

grimar updated this revision to Diff 108908.Jul 31 2017, 5:41 AM
grimar retitled this revision from [ELF] - Introduce `std::vector<InputFile *> InputFiles` global array. to [ELF] - Introduce std::vector<InputFile *> global arrays..
grimar edited the summary of this revision. (Show Details)
  • Split single grobal array of input files into several.
In D35987#824273, @ruiu wrote:

Hmm, this also looks odd to me. At least it's not straightforward. You put everything in one bag and then search from the bag, while you could separate them from beginning

What do you think about updated version ? (btw it can be reduced slightly if we land D35936 first, though this can be done afterward if we land it too).

grimar updated this revision to Diff 115397.Sep 15 2017, 4:52 AM
  • Rebased.
  • Removed some relative templating to demonstrate this change can be useful.
ruiu added a comment.Sep 15 2017, 10:39 AM

It doesn't look as bad as I first thought. It is perhaps an improvement.

ELF/SymbolTable.cpp
595 ↗(On Diff #115397)

Remove this temporary variable.

ELF/Writer.cpp
118 ↗(On Diff #115397)

Remove template.

ruiu added a comment.Sep 15 2017, 10:41 AM

(But I'm not too excited about "remove templates as many as possible" patches. Some template uses are legitimate. Please do not try too hard.)

grimar updated this revision to Diff 115654.Sep 18 2017, 8:05 AM
  • Addressed review comments.
ELF/SymbolTable.cpp
595 ↗(On Diff #115397)

Done.

ELF/Writer.cpp
118 ↗(On Diff #115397)

Done.

ruiu accepted this revision.Sep 18 2017, 6:17 PM

LGTM

This revision is now accepted and ready to land.Sep 18 2017, 6:17 PM
This revision was automatically updated to reflect the committed changes.