Petr Hosek via llvm-dev
2020-Sep-22 18:57 UTC
[llvm-dev] Unifying CMake variable names used in checks across subprojects
We've been using the runtimes build for a while now and we're very happy with it. However, with an increasing number of targets, it can be fairly slow and I have noticed that we now spend more time in CMake than in Ninja. There are various ways we could improve things like eliminating unnecessary checks. When running checks like check_c_compiler_flag, check_cxx_compiler_flag or check_library_exists, CMake caches the resulting variable and doesn't run the check again. The problem is that in LLVM, each subproject uses different variable names for results of these checks. For example, most subprojects check if pthread is available and store the result in: COMPILER_RT_HAS_LIBPTHREAD (compiler-rt) LIBCXX_HAS_PTHREAD_LIB (libc++) LIBCXXABI_HAS_PTHREAD_LIB (libc++abi) LIBUNWIND_HAS_PTHREAD_LIB (libunwind) HAVE_LIBPTHREAD (llvm) This means that even though this check would ideally be performed just once (per target) and reused everywhere, it's performed 5 times. The same is true for most flags and library checks. I think that this is really unnecessary and could be easily improved by unifying CMake variable names used in checks across subprojects to benefit from caching. I've looked at naming conventions used across all subprojects and I'm proposing the following: C_SUPPORTS_${mangled_name}_FLAG for check_c_compiler_flag CXX_SUPPORTS_${mangled_name}_FLAG for check_cxx_compiler_flag HAVE_${mangled_name} for check_library_exists Note: It'd be more consistent for check_library_exists to use HAS_${mangled_name}_LIB but that's going to cause more churn in LLVM so that's something to consider. This change should be mostly invisible to LLVM developers (except for the handful of build maintainers), but it should considerably speed up the runtimes build and hopefully pave the way to eventually hoist most of the common CMake logic into a shared location. I'm happy to implement this change, but I want to get your opinion on the proposal as well as the proposed naming. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200922/c1df543f/attachment.html>
Eric Christopher via llvm-dev
2020-Sep-22 19:28 UTC
[llvm-dev] Unifying CMake variable names used in checks across subprojects
>From the "not largely affected" camp:- the churn doesn't feel that major for HAS_ and ... - the uniformity feels nice and in general feels nice and in pursuit of the longer term goals here. -eric On Tue, Sep 22, 2020 at 11:58 AM Petr Hosek via llvm-dev < llvm-dev at lists.llvm.org> wrote:> We've been using the runtimes build for a while now and we're very happy > with it. However, with an increasing number of targets, it can be fairly > slow and I have noticed that we now spend more time in CMake than in Ninja. > There are various ways we could improve things like eliminating unnecessary > checks. > > When running checks like check_c_compiler_flag, check_cxx_compiler_flag > or check_library_exists, CMake caches the resulting variable and doesn't > run the check again. The problem is that in LLVM, each subproject uses > different variable names for results of these checks. For example, most > subprojects check if pthread is available and store the result in: > > COMPILER_RT_HAS_LIBPTHREAD (compiler-rt) > LIBCXX_HAS_PTHREAD_LIB (libc++) > LIBCXXABI_HAS_PTHREAD_LIB (libc++abi) > LIBUNWIND_HAS_PTHREAD_LIB (libunwind) > HAVE_LIBPTHREAD (llvm) > > This means that even though this check would ideally be performed just > once (per target) and reused everywhere, it's performed 5 times. The same > is true for most flags and library checks. > > I think that this is really unnecessary and could be easily improved by > unifying CMake variable names used in checks across subprojects to benefit > from caching. > > I've looked at naming conventions used across all subprojects and I'm > proposing the following: > > C_SUPPORTS_${mangled_name}_FLAG for check_c_compiler_flag > CXX_SUPPORTS_${mangled_name}_FLAG for check_cxx_compiler_flag > HAVE_${mangled_name} for check_library_exists > > Note: It'd be more consistent for check_library_exists to > use HAS_${mangled_name}_LIB but that's going to cause more churn in LLVM so > that's something to consider. > > This change should be mostly invisible to LLVM developers (except for the > handful of build maintainers), but it should considerably speed up the > runtimes build and hopefully pave the way to eventually hoist most of the > common CMake logic into a shared location. > > I'm happy to implement this change, but I want to get your opinion on the > proposal as well as the proposed naming. > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200922/5997e795/attachment.html>
Louis Dionne via llvm-dev
2020-Sep-22 19:42 UTC
[llvm-dev] Unifying CMake variable names used in checks across subprojects
> On Sep 22, 2020, at 15:28, Eric Christopher <echristo at gmail.com> wrote: > > From the "not largely affected" camp: > > - the churn doesn't feel that major for HAS_ and ... > - the uniformity feels nice > > and in general feels nice and in pursuit of the longer term goals here. > > -eric > > On Tue, Sep 22, 2020 at 11:58 AM Petr Hosek via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > We've been using the runtimes build for a while now and we're very happy with it. However, with an increasing number of targets, it can be fairly slow and I have noticed that we now spend more time in CMake than in Ninja. There are various ways we could improve things like eliminating unnecessary checks. > > When running checks like check_c_compiler_flag, check_cxx_compiler_flag or check_library_exists, CMake caches the resulting variable and doesn't run the check again. The problem is that in LLVM, each subproject uses different variable names for results of these checks. For example, most subprojects check if pthread is available and store the result in: > > COMPILER_RT_HAS_LIBPTHREAD (compiler-rt) > LIBCXX_HAS_PTHREAD_LIB (libc++) > LIBCXXABI_HAS_PTHREAD_LIB (libc++abi) > LIBUNWIND_HAS_PTHREAD_LIB (libunwind) > HAVE_LIBPTHREAD (llvm) > > This means that even though this check would ideally be performed just once (per target) and reused everywhere, it's performed 5 times. The same is true for most flags and library checks. > > I think that this is really unnecessary and could be easily improved by unifying CMake variable names used in checks across subprojects to benefit from caching. > > I've looked at naming conventions used across all subprojects and I'm proposing the following: > > C_SUPPORTS_${mangled_name}_FLAG for check_c_compiler_flag > CXX_SUPPORTS_${mangled_name}_FLAG for check_cxx_compiler_flag > HAVE_${mangled_name} for check_library_existsIMO, these issues are a manifestation of the fact that we basically have (at least) 4 times the same overall logic, once for each runtime project: compiler-rt, libunwind, libcxxabi, libcxx. At the end of the day, what we're trying to achieve is link against the right system libraries when building the various runtimes. Would it make sense to instead bundle together the logic of searching for these libraries and adding the right compiler flags? We could use interface targets to achieve that. IOW, from libc++'s CMake, I'd love to just be able to write: target_link_libraries(cxx PUBLIC runtimes-system-libraries) This would add -lpthread, -lgcc -lgcc_s, -lSystem or whatever else is needed on the system. I think this approach would provide more build system simplification and be more robust in the long term than relying on a naming convention to achieve sharing. What do you think? Louis -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200922/ff337c40/attachment.html>