This is an archive of the discontinued LLVM Phabricator instance.

Reduce the number of components initialized for LLGS further.
ClosedPublic

Authored by flackr on Mar 6 2015, 10:13 AM.

Details

Summary

In http://reviews.llvm.org/D7880 the initialization for LLGS was separated out so that LLGS could initialize only the components it needs to. This further reduces the set of components initialized for LLGS.

Diff Detail

Repository
rL LLVM

Event Timeline

flackr updated this revision to Diff 21369.Mar 6 2015, 10:13 AM
flackr retitled this revision from to Reduce the number of components initialized for LLGS further..
flackr updated this object.
flackr edited the test plan for this revision. (Show Details)
flackr added reviewers: tberghammer, clayborg.
flackr set the repository for this revision to rL LLVM.
flackr added a subscriber: Unknown Object (MLST).
flackr edited the test plan for this revision. (Show Details)Mar 6 2015, 10:36 AM
tberghammer edited edge metadata.Mar 8 2015, 6:03 AM

It looks good to me, but I think you can remove the following modules also (haven't tested it):

ObjectContainerBSDArchive
ObjectContainerUniversalMachO
ObjectFileELF
ObjectFileMachO
ObjectFilePECOFF
DynamicLoaderPOSIXDYLD
DynamicLoaderMacOSXDYLD
DynamicLoaderDarwinKernel

vharron added a subscriber: vharron.Mar 8 2015, 8:19 AM

ObjectFileELF

LLGS needs this.

ObjectFileELF

LLGS needs this.

What is it used for?

AFAIK it is only used by ProcessElfCore what is currently not initialized by LLGS. Is it mean we have to put back the initialization of ProcessElfCore into LLGS?

LLGS uses it to confirm that the target file has the correct architecture
(e.g. x86_64) before launch. If it doesn't have ObjectFileELF plugin
loaded, nothing can parse the ELF file.

Thanks for your explanation. It makes sense, but I think LLGS not necessarily have to parse any ELF file because it can be done on LLDB side (it already can send down the architecture of an executable). I suggest to leave it in LLGS for now, and if later we want to reduce the size of LLGS further, then we can remove it (possibly after some re-factoring).

Agreed completely.

flackr added a comment.Mar 8 2015, 5:56 PM

LLGS uses it to confirm that the target file has the correct architecture
(e.g. x86_64) before launch. If it doesn't have ObjectFileELF plugin
loaded, nothing can parse the ELF file.

Yes, just to confirm this is exactly what I was seeing when I tried to remove this. The long term removal plan sounds good to me too.

tberghammer accepted this revision.Mar 10 2015, 3:27 AM
tberghammer edited edge metadata.

I don't see any removed component needed in LLGS and we can address the removal of the additional components later.

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