This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] build compiler-rt runtimes without LTO
ClosedPublic

Authored by inglorion on Mar 21 2017, 3:28 PM.

Details

Summary

Currently, we build the compiler-rt runtimes with link-time optimization if LTO is configured for the LLVM project. This will break external programs that don't invoke the linker in such a way that it supports LLVM's LTO. To avoid this, this change causes the compiler-rt runtimes to be compiled with -fno-lto. This also makes the check-profile tests work on systems when doing a lld LTO build on a system where the system linker does not support LLVM LTO.

Diff Detail

Repository
rL LLVM

Event Timeline

inglorion created this revision.Mar 21 2017, 3:28 PM
rnk edited edge metadata.Mar 21 2017, 3:34 PM

I agree with this in principle. The high-level LLVM_ENABLE_LTO flag shouldn't affect compiler-rt. We should have a different way of getting compiler-rt bitcode libraries if we want them.

Are you sure it's best to do -flto=thin/full -fno-lto instead of filtering out -flto[^ ]*?

@rnk, thanks for taking a look. I have an alternative implementation that records which flags were added to enable LTO and provides functions to filter those out. That might lead to cleaner command lines. On the other hand, this is simpler and follows what we already do for SANITIZER_COMMON_FLAGS (in compiler-rt/CMakeLists.txt), so I decided to see what people think about this implementation, first.

davidxl edited edge metadata.Mar 21 2017, 3:45 PM

Even the system linker supports lto, there might be issues with bit code version mismatches etc. In general it is good idea to not build compiler rt with lto. It is a different story if we want to support cross module inlining of some compiler-rt functions in the future. For now it is fine, but with a brief comment there.

For the same reason, profile rt should not be built with -fprofile-generate or -fprofile-instr-generate, but we did not filter those out -- perhaps a different patch.

inglorion updated this revision to Diff 92563.Mar 21 2017, 4:01 PM

added comment, as suggested by @davidxl

davidxl accepted this revision.Mar 21 2017, 4:03 PM

lgtm

This revision is now accepted and ready to land.Mar 21 2017, 4:03 PM
This revision was automatically updated to reflect the committed changes.