[CMake] Don't LTO optimize targets on Darwin either

[phosek]

Summary

This is a follow up to D102732 which also expands the logic to Darwin.

[smeenai]

Hmm, LLD for Mach-O already has some support for LTO. @int3, do we expect --lto-O0 to work for it?

[int3]

Nope, we didn't add support for --lto-O0. I *think* the ld64 equivalent is -flto-codegen-only, but we don't support that yet either (and it's undocumented). However, we do support -mllvm, so -mllvm,-O0 should work.

[phosek]

Shall I drop the NOT LINKER_IS_LLD check so it applies to both ld64 and ld64.lld?

[smeenai]

In D104764#2836658, @phosek wrote:

Shall I drop the NOT LINKER_IS_LLD check so it applies to both ld64 and ld64.lld?

Sounds good.

[smeenai]

LGTM, thanks!

[azharudd]

Should this apply to Darwin even when not using LLD?

I'm seeing an error which tracks down to this change. It works fine without the -mllvm,-O0 link flags.

FAILED: bin/llvm-tblgen 
[...snip...]
Remaining virtual register operands
UNREACHABLE executed at .../llvm-project/llvm/lib/CodeGen/MachineRegisterInfo.cpp:209!
clang-13: error: unable to execute command: Abort trap: 6
clang-13: error: linker command failed due to signal (use -v to see invocation)

https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/4221/consoleFull

[phosek]

In D104764#2895621, @azharudd wrote:

Should this apply to Darwin even when not using LLD?

I'm seeing an error which tracks down to this change. It works fine without the -mllvm,-O0 link flags.

FAILED: bin/llvm-tblgen 
[...snip...]
Remaining virtual register operands
UNREACHABLE executed at .../llvm-project/llvm/lib/CodeGen/MachineRegisterInfo.cpp:209!
clang-13: error: unable to execute command: Abort trap: 6
clang-13: error: linker command failed due to signal (use -v to see invocation)

https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/4221/consoleFull

It should, ld64 supports passing flags to libLTO via the -mllvm flag. We have been using this on our macOS bots for a few weeks and haven't run into any issues (they are using using ld64, not lld).

[azharudd]

In D104764#2895638, @phosek wrote:

In D104764#2895621, @azharudd wrote:

Should this apply to Darwin even when not using LLD?

I'm seeing an error which tracks down to this change. It works fine without the -mllvm,-O0 link flags.

FAILED: bin/llvm-tblgen 
[...snip...]
Remaining virtual register operands
UNREACHABLE executed at .../llvm-project/llvm/lib/CodeGen/MachineRegisterInfo.cpp:209!
clang-13: error: unable to execute command: Abort trap: 6
clang-13: error: linker command failed due to signal (use -v to see invocation)

https://green.lab.llvm.org/green/job/clang-stage2-Rthinlto/4221/consoleFull

It should, ld64 supports passing flags to libLTO via the -mllvm flag. We have been using this on our macOS bots for a few weeks and haven't run into any issues (they are using using ld64, not lld).

@phosek, the config where I'm seeing this is a two stage bootstrap build. Could you try building again with the just built clang and see if it still works for you? I seem to be able to reliably reproduce the error locally.

[phosek]

In D104764#2895650, @azharudd wrote:

@phosek, the config where I'm seeing this is a two stage bootstrap build. Could you try building again with the just built clang and see if it still works for you? I seem to be able to reliably reproduce the error locally.

Yes, that's what we use on our bots as well. However, I just noticed that your bots are using ThinLTO, is that correct? We always use full LTO and it's possible that this issue only manifests in ThinLTO.

[azharudd]

In D104764#2895658, @phosek wrote:

In D104764#2895650, @azharudd wrote:

@phosek, the config where I'm seeing this is a two stage bootstrap build. Could you try building again with the just built clang and see if it still works for you? I seem to be able to reliably reproduce the error locally.

Yes, that's what we use on our bots as well. However, I just noticed that your bots are using ThinLTO, is that correct? We always use full LTO and it's possible that this issue only manifests in ThinLTO.

Yes, correct, this is with ThinLTO. I tried full LTO and I don't see the error, so, yes this appears to be an issue only with ThinLTO.

For now, let me guard this behavior to if its not ThinLTO, until we get the root cause fixed.

llvm/cmake/modules/AddLLVM.cmake

 function(add_link_opts target_name)
   get_llvm_distribution(${target_name} in_distribution in_distribution_var)
   if(NOT in_distribution)
     # Don't LTO optimize targets that aren't part of any distribution.
     if (LLVM_ENABLE_LTO)
       # We may consider avoiding LTO altogether by using -fembed-bitcode
       # and teaching the linker to select machine code from .o files, see
       # https://lists.llvm.org/pipermail/llvm-dev/2021-April/149843.html
-      if((UNIX OR MINGW) AND LLVM_USE_LINKER STREQUAL "lld")
+      if((UNIX OR MINGW) AND LINKER_IS_LLD)
         set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                       LINK_FLAGS " -Wl,--lto-O0")
       elseif(LINKER_IS_LLD_LINK)
         set_property(TARGET ${target_name} APPEND_STRING PROPERTY
                       LINK_FLAGS " /opt:lldlto=0")
+      elseif(APPLE AND NOT LINKER_IS_LLD)
+        set_property(TARGET ${target_name} APPEND_STRING PROPERTY
+                      LINK_FLAGS " -Wl,-mllvm,-O0")
       endif()
     endif()
   endif()
 
   # Don't use linker optimizations in debug builds since it slows down the
   # linker in a context where the optimizations are not important.
   if (NOT uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
 
function(add_unittest test_suite test_name)
 
   set(LLVM_REQUIRES_RTTI OFF)
 
   list(APPEND LLVM_LINK_COMPONENTS Support) # gtest needs it for raw_ostream
   add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO NO_INSTALL_RPATH ${ARGN})
 
   # The runtime benefits of LTO don't outweight the compile time costs for tests.
   if(LLVM_ENABLE_LTO)
-    if((UNIX OR MINGW) AND LLVM_USE_LINKER STREQUAL "lld")
+    if((UNIX OR MINGW) AND LINKER_IS_LLD)
       set_property(TARGET ${test_name} APPEND_STRING PROPERTY
                     LINK_FLAGS " -Wl,--lto-O0")
     elseif(LINKER_IS_LLD_LINK)
       set_property(TARGET ${test_name} APPEND_STRING PROPERTY
                     LINK_FLAGS " /opt:lldlto=0")
+    elseif(APPLE AND NOT LINKER_IS_LLD)
+      set_property(TARGET ${target_name} APPEND_STRING PROPERTY
+                    LINK_FLAGS " -Wl,-mllvm,-O0")
     endif()
   endif()
 
   set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
   set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir})
   # libpthreads overrides some standard library symbols, so main
   # executable must be linked with it in order to provide consistent
   # API for all shared libaries loaded by this executable.

llvm/cmake/modules/HandleLLVMOptions.cmake

 # This CMake module is responsible for interpreting the user defined LLVM_
 # options and executing the appropriate CMake commands to realize the users'
 # selections.
 
 # This is commonly needed so make sure it's defined before we include anything
 # else.
 string(TOUPPER "${CMAKE_BUILD_TYPE}" uppercase_CMAKE_BUILD_TYPE)
 
 include(CheckCompilerVersion)
 include(HandleLLVMStdlib)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckSymbolExists)
 include(CMakeDependentOption)
 include(LLVMProcessSources)
 
+if(CMAKE_LINKER MATCHES ".*lld" OR (LLVM_USE_LINKER STREQUAL "lld" OR LLVM_ENABLE_LLD))
+  set(LINKER_IS_LLD TRUE)
+else()
+  set(LINKER_IS_LLD FALSE)
+endif()
+
 if(CMAKE_LINKER MATCHES "lld-link" OR (MSVC AND (LLVM_USE_LINKER STREQUAL "lld" OR LLVM_ENABLE_LLD)))
   set(LINKER_IS_LLD_LINK TRUE)
 else()
   set(LINKER_IS_LLD_LINK FALSE)
 endif()
 
 set(LLVM_ENABLE_LTO OFF CACHE STRING "Build LLVM with LTO. May be specified as Thin or Full to use a particular kind of LTO")
 string(TOUPPER "${LLVM_ENABLE_LTO}" uppercase_LLVM_ENABLE_LTO)