This is an archive of the discontinued LLVM Phabricator instance.

[lld] COFF: Implement parallel LTO code generation.
ClosedPublic

Authored by pcc on Aug 24 2015, 8:08 PM.

Details

Summary

This is exposed via a new flag /opt:lldltojobs=N, where N is the number of
code generation threads.

Depends on D12260

Diff Detail

Repository
rL LLVM

Event Timeline

pcc updated this revision to Diff 33041.Aug 24 2015, 8:08 PM
pcc retitled this revision from to [lld] COFF: Implement parallel LTO code generation..
pcc updated this object.
pcc added a reviewer: ruiu.
pcc added a subscriber: llvm-commits.
ruiu added inline comments.Aug 24 2015, 8:20 PM
COFF/Driver.cpp
382 ↗(On Diff #33041)

Remove { and }.

COFF/SymbolTable.cpp
377 ↗(On Diff #33041)

The type of Obj is not obvious from the context, so please use its real type.

377 ↗(On Diff #33041)

Please use the real type instead of "auto".

418 ↗(On Diff #33041)

Why std::list? (My understanding is std::list is generally slower than std::vector, and there seems no obvious reason to use that instead of std::vector here.)

420 ↗(On Diff #33041)

Ditto

420 ↗(On Diff #33041)

Use real type.

429 ↗(On Diff #33041)

Ditto

429 ↗(On Diff #33041)

Ditto

COFF/SymbolTable.h
111 ↗(On Diff #33041)

Previously, LTOMB is a member of this class to keep the ownership of the memory buffer. Now this member owns nothing. Can we make this a function-local variable now?

pcc updated this revision to Diff 33049.Aug 24 2015, 11:13 PM
  • Address comments
COFF/Driver.cpp
382 ↗(On Diff #33041)

Done, also for lldlto=.

COFF/SymbolTable.cpp
377 ↗(On Diff #33041)

Done

418 ↗(On Diff #33041)

Because I collect a vector of pointers in OSPtrs as I go. If I used std::vector here the pointers to the vector elements could be invalidated on insertion. I've added a comment here for clarity. (This is a little awkward. I posted an idea for a better API in D12260.)

420 ↗(On Diff #33041)

Done

429 ↗(On Diff #33041)

Done.

COFF/SymbolTable.h
111 ↗(On Diff #33041)

We still need to own the memory buffers; the ownership in this case comes from the SmallVector objects in this data member.

ruiu accepted this revision.Aug 25 2015, 12:03 AM
ruiu edited edge metadata.
ruiu added inline comments.
COFF/SymbolTable.cpp
418 ↗(On Diff #33049)

Aha, understood. Maybe I would give an initial capacity to avoid std::vector's resizing instead of std::list, but this code is okay.

This revision is now accepted and ready to land.Aug 25 2015, 12:03 AM
This revision was automatically updated to reflect the committed changes.