[cmake] Prevent test failures by disabling LTO for swift runtime

[thevinster]

We build a unified LLVM build with Swift and when enabling LTO, swift validation test suite fails for several ELF tests. The swift compiler defaults to the gold linker in these tests and fails to recognize bitcode files as inputs. This is also known issue for gold LTO as documented here (https://github.com/apple/swift/blob/main/lib/Driver/UnixToolChains.cpp#L166-L168). To prevent such test failures, prevent certain libraries, swiftrt and swiftDemangling from compiling with LTO.

[drodriguez]

@swift-ci please smoke test

[drodriguez]

@swift-ci please smoke test Linux platform

[drodriguez]

@al45tair: Just in case you see some drawback to this approach. We build Swift as an external project of LLVM to speed up the build in our setup. When trying to enable LTO in the LLVM side, and testing Swift, the runtime is a LTO object which Gold cannot use (we use LLD for linking, but the test suite is not prepared for that). Explicitly disabling LTO for those two object libraries seems like the best idea (it also leaves them as object files in the SDK, which any linker can use).

[al45tair]

I think both of these changes should be conditional on the linker actually being gold, since that's where the problem lies — and we don't use gold every platform (not even every Linux platform, since Android doesn't use it).

Something like

# LTO doesn't work on gold
set(gold_no_lto)
if ("${SWIFT_USE_LINKER}" STREQUAL "gold")
  set(gold_no_lto "-fno-lto")
endif()

then replace the -fno-lto with ${gold_no_lto}.

[thevinster]

@al45tair Thanks for the review! I don't think it's possible to gate it with SWIFT_USE_LINKER because the problem isn't that the linker we used is gold for building Swift (we use lld actually). The problem is that swiftc used in the test suite defaults to gold unless -fuse-ld is provided, and gold runs into issues when it sees bitcode files instead of native objects. In short of making lld the default linker for ELF, this seems to be the lesser of two evils.

[al45tair]

Hmmmm. I see. What I'm concerned about here is that we're effectively turning off LTO for Darwin because it doesn't work when you build on Linux in the manner that you're doing, even though Darwin doesn't have this problem because we don't use gold there. So maybe if it isn't conditional on SWIFT_USE_LINKER, the extra flag should be conditional on building for Linux.

Unfortunately, add_swift_target_library() doesn't seem to have the necessary code for that already — it does have platform specific compiler flags for Swift, but not for C. Basically, after

      set(swiftlib_c_compile_flags_all ${SWIFTLIB_C_COMPILE_FLAGS})

on line 2027 of stdlib/cmake/modules/AddSwiftStdlib.cmake we need to add something like the stanza on line 1876, e.g.

    if(${sdk} STREQUAL OSX)
      list(APPEND swiftlib_c_compile_flags_all
           ${SWIFTLIB_C_COMPILE_FLAGS_OSX})
    elseif(${sdk} STREQUAL IOS OR ${sdk} STREQUAL IOS_SIMULATOR)
      list(APPEND swiftlib_c_compile_flags_all
           ${SWIFTLIB_C_COMPILE_FLAGS_IOS})
    elseif(${sdk} STREQUAL TVOS OR ${sdk} STREQUAL TVOS_SIMULATOR)
      list(APPEND swiftlib_c_compile_flags_all
           ${SWIFTLIB_C_COMPILE_FLAGS_TVOS})
    elseif(${sdk} STREQUAL WATCHOS OR ${sdk} STREQUAL WATCHOS_SIMULATOR)
      list(APPEND swiftlib_c_compile_flags_all
           ${SWIFTLIB_C_COMPILE_FLAGS_WATCHOS})
    elseif(${sdk} STREQUAL LINUX)
      list(APPEND swiftlib_c_compile_flags_all
           ${SWIFTLIB_C_COMPILE_FLAGS_LINUX})
    elseif(${sdk} STREQUAL WINDOWS)
      list(APPEND swiftlib_c_compile_flags_all
           ${SWIFTLIB_C_COMPILE_FLAGS_WINDOWS})
    endif()

and we need to declare the new options on line 1640, i.e.

  set(SWIFTLIB_multiple_parameter_options
        C_COMPILE_FLAGS
        C_COMPILE_FLAGS_IOS
        C_COMPILE_FLAGS_OSX
        C_COMPILE_FLAGS_TVOS
        C_COMPILE_FLAGS_WATCHOS
        C_COMPILE_FLAGS_LINUX
        ...

Then your change will just need to add

    C_COMPILE_FLAGS_LINUX -fno-lto

in both places.

[thevinster]

@al45tair I implemented the changes as you suggested. It works when I tried it building for Darwin and for Linux so I think this change should be safe.

@drodriguez Could you help me smoke test this again just to make sure it also passes in upstream CI?

[drodriguez]

@swift-ci please smoke test

[al45tair]

LGTM

[al45tair]

Linux build failure looks unrelated to these changes.

[al45tair]

@swift-ci Please smoke test Linux platform

[thevinster]

@al45tair Would it be possible to merge this change into release/5.7 branch as well? We build and track against the 5.7 release branch, and it would be nice to not have to cherry-pick changes locally.

[thevinster]

Created a different PR that cherry-picks this change to release/5.7.

[al45tair]

It might be possible; the issue there is that you'll have to persuade one of the branch managers that it's important enough to merge it there.

[al45tair]

I've pinged the branch managers (they'll get a notification anyway because you raised the PR, but it's a good idea to ask them as well). The main thing is to emphasise that this has no impact except on Linux, where it fixes a link problem. That makes it less risky to merge.