[Unix] Move all ELF SDK runtime libraries into their own architecture-specific directories #63782
Conversation
| set(output_sub_dir ${SWIFTLIB_SINGLE_SUBDIR}) | ||
| else() | ||
| # In the bootstrapping builds, we only have the single host architecture. | ||
| # So generated the library directly in the parent SDK specific directory | ||
| # So generate the Darwin library directly in the parent SDK specific directory |
There was a problem hiding this comment.
It may be possible to continue placing the bootstrapping libraries in a non-architecture-specific directory like on Darwin, then only link the rpath for the final bootstrapped Swift compiler against the architecture-specific directory, but I'm not interested in digging into all the CMake config to make that happen.
It is easier to simply place the bootstrapping libraries in architecture-specific directories too on ELF platforns. I don't check for Windows here because my understanding is that bootstrapping the compiler doesn't work on Windows: @compnerd, correct me if I'm wrong.
There was a problem hiding this comment.
Nope, bootstrapping isn't enabled yet on Windows.
| @@ -1253,11 +1253,12 @@ function(add_swift_target_library_single target name) | |||
| ${SWIFTLIB_SINGLE_C_COMPILE_FLAGS} "-DSWIFT_TARGET_LIBRARY_NAME=${name}") | |||
| set(link_flags ${SWIFTLIB_SINGLE_LINK_FLAGS}) | |||
|
|
|||
| set(library_search_subdir "${SWIFT_SDK_${SWIFTLIB_SINGLE_SDK}_LIB_SUBDIR}") | |||
There was a problem hiding this comment.
This is unused, so I removed it.
| @@ -2373,8 +2376,10 @@ function(add_swift_target_library name) | |||
| if (SWIFTLIB_BACK_DEPLOYMENT_LIBRARY) | |||
| # Back-deployment libraries get installed into a versioned directory. | |||
| set(install_dest "lib${LLVM_LIBDIR_SUFFIX}/${resource_dir}-${SWIFTLIB_BACK_DEPLOYMENT_LIBRARY}/${resource_dir_sdk_subdir}") | |||
| else() | |||
| elseif(sdk STREQUAL WINDOWS OR sdk IN_LIST SWIFT_DARWIN_PLATFORMS) | |||
There was a problem hiding this comment.
I generally kept things the same on Windows, even though it already moved to architecture-specific directories. @compnerd, if you want me to consolidate Windows more, as I did in the compiler, let me know.
There was a problem hiding this comment.
I am a fan of having fewer cases, as long as things continue to work, I don't mind seeing things becoming uniform :)
There was a problem hiding this comment.
I believe Windows is no different from Generic Unix here, so let’s make Darwin the only special case.
There was a problem hiding this comment.
I'm not sure what Windows needs, which is why I mentioned this. Before, all three of Windows, Darwin, and Unix were installing here to toolchain-os-arch/usr/lib/swift/os, which is only done when cross-compiling the stdlib for Windows because the first block above is invoked instead when natively compiling the stdlib on Windows.
Now that I'm moving Unix to this architecture-specific install directory, I wasn't sure if the same would work when cross-compiling the Windows stdlib, so I left it the same. If you think that will work for cross-compiling the Windows stdlib too, I will change it, but it will be untested, unless one of you tries it.
There was a problem hiding this comment.
I think that we should assume that it will be the same and should be safe to make this elseif (sdk IN_LIST SWIFT_DARWIN_PLATFORMS).
There was a problem hiding this comment.
OK, can you add that later for Windows? I don't want to kick off another whole CI run for this unimportant change, would rather just get these pulls in first.
| @@ -9,7 +9,7 @@ | |||
| // RUN: %empty-directory(%t) | |||
|
|
|||
| // RUN: %target-build-swift %S/Inputs/ConcreteTypes.swift %S/Inputs/GenericTypes.swift %S/Inputs/Protocols.swift %S/Inputs/Extensions.swift %S/Inputs/Closures.swift %S/Inputs/Conformances.swift -parse-as-library -emit-module -emit-library -module-name ConformanceCheck -o %t/Conformances | |||
| // RUN: %target-swift-reflection-dump %t/Conformances %platform-module-dir/%target-library-name(swiftCore) | %FileCheck %s | |||
| // RUN: %target-swift-reflection-dump %t/Conformances %platform-dylib-dir/%target-library-name(swiftCore) | %FileCheck %s | |||
There was a problem hiding this comment.
Now that the platform-dylib-dir is different from the platform-module-dir, this is the right path. Most tests use this platform-dylib-dir/%target-library-name formulation, except for two more mac-only tests added in recent years.
@artemcm or @al45tair, let me know if I should change those two also.
…-specific directories This is needed for all platforms that don't have multi-architecture libraries like Darwin. Also, add the new architecture-specific rpath to the resulting swift-driver. This is the Swift translation of the Driver-specific changes of swiftlang/swift#63782. Resolves #63645
…-specific directories This is needed for all platforms that don't have multi-architecture libraries like Darwin. Also, add the new architecture-specific rpath to the resulting swift-driver. This is the Swift translation of the Driver-specific changes of swiftlang/swift#63782. Resolves swiftlang/swift#63645
|
@al45tair, I see you're up, would you run the CI on this? @stevapple and @keith, I'd like to get this in before the 5.9 branch in a couple months, appreciate your input. |
|
@swift-ci please test |
|
@buttaface plz ask for CI access now 😛 . I think the value you'd get from having it outweighs the very slim security concern |
|
I +1 the idea for sure |
|
Alright, only one linux-only test from the compiler validation suite failing and macOS/Windows CIs essentially passing (late mac test failure is unrelated, other mac CI runs failed with that same test), good sign that this only affects the Unix toolchain. I will get this test fixed and submit the remaining six small ELF rpath change pulls to the corelibs for the runtime library directory next, so this passes the linux CI. |
…ELF platforms This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
…F platforms This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
finagolfin
force-pushed
the
multi-arch
branch
from
119f295 to
1d7dff9
Compare
| @@ -1,4 +1,4 @@ | |||
| // REQUIRES: OS=linux-gnu | |||
| // UNSUPPORTED: OS=macosx || OS=tvos || OS=ios || OS=watchos || OS=windows-msvc | |||
There was a problem hiding this comment.
Now enabled this one failing test on the linux CI for all Unix platforms, ie including the BSDs and Android.
…pecific directory. This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
… for the runtime libraries. This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
… for the runtime libraries. This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
|
I've submitted all the smaller pulls for other Swift repos, with which I was able to build a full Swift toolchain natively on Android with these runtime libraries moved to an architecture-specific directory. @keith, if you would run the CI with these seven pulls applied, let's see if it passes: swiftlang/swift-corelibs-libdispatch#780 |
|
@MaxDesiatov, mind running the CI on all the pulls listed in the last comment together? These pulls affect all non-Darwin platforms, so please test them with WASM and let me know if they work for you. Same for @3405691582 with OpenBSD and @mhjacobson with FreeBSD, try them out and let me know if I missed anything. |
… for the runtime libraries. This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
|
Linux and mac CI passed, single test failure on Windows is unrelated and was failing on all other pulls' Windows CI runs this morning. This is ready to go in, just need someone to merge. |
|
@egorzhdan, now that passing the Windows CI is required, please rerun the 11 pulls on the Windows CI alone, should pass as before. |
|
@bnbarham, would you trigger one last CI run with these 10 pulls only on the Windows CI? swiftlang/llvm-project#6383 |
|
@shahmishal, these 11 pulls can be merged now. I don't know if that requires special privileges that only you have to merge multiple pulls together, or if any committer can do that, which is why I'm asking you. |
|
Ping @al45tair, @shahmishal, can these 11 pulls go in before the branch? |
…ELF platforms This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.
There was a problem hiding this comment.
After some conversations, it would be better to rip the bandaid off now and move to full triples instead of having intermediate directory layouts. That way nothing can come up in the future (like having musl ABI on x86_64-linux) that will require us to change the format.
I'm not sure if we should just use the module triple that the driver already knows about for swift modules, or if we may need to include the version numbers (like how the Android API-level impacts what symbols are available in bionic, for instance). Thoughts?
|
Interesting, so Apple is also going to move from I think it would be a good idea to include the version numbering too in the triple-named directories, as one could distribute SDKs for different versioned triples like on macOS, Android, and the BSDs. Obviously, somebody at Apple would have to implement this larger change, but it would benefit all platforms. In the meantime, I don't see any problem with getting this in for now, as it creates no such new directories, and merely modifies around a dozen lines in the Swift compiler to reuse the existing architecture-specific directories. Whenever someone at Apple implements the triple-named directories, we can shift everything over to that. |
Our directory layout doesn't look like that. The runtimes on the various platforms are all installed in
I think the suggestion is that these PRs should be updated to use triples before they are merged. |
Yes, I meant the SDK runtime directories from Xcode that contain those dylibs, not the system runtime libraries in
That would be done in Of course, we could put in some platform-specific logic to only shift the Unix toolchain over to triple-named module directories and see how that works on Unix first, before shifting other platforms to triple-named resource directories. If whoever is in charge of this is willing to commit to shifting Unix alone to triple-named directories, I'm willing to spend a couple days changing these pull to implement that, just let me know. |
FWIW, they're in The only time they're in a directory with a platform name is in the @compnerd Windows also has a similar SDK-based approach, I believe? |
So you're saying all the different SDK dylibs for different Darwin platforms are placed directly in |
Yes. If you look at one of the SDKs, you'll see that the OTOH I don't know exactly what Windows does (@compnerd?), but I believe it has an SDK-like mechanism and in any case linking on Windows requires that you link against the |
The Windows SDK layout is exactly what this PR is proposing for generic Unix, except that Windows SDK has |
|
So swiftmodules aren't changing, they already have triples associated with them. e.g. With respect to libraries, as @al45tair said, we have fat binaries, and the linker and loader know how to pull out the appropriate slice for you architecture. When cross-compiling to another platform, you use the |
OK, so switching to triples wouldn't affect Darwin platforms then, but my question remains, are we going to switch all other platforms to triple-named module directories, like Windows and wasm? If just Unix platforms for now, I'll implement it if it's pre-approved in principle (obviously the work can still be held up for implementation problems or other issues, just not because somebody changed their mind above moving to triples). |
|
IMO Wasm would benefit from a switch to triples. While I don't see LLVM and Clang encoding Wasm features in triples right now, I know WASI came up with a new |
|
I do have some questions about using triple. The first problem is by doing so we’re diverging with Clang, while Clang libraries are available to Swift through Another problem is, if we still use strict triple matching in Swift, associating libraries with triples will make bigger troubles. eg., now we have |
If something somewhere is done that's unsuitable for Swift and prevents Swift from supporting more platforms in a clean way, it doesn't mean we have to perpetuate that decision and reimplement in our build system without ever questioning it.
The amount of triples Swift supports is fixed at a given time and these triples are known upfront and encoded in the build system. A random triple appearing out of the blue in |
So first, we've already diverged from Clang/normal runtime layouts. The expected place for your libraries is generally Second, it doesn't actually diverge that far from some uses of clang, and adding a triple certainly doesn't diverge from gcc where the cross-compiling toolchain is built for a specific triple. Looking at the Android NDK layout, you have Then inside of those you have your NDK-specific, non-ABI-stable libraries that don't ship on a given Android API-level ( So the question is, are we building stand-alone cross-compiling toolchains + sysroots, or are we augmenting an existing toolchain + sysroot? If we're augmenting, staying consistent with the layouts of the toolchain we're augmenting. For android, this means using a full triple. For the gcc toolchains I've worked with, this also means full triples. Going back to android for a sec, I think the only piece we're missing is that we do link the C++ stdlib for parts of the Swift runtime, so that unfortunately ties the built library to a specific NDK version because the C++ runtime symbols are versioned on the NDK, which isn't tracked in this scheme. I think technically we should be tracking the API level and NDK version, but I don't know how we would want to do that for this case. It's sort of specialized. Maybe the answer is that we have an augmented NDK with the Swift pieces? Then we don't have to worry about the different ABIs of each NDK. I don't know. If we're building a standalone toolchain(s), we have a bit more flexibility in the layout. The Xcode SDKs layout is pretty deep, but works reasonably well for preventing collisions between platforms and triples. You have the I haven't had time to fully formalize, propose, or implement all of this, but I think this ends up covering most of our requirements, with the exception of version numbers. |
|
Correction, for Ubuntu and Debian based things |
Interesting, I'm looking into adding separate Swift runtime library packages on Android, which is why I recently rolled these patches into the upcoming 5.8 release, so that one
The Termux app that I package Swift for uses a similar layout for cross-compilation, ie Of course, these install path changes are only relevant for "system" installs, not for standalone toolchains, though they could one day be upstreamed as a separate build option for system installers to use. Given the various issues you point out for linux triples, I'm not going to implement that either, as I suggested a couple days ago, as I don't even use Swift on linux, whereas these 11 pulls I proposed were just reusing the existing layout so they worked on all currently supported non-Darwin platforms. What I can do is rework these pulls to only use triple-named directories for Android, and we can use that to test out such a move, before devs on other platforms like linux, Windows, and Wasm submit their own separate pulls when they are ready to move to triples. One further issue when moving to triples, should I move all 13 files in It wouldn't make much sense to put them in @etcwilde, let me know what you think. |
|
Closing since the Swift devs apparently want to take a different approach, but it has not been decided what that approach will be. I simply moved these libraries to different locations for my Android SDK and native toolchain, which required further configuration since the current defaults that I tried to change with these pulls do not allow multi-arch setups. Whenever the Swift devs figure out how you want to implement this instead, I will check your work on Android. |
This is needed for all platforms that don't have multi-architecture libraries like Darwin.
Resolves #63645
I built this with the Feb. 9 source snapshot natively on my Android phone and saw no regressions in the compiler validation suite. I couldn't test the bootstrapping changes on Android because of #60272, so I tried to build a more recent snapshot on linux x86_64 without this pull and that wouldn't build either. I didn't want to look into that issue with bootstrapping the compiler on linux, so I left it.
This pull will require additional small pulls for the corelibs and some SPM products, which I will submit next. In the meantime, let's see if this passes the compiler validation suite first. I've also submitted the swift-driver equivalent, swiftlang/swift-driver#1294.
@Azoy, would you trigger the CI for me?