[PATCH] D102732: [CMake] Don't LTO optimize targets that aren't part of any distribution

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a subscriber: thakis.
smeenai added a comment.

This is a neat idea.



================
Comment at: llvm/CMakeLists.txt:934
 
+include(LLVMDistributionSupport)
+
----------------
Is this moved up just to get the distribution target map built in time for the new use?


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:221
 function(add_link_opts target_name)
+  if(LLVM_DISTRIBUTION_COMPONENTS OR LLVM_DISTRIBUTIONS)
+    get_property(distribution GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${target_name})
----------------
In https://reviews.llvm.org/D89177?vs=on&id=343946#change-XYKH4tDojBz0, I had a `get_llvm_distribution` target in LLVMDistributionSupport that would tell you whether a target was in a distribution or not. I removed it from the final version of that diff because I added a more specific function instead that handled all the use cases I had, but for this use case, I think adding that function back would be nicer, so that e.g. the LLVM_DISTRIBUTION_FOR_* property naming scheme remains centralized inside LLVMDistributionSupport.

The other thing that function handled was umbrella targets (e.g. making sure `llvm-libraries` pulled in all LLVM libraries). Do you care about that support here? I haven't actually thought about how LTO would work if you're distributing (static) libraries as part of your distribution, as in would you wanna build them as bitcode or regular archives.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:242
   # linker in a context where the optimizations are not important.
   if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
 
----------------
phosek wrote:
> I'm wondering if we should avoid this branch for non-distribution targets as well to further reduce cost, what do you think?
I think it'd make sense. Not related to your diff though, but depending on where the linker handles dead-stripping, I imagine it might actually make links *faster*, since it's reducing the amount of data the linker has to process. I think @thakis might have observed that when implementing dead-stripping in LLD for Mach-O.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102732/new/

https://reviews.llvm.org/D102732