[PATCH] D30240: enable building with LTO on Windows using clang-cl and lld

[Bob Haarman via Phabricator via llvm-commits]

inglorion added inline comments.


================
Comment at: cmake/modules/HandleLLVMOptions.cmake:416
+    # or using LTO (/Brepro with LTO would result in a warning about the flag
+    # being unused, because we're not generating object files).
     # This checks CMAKE_CXX_COMPILER_ID in addition to check_cxx_compiler_flag()
----------------
hans wrote:
> Maybe we should just make `/Brepro` be ignored with `-flto` in clang-cl instead of making users remove the flag?
I considered that, but it's not what clang (non-cl) does. I would be happy to implement that (for both?) if we feel that is the right way to go. For now, I decided to keep clang-cl consistent with clang and change the build configuration to not pass an assembler flag when we're not using the assembler.


================
Comment at: cmake/modules/HandleLLVMOptions.cmake:707
+  if(NOT CMAKE_LINKER MATCHES "link.exe$")
+    append("-flto=thin" CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
+  endif()
----------------
mehdi_amini wrote:
> hans wrote:
> > Does this mean that if the user sets `LLVM_ENABLE_LTO` but forgets to set the build to use lld, we silently don't do LTO? Maybe we should error in the non-lld case instead?
> Especially since otherwise the link will fail badly right? Preventing from generating an invalid configuration seems better to me.
> 
Actually, using LLVM_ENABLE_LTO with MSVC's linker would fail with and without this change. Having said that, I agree it makes sense to think about how we can make this friendlier while we're making changes anyway.


https://reviews.llvm.org/D30240