This is an archive of the discontinued LLVM Phabricator instance.

[LTO] SplitCodeGen new API
ClosedPublic

Authored by davide on Apr 14 2016, 4:21 PM.

Details

Summary

.. so that we don't have to create TM in two places in lld. See http://reviews.llvm.org/D18999 for reference.
Hopefully should work, assuming TheTarget is thread-safe. Thanks to Michael Spencer for help with this patch.

Diff Detail

Event Timeline

davide updated this revision to Diff 53804.Apr 14 2016, 4:21 PM
davide retitled this revision from to [LTO] SplitCodeGen new API.
davide updated this object.
davide added reviewers: Bigcheese, rafael, pcc.
davide set the repository for this revision to rL LLVM.
davide added subscribers: llvm-commits, mehdi_amini.

Looks OK, see some inline comments.

include/llvm/CodeGen/ParallelCG.h
49

Is there a better name for F?
Also can you add a doxygen for this API (you can refer to the other saying this is a variant that takes a factory function for the TargetMachine)

49

Also: const &

lib/CodeGen/ParallelCG.cpp
29

Use another name than F, and take a const &.

51

Do you really mean = here? I'd use &.

Bigcheese added inline comments.Apr 14 2016, 4:41 PM
lib/CodeGen/ParallelCG.cpp
50–54

clang-format.

davide updated this revision to Diff 53814.Apr 14 2016, 4:56 PM
davide removed rL LLVM as the repository for this revision.

Comments addressed.

davide updated this revision to Diff 53815.Apr 14 2016, 4:57 PM

Correct upload.

mehdi_amini accepted this revision.Apr 14 2016, 5:03 PM
mehdi_amini added a reviewer: mehdi_amini.

LGTM, with a few more fixes (see inline)

include/llvm/CodeGen/ParallelCG.h
49

Add in the documentation the fact that TMFactory needs to be thread safe on the client side.

lib/CodeGen/ParallelCG.cpp
33

still missing const & here.

98

&TMFactory (avoid copy here).

This revision is now accepted and ready to land.Apr 14 2016, 5:03 PM

Thank you Mehdi, r266390.

This revision was automatically updated to reflect the committed changes.