Skip to content

[CMake] Add broader support for cross-compiling the portions of the compiler that are written in Swift to non-Darwin Unix #71552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

finagolfin
Copy link
Member

@finagolfin finagolfin commented Feb 12, 2024

Add cross-compilation flags for the newly added Swift source in lib/ASTGen/, similar to how SwiftCompilerSources/ is already cross-compiled for other platforms. Make sure the Swift source in the compiler builds and links against SWIFTLIB_DIR in this cross-compilation build directory, not the one that comes with the native host compiler.

This requires changing the dependency chain in CROSSCOMPILE mode, as normally the Swift compiler is built first when building natively for the host, then it's used to build the stdlib. However, when cross-compiling the toolchain, the stdlib must be cross-compiled first by the host compiler, then the portions of the Swift compiler written in Swift must be cross-compiled with that new stdlib. All these dependency changes simply change that compilation order when cross-compiling, including removing the dependency that the Swift compiler is built before the stdlib when cross-compiling the Swift compiler.

All changes in this pull are gated on the CROSSCOMPILE mode, so they will not affect any of the existing CI or build presets.

This implements 1. of #71507, generalizing my Android patch from late last year to more non-Darwin platforms. @compnerd, let me know what you think, could be useful for Windows too. @rintaro and @bnbarham, you added some of this CMake support for linux that I'm now modifying for cross-compilation, input appreciated.

@drodriguez, you may be the only person other than me using this CROSSCOMPILE mode, considering the slight modifications you added in #70134 a couple months ago. What platforms are you building for? Let me know what you think of this pull, and if you need me to pull out the SWIFT_COMPILER_SOURCES_SDK_FLAGS override you added there and apply it to all this Swift source that is now built into the compiler.

Copy link
Contributor

@drodriguez drodriguez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do CROSSCOMPILE. We try to avoid the bootstrapping mode. The default OFF is used, but because we want Swift Syntax, eventually gets changed to HOSTTOOLS. Those changes were done for a Linux to Linux compilation, but the sysroot of the destination Linux is not / (and requires many command line arguments to setup). I think the pieces that were not touch by that variable were correctly affected by CMAKE_Swift_FLAGS, and both are setup similarly. Ideally, the compiler sources eventually will move to use CMAKE_Swift_* variables, but I don't know what was the reason why they couldn't be used initially.

We have no experience with CROSSCOMPILE mode. I cannot give opinions on the changes. My only comment is just a style one.

@ahoppen ahoppen removed their request for review February 12, 2024 23:15
@finagolfin
Copy link
Member Author

@drodriguez, OK, I see. You don't use this mode, but you generally just wanted to pass in some Swift flags, so you passed them into both.

…ompiler that are written in Swift to non-Darwin Unix

Add cross-compilation flags for the newly added Swift source in `lib/ASTGen/`,
similar to how `SwiftCompilerSources/` is already cross-compiled for other
platforms. Make sure the Swift source in the compiler builds and links against
`SWIFTLIB_DIR` in this cross-compilation build directory, not the one that comes
with the native host compiler.

This requires changing the dependency chain in `CROSSCOMPILE` mode, as normally
the Swift compiler is built first when building natively for the host, then it's
used to build the stdlib. However, when cross-compiling the toolchain, the stdlib
must be cross-compiled first by the host compiler, then the portions of the
Swift compiler written in Swift must be cross-compiled with that new stdlib. All
these dependency changes simply change that compilation order when cross-compiling,
including removing the dependency that the Swift compiler is built before the
stdlib when cross-compiling the Swift compiler.

All changes in this pull are gated on the `CROSSCOMPILE` mode, so they will
not affect any of the existing CI or build presets.
@finagolfin
Copy link
Member Author

Ping @eeckstein, maybe you can review?

Copy link
Contributor

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine with me as long as it is tested well.

@finagolfin
Copy link
Member Author

Yep, I've been using most of it to cross-compile the Swift source in the compiler for Android since the 5.9 release.

@kateinoigakukun, please run the CI.

@kateinoigakukun
Copy link
Member

@swift-ci test

@finagolfin
Copy link
Member Author

Ping @bnbarham, ready for merge.

@@ -60,6 +60,16 @@ function(_add_host_swift_compile_options name)
$<$<COMPILE_LANGUAGE:Swift>:none>)

target_compile_options(${name} PRIVATE $<$<COMPILE_LANGUAGE:Swift>:-target;${SWIFT_HOST_TRIPLE}>)
if(BOOTSTRAPPING_MODE STREQUAL "CROSSCOMPILE")
add_dependencies(${name} swift-stdlib-${SWIFT_SDK_${SWIFT_HOST_VARIANT_SDK}_LIB_SUBDIR}-${SWIFT_HOST_VARIANT_ARCH})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the stdlib must be cross-compiled first by the host compiler,

The standard library must be built with the new compiler. Requiring the new stdlib to build the new compiler while also requiring the new compiler to build the stdlib will result in a situation where we can't build anything. I'm confused about how this is working for you at all given that changes landed literally yesterday that cause the stdlib to not build with the nightly toolchain compiler. You should be seeing something like this if you try:

swift/stdlib/public/core/KeyPath.swift:3696:5: error: type '(Any).Type' cannot conform to '_BitwiseCopyable'
3414 │     return (baseAddress, misalign)
3415 │   }
3416 │   mutating func pushDest<T : _BitwiseCopyable>(_ value: T) {
     │                 ╰─ note: required by instance method 'pushDest' where 'T' = '(Any).Type'
3417 │     let size = MemoryLayout<T>.size
3418 │     let (baseAddress, misalign) = adjustDestForAlignment(of: T.self)
     ┆
3694 │                      genericEnvironment: genericEnvironment,
3695 │                      arguments: patternArgs)
3696 │     pushDest(metadata)
     │     ├─ error: type '(Any).Type' cannot conform to '_BitwiseCopyable'
     │     ╰─ note: only concrete types such as structs, enums and classes can conform to protocols
3697 │     base = metadata
3698 │   }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The standard library must be built with the new compiler. Requiring the new stdlib to build the new compiler while also requiring the new compiler to build the stdlib will result in a situation where we can't build anything. I'm confused about how this is working for you at all

Let me step back a bit and explain what I'm doing here. This CROSSCOMPILE bootstrap mode is only invoked when cross-compiling the Swift compiler itself for a different platform, say you want to build the Swift compiler for linux AArch64 on a linux x86_64 host. As such, these dependencies have to be reordered as I laid out in the commit message.

There are two ways to accomplish this, but the most likely is to first build the Swift compiler from source natively for the host, then turn around and cross-compile the exact same compiler source for another platform, using the freshly-built compiler to build the Swift portions of the cross-compiled compiler. That is what these reordered dependencies when cross-compiling the compiler are needed for.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etcwilde, let me add more detail on why this is needed. Previously, the Swift compiler was written purely in C++ and the CMake dependency order was to always build the Swift compiler first, then the Swift stdlib, especially because for a native host build, that freshly-built Swift compiler is used to build the native Swift stdlib. However, cross-compiling the Swift compiler and stdlib is also supported and in that case, the freshly-built native clang and Swift compilers still cross-compile the Swift compiler first, then cross-compile the Swift stdlib. No dependency re-ordering was needed, because which was cross-compiled first didn't matter.

However, now that significant portions of the Swift compiler are written in Swift, the order does matter. You have to cross-compile the Swift stdlib first, use that cross-compiled stdlib to cross-compile SwiftCompilerSources/ and lib/ASTGen/ next, and only then can you link the cross-compiled Swift compiler. These CMake dependency changes make sure that re-ordering is done when cross-compiling the Swift stdlib and compiler.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware of the Swift in the swift compiler. I am concerned about the abuse of CMake here (and I'm not blaming you, the whole build is bad). A given CMake invocation should only be building for one thing at a time. For cross-compiling, I would expect two CMake invocations. One to build your cross-compiling toolchain with the necessary SDKs enabled. Then, in a separate CMake invocation, use that just-built toolchain to cross compile the compiler and the SDKs that you plan on shipping.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware of the Swift in the swift compiler.

I know, I'm just making clear the implications of that change for cross-compiling.

A given CMake invocation should only be building for one thing at a time. For cross-compiling, I would expect two CMake invocations. One to build your cross-compiling toolchain with the necessary SDKs enabled. Then, in a separate CMake invocation, use that just-built toolchain to cross compile the compiler and the SDKs that you plan on shipping.

So, technically, that is mostly how the build-script currently works. It invokes CMake for each Swift repo separately, like the compiler or foundation, to natively build them, collects all the build products in one native toolchain directory, then uses that freshly-built native toolchain to cross-compile each of those repos with multiple CMake invocations with different cross-compilation flags.

The only difference from what you said is that rather than mixing cross-compiling the stdlib in with natively compiling initially, it does all the native compilation first, then cross-compiles for each cross-compilation non-Darwin platform separately.

I see no reason why your preference is better.

I am concerned about the abuse of CMake here (and I'm not blaming you, the whole build is bad).

What specifically are you worried about in the CMake usage here? Swift does not use CMake's built-in Swift support when building the Swift compiler, because obviously that CMake support didn't exist when this Swift project started.

@finagolfin
Copy link
Member Author

@etcwilde, do you think you will get your cross-compilation changes into 6.0 before the release later this year? If not, maybe we can get this in now, and you can get your changes into trunk after the 6.0 branch, to be released with 6.1 next year.

@etcwilde
Copy link
Contributor

@etcwilde, do you think you will get your cross-compilation changes into 6.0 before the release later this year? If not, maybe we can get this in now, and you can get your changes into trunk after the 6.0 branch, to be released with 6.1 next year.

No, I think there is too much risk to slide my rewrite into 6.0. I'm aiming for 6.1 with it. I'll leave it up to @bnbarham what he wants to do with things in their current state.

@finagolfin
Copy link
Member Author

Alright, @bnbarham, do you think we can get this small pull in? It has no effect on the CI and will simply help with cross-compiling the compiler for Unix platforms until Evan does his big CMake rewrite.

@bnbarham
Copy link
Contributor

Alright, @bnbarham, do you think we can get this small pull in? It has no effect on the CI and will simply help with cross-compiling the compiler for Unix platforms until Evan does his big CMake rewrite.

These components aren't required today and they won't be for 6 - there shouldn't be anything stopping you from building the compiler right now. So given that, I would strongly prefer we don't propagate BOOTSTRAPPING_MODE further.

@finagolfin
Copy link
Member Author

@bnbarham, I think you misunderstand what I'm using this for. BOOTSTRAPPING_MODE is only used in this pull to determine if the compiler is being cross-compiled or not: it has nothing to do with bootstrapping but is simply a signifier of what kind of build is being undertaken. Evan will have to do something similar for his rewrite later this year, then we can just take all this out.

Until then, this is necessary to cross-compile the Swift portions of the compiler for Unix platforms, so I think we should get it in.

@finagolfin
Copy link
Member Author

@eeckstein, do you mind merging? I don't think other Swift devs understand that this has nothing to do with bootstrapping, but rather the distinction between building with host tools for the native host or a cross-compilation host.

Once Evan puts up his CMake rewrite, presumably using some other way to distinguish between native and cross compilation, it should be easy to remove this support, as it is all scoped by checking CROSSCOMPILE. I have told him that I will try out his new approach and contribute to get that working well then.

@eeckstein
Copy link
Contributor

Fine with me to merge next week (after the branch). @etcwilde @bnbarham are you fine merging this?

@finagolfin
Copy link
Member Author

Why after the branch? It has no effect on the CI, as none of the CI cross-compile the toolchain for non-Darwin platforms. I'm asking now since Evan said his rewrite won't make it into 6.0 anyway.

@eeckstein
Copy link
Contributor

Is this essential for 6.0?

@finagolfin
Copy link
Member Author

No, it isn't. It is a necessary patch for people like me, who cross-compile the toolchain for non-Darwin platforms, but for most other users of 6.0, it has both little benefit and pretty much zero risk of breaking anything.

@eeckstein
Copy link
Contributor

So, can we wait until Monday for merging?

@finagolfin
Copy link
Member Author

I'd rather not, because this will allow me to retire a patch for my 6.0 builds. The whole point of why I submitted this a month ago is to get it in before the branch, as I don't want to deal with all the red tape of getting it in after then, when I submit it again for 6.0 and people saying they don't want to risk breaking the CI on a release branch, even though it cannot break anything because no official CI uses this CROSSCOMPILE mode.

Let's just get it in now. I honestly don't understand the hesitation, given no official CI uses this mode and it is only helpful for outside toolchains like mine.

@eeckstein eeckstein merged commit 9504de5 into swiftlang:main Mar 15, 2024
@eeckstein
Copy link
Contributor

okay, fine with me

@bnbarham
Copy link
Contributor

@bnbarham, I think you misunderstand what I'm using this for.

I'm not misunderstanding anything here. My objection to the PR is more about not adding even more to our already complex build system than it is to do with bootstrapping/cross-compiling. These components aren't required today, so this isn't needed. Given it enables various functionality though, I won't block it (or revert it as the case would be now).

@finagolfin
Copy link
Member Author

My objection to the PR is more about not adding even more to our already complex build system than it is to do with bootstrapping/cross-compiling.

Everything in this pull is scoped with if(BOOTSTRAPPING_MODE STREQUAL "CROSSCOMPILE"), so I disagree that this is adding any complexity to the normal build paths that most devs use.

I'm not misunderstanding anything here... These components aren't required today, so this isn't needed.

What components aren't required today? This pull does three things:

  1. fixes the cross-compilation build for SwiftCompilerSources/
  2. adds cross-compilation flags for lib/ASTGen/ for the first time
  3. reorders the build dependencies so the Swift stdlib is cross-compiled before the Swift portions of the compiler are cross-compiled

Maybe you believe 1. isn't necessary, but I don't see how you think the others aren't.

@finagolfin finagolfin deleted the cross-compile branch March 16, 2024 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants