This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Get rid of LinkerScript::adjustSectionsBeforeSorting().
AbandonedPublic

Authored by grimar on Oct 5 2017, 6:19 AM.

Details

Reviewers
ruiu
rafael
Summary

I suggest to get rid of LinkerScript::adjustSectionsBeforeSorting() method.
It is used to force output sections that does not have any input sections
to be live when they have data bytes commands or assignments inside.

Probably there is no reason to delay that and do separatelly, I believe it
is simpler to set this flag when we do linker script commands proccessing,
as at that place we already know everything we need to do that.

Diff Detail

Event Timeline

grimar created this revision.Oct 5 2017, 6:19 AM
grimar updated this revision to Diff 117812.Oct 5 2017, 6:40 AM
  • Better version.
ruiu added inline comments.Oct 5 2017, 8:16 PM
ELF/LinkerScript.cpp
441–444

I don't like to do more things in a single for-loop. Previously, this loop was just three lines. Now it does more things. It is hard to understand now. Please try to split it into smaller pieces.

449–457

I really do not understand how this comment explains the following code.

grimar updated this revision to Diff 118899.Oct 13 2017, 5:26 AM
  • Addressed review comments.
grimar updated this revision to Diff 118900.Oct 13 2017, 5:39 AM
  • Cosmetic changes.
grimar updated this revision to Diff 119660.Oct 20 2017, 8:30 AM

Ping.

  • Rebased, fixed comment.
ruiu edited edge metadata.Oct 24 2017, 9:18 PM

Hmm, does this actually make things easier to understand? I'm not sure about that. At least, when I ask you to split the loop, I was talking about the outermost loop. You created a small function for the innermost part, but that's not what I meant.

grimar planned changes to this revision.Oct 30 2017, 7:20 AM

I created D39418 to split the loop.
I'll rebase this patch later if/when we land above one.

Also it may worth to land D39094 to simplify it a bit too.

grimar abandoned this revision.Dec 1 2017, 4:18 AM