Refactor gdb index creation
ClosedPublic

Authored by rafael on Mon, Jul 10, 6:40 PM.

Details

Reviewers
ruiu
grimar
Summary

I have a patch to let DwarfContext defer to lld for getting section contents and relocations.

That is a pretty big performance improvement.

This is just a refactoring to make that easier to do.

This change makes the *creation* of gdb index a dedicated step and makes that templated. That is so that we can uses Elf_Rel in the code.

The change also removes the pubnames and pubtypes later, so that there is an InputSection for them that we can use.

Diff Detail

rafael created this revision.Mon, Jul 10, 6:40 PM

With the full patch (not just this refactoring) linking clang with debug index goes from 33.39 seconds to 20.59 seconds.

grimar added inline comments.Tue, Jul 11, 4:26 AM
ELF/GdbIndex.h
48

DebugInfo/DebugInfoSec may be ? Sec name has unclear meaning.

ELF/SyntheticSections.cpp
1763

Since this method now not only returns .debug_info sections, but also
drops .debug_gnu_pub* sections, should it have different name may be ?

1785

Probably you can call

Script->discard({S}};

for .debug_gnu_pub* sections instead of modifying InputSections.

ruiu added inline comments.Tue, Jul 11, 2:21 PM
ELF/InputFiles.cpp
504 ↗(On Diff #105956)

I think this is cleaner way of eliminating .debug_gnu_{pubnames,pubtypes} than doing it later. Is there any reason to move this code?

ELF/SyntheticSections.h
506

Don't you want to add &&?

rafael updated this revision to Diff 106280.Wed, Jul 12, 12:33 PM

Address review comments.

This is now just about moving the section creation earlier where it can be easily templated.

rafael updated this revision to Diff 106282.Wed, Jul 12, 12:38 PM

Remove leftover changes.

rafael updated this revision to Diff 106286.Wed, Jul 12, 12:42 PM

Upload correct version. Sorry about the noise.

ruiu accepted this revision.Wed, Jul 12, 2:01 PM

LGTM

ELF/SyntheticSections.cpp
1811

Looks like you can remove V as it is used only once.

ELF/SyntheticSections.h
539

nit: add a blank line before this line.

This revision is now accepted and ready to land.Wed, Jul 12, 2:01 PM
thakis closed this revision.Mon, Jul 17, 5:26 AM
thakis added a subscriber: thakis.

(landed in r307867)