Skip to content

A small step toward building a toolchain on M1 Mac #3500

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

A small step toward building a toolchain on M1 Mac #3500

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26096a3
A small step toward building a toolchain on M1 Mac
johnno1962 May 14, 2021
99f7122
Remove filtering by "build_target"
johnno1962 May 19, 2021
925f892
check only for presence of cross_compile_hosts preset
johnno1962 May 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Utilities/bootstrap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ def get_swiftpm_flags(args):

build_target = get_build_target(args)
cross_compile_hosts = args.cross_compile_hosts
if build_target == 'x86_64-apple-macosx' and "macosx-arm64" in cross_compile_hosts:
if cross_compile_hosts is not None:
Copy link
Member

Choose a reason for hiding this comment

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

This is not going to work when other cross-compilation targets are added, such as in my Android pull #3403. As I said, this is a hack that should not be merged: the real fix is upstream in the Swift compiler repo where this flag is incorrectly passed in.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see what you mean. I wonder if we're conflating two concepts here: compile for a different platform vs. compile for all the architectures for a platform. For example, in general it's not going to be possible (I believe) to build for more than one platform at a time, while it's possible to build for as many architectures as that one platform will support.

When you say that the Swift compiler repo is incorrectly passing this flag: would it work to have a different flag that causes all the architectures to be built? It seems to me that that's what's needed when building toolchains on a particular platform, whether it's Darwin or something else: the distributable builds should have all the architectures supported by that platform (whereas debug builds might not, for performance reasons).

Copy link
Member

Choose a reason for hiding this comment

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

Let me begin by saying that this line that is being replaced was a hack too, as it was added just a month before the 5.3 release in 6306663 then merged back to master in 242e733. It only cross-compiles one-way, from macOS Intel to arm64 as you noticed too, which made sense as there wasn't much Apple Silicon (AS) hardware at the time. It will need to be cleaned up now that there is more AS hardware out there, but this pull is the wrong way to do it.

I suggest that one of you talk to Mishal and find out if he really wants build-toolchain to build both toolchains by default, as that is the command prescribed to even new Swift devs. I suspect he did not intend that and just meant to test this cross-compilation on the CI. Depending on what he says, you can work out how best to rework this. Note that if you really know what you're doing and use build-script instead, you will not hit this issue: this is only a problem with that preset and build-toolchain.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the additional observations @buttaface — I didn't realize that the desktop build instructions for Swift involves a universal build. That seems suboptimal.

@shahmishal Can we fix this so that:

  • build-toolchain (or whatever script we tell people to use on the desktop) only builds the host architecture by default
  • the script that builds the official toolchain passes some option to build it universal
  • we change the SwiftPM bootstrap script to let it be built universally regardless of what architecture it's currently being built on

I can take care of the SwiftPM changes but it sounds as if there are other changes needed to the Swift build scripts, and I'm not at all familiar with them. What other things need to happen to fix this?

What I'd like to end up with from SwiftPM's perspective is that running bootstrap with no options should only build for the host (as it does today), but that there should be a single standard option (e.g. --build-universal) that builds for all the architectures (currently x86_64 and arm64 on Darwin, but possibly other combinations on other platforms) regardless of the host architecture.

Copy link
Member

Choose a reason for hiding this comment

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

build-toolchain (or whatever script we tell people to use on the desktop) only builds the host architecture by default

We should add a flag to build-toolchain to all for user to build universal or host specific toolchain.

the script that builds the official toolchain passes some option to build it universal

Agree, we should continue to build universal for nightly and release toolchains.

we change the SwiftPM bootstrap script to let it be built universally regardless of what architecture it's currently being built on

This should be based on the argument provided by build-script to avoid building missing arch in swiftpm. I wonder if we can look up which arch swift was built with and build matching arch. (lipo -info)

build_flags += ["--arch", "x86_64", "--arch", "arm64"]
elif cross_compile_hosts:
error("cannot cross-compile for %s" % cross_compile_hosts)
Expand Down