This is an archive of the discontinued LLVM Phabricator instance.

[clang][ThinLTO][Legacy] Don't add -fsplit-lto-unit for legacy thin LTO builds
ClosedPublic

Authored by evgeny777 on Oct 18 2019, 8:43 AM.

Details

Summary

This is an addition to D68950 which has been recently landed. It prevents adding -fsplit-lto-unit for Mac and PS4 thin LTO builds, because legacy LTO library is not capable of unit splitting.

Diff Detail

Event Timeline

evgeny777 created this revision.Oct 18 2019, 8:43 AM

A small suggestion for improvement.

lib/Driver/ToolChains/Clang.cpp
5400

I think this would be cleaner if we add a virtual function supportSplitLTOUnit to Toolchain, and let toolchain decide if it is supported, rather than hard code this in Clang.cpp.

evgeny777 updated this revision to Diff 225864.Oct 21 2019, 6:08 AM
evgeny777 marked an inline comment as done.

Addressed

lib/Driver/ToolChains/Clang.cpp
5400

It doesn't make sense to use virtual function, because there is no toolchain class for PS4. I'd suggest make an ordinary member function checking triple

That is not exactly what I meant. I meant:

in Toolchain.h: virtual bool canSplitThinLTOUnit() const { return true; }
in lib/lib/Driver/ToolChains/{PS4CPU.h,Darwin.h}: return false; // Using legacy LTO library

steven_wu added inline comments.Oct 21 2019, 10:35 AM
lib/Driver/ToolChains/Clang.cpp
5400

PS4CPU.h is not PS4 toolchain? If there is no PS4 toolchain, let's stick with your original patch.

evgeny777 marked an inline comment as done.Oct 22 2019, 8:03 AM
evgeny777 added inline comments.
lib/Driver/ToolChains/Clang.cpp
5400

Hm... I somehow didn't realized that PS4CPU is derived from ToolChain. I'll update the patch

This revision is now accepted and ready to land.Oct 23 2019, 7:36 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 24 2019, 4:18 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript