FoundationMacros: use cross-compilation to build for host #714

compnerd · 2024-07-03T15:39:53Z

Use ExternalProject to switch FoundationMacros to cross-compilation. This allows us to build the macros for the right OS/architecture when cross-compiling Foundation for other environments.

etcwilde

Few nits, but otherwise LGTM.

Sources/FoundationMacros/CMakeLists.txt

Sources/CMakeLists.txt

compnerd · 2024-07-03T16:56:58Z

@swift-ci please test

shahmishal · 2024-07-10T22:19:23Z

@swift-ci please test

shahmishal · 2024-07-10T22:21:52Z

@swift-ci please test

compnerd · 2024-07-17T18:09:34Z

CC: @bnbarham there was a conversation on swiftlang/swift#75206 which seems to have converged around moving ahead with this change.

compnerd · 2024-07-18T20:10:56Z

@jmschonfeld @bnbarham - any idea if this is something we should be moving forward with? This is going to break the windows builds at least, but I want to ensure that we only are investing in this if we are going to move forward with this.

jmschonfeld · 2024-07-18T20:54:46Z

@jmschonfeld @bnbarham - any idea if this is something we should be moving forward with? This is going to break the windows builds at least, but I want to ensure that we only are investing in this if we are going to move forward with this.

I think I'm still a little fuzzy on the details for this, so just to lay it out to confirm:

Currently we have:

(builder platform = host building the toolchain, toolchain platform = host where the toolchain will be used, sdk platform = platform where things the toolchain builds will be run)

Linux:

Macro builds as a library and links the early swift syntax build
Macro builds for builder platform (platform that is building the toolchain)
Macro gets used for FoundationEssentials build
Macro gets installed into the toolchain since in the supported case, builder platform == toolchain platform

Windows: Macro does not build

And with this PR, can you clarify what changes? In specific

Does this still install the macro on linux where builder platform == toolchain platform
Do we build the macro on Windows for the builder platform to use to build FoundationEssentials (this is yes right?)
Do we build the macro on Windows for the toolchain platform? And it is not built for the SDK platform right?
Is it still built as a library?

I think I'm still unclear on those points and I want to make sure that we do not regress what Linux already has working in the process of getting it right on Windows (but I agree we need to make sure we get it right on Windows too)

etcwilde · 2024-07-19T04:56:10Z

So the goal is to split where the SDK library that actually goes in the runtime environment and the macros that are only useful if you're compiling things against that library.
Right now, the build is set up such that the macro will always build for whatever you're building swift-foundation to run on, but not necessarily where you're running the compiler to build things against that SDK, which isn't particularly helpful if you don't want/need the macro to run there. It also means that we'll need to build and include Swift-Syntax in the built SDK, even though it will just be dead weight in situations where you're never running a compiler.

What this PR does is, when building Swift-Foundation, build the macro for the platform that is doing the build, rather than what swift-foundation is being cross-compiled for. This is just like what we're doing for Swift-Testing (https://github.com/apple/swift-testing/blob/b19c9af0750e219fc972217af3f061b1b0b60897/Sources/CMakeLists.txt#L22-L30).

It's still not perfect, but it's better because it means that we can use these macros in the build of swift-foundation.
In a future iteration, we might be able to teach the build to produce macros for each of the toolchain platforms that we support so that we can have an SDK that works universally with each of the supported toolchains (good for cross-compiling story) instead of only the toolchain that is on the system that you're deploying for (not great if you're compiling for a low-powered phone or something).

compnerd · 2024-07-19T05:07:24Z

Just to clear up some of the confusion on what is happening today:

Windows:

Macros build as executables, link the early swift syntax build
Macros build for the platform that foundation are being built for (e.g. Android)
Macros get used for the tests

Note that this is identical to Linux where the macros are built for the platform that foundation is being built for (e.g. macOS).

bnbarham · 2024-07-19T17:11:00Z

Macros build as executables, link the early swift syntax build

In the toolchain build or standalone? If the former, how are they being picked up at runtime? IIRC we only add toolchain search paths for the libraries, not executables.

compnerd · 2024-07-19T21:50:10Z

Macros build as executables, link the early swift syntax build

In the toolchain build or standalone? If the former, how are they being picked up at runtime? IIRC we only add toolchain search paths for the libraries, not executables.

Ugh, wait, I mixed up two scenarios. The reason that it is building with the toolchain early swift-syntax is because I'm building locally with a reference to the toolchain build. That change is dependent on the other change that I have pending.

The CI builds currently use SPM based builds in a docker container, so they are building their own copy of swift-syntax.

For the toolchain distribution, we should be using the toolchain build of swift-syntax though to avoid bloat (the toolchain is already large enough that it prevents pre-installation on GHA agents).

bnbarham · 2024-07-19T22:39:02Z

For the toolchain distribution, we should be using the toolchain build of swift-syntax though to avoid bloat

Sure, that should be the case here - we should be passing through SwiftSyntax_DIR and thus going down the FindPackage path.

compnerd · 2024-07-20T02:28:42Z

For the toolchain distribution, we should be using the toolchain build of swift-syntax though to avoid bloat

Sure, that should be the case here - we should be passing through SwiftSyntax_DIR and thus going down the FindPackage path.

I don't think that was wired up in the swift-corelibs-foundation build which is now re-cored to swift-foundation. We don't build swift-foundation on its own yet, so this particular path is more future direction and I don't think that we are setup to test it in CI.

jmschonfeld · 2024-07-22T18:29:12Z

For the toolchain distribution, we should be using the toolchain build of swift-syntax though to avoid bloat

Sure, that should be the case here - we should be passing through SwiftSyntax_DIR and thus going down the FindPackage path.

I don't think that was wired up in the swift-corelibs-foundation build which is now re-cored to swift-foundation. We don't build swift-foundation on its own yet, so this particular path is more future direction and I don't think that we are setup to test it in CI.

AFAIK it should be - when invoking the swift-corelibs-foundation build from both build.ps1 and the build-script, we provide SwiftSyntax_DIR (and swift-corelibs-foundation pulls swift-foundation in via FetchContent and the goal is to go down the find_package path, if that's not happening it's likely a bug we need to fix in the build script or CMake file

jmschonfeld · 2024-07-22T18:33:05Z

@etcwilde @compnerd ah thanks for clearing up a lot of those details, I think I understand the problem space better now. I think overall this approach sounds good, the only two points that I think we should address before merging this are:

Can we make sure to install the macro into the just-built toolchain for cases where the current host platform matches the platform that the toolchain will be used on (i.e. whenever we're not cross compiling like when we build the linux toolchain)? Not sure if there's a way we can dynamically detect this
Based on @bnbarham's comment in the thread above, it sounds like we should ensure we still build the macro as a library like we currently do on Linux rather than as an executable. Is that feasible or do we have an explicit need to use an executable instead?

compnerd · 2024-07-23T15:22:05Z

For the installation, I don't think that we have a good way to detect when it is being built for a toolchain host. There is CMAKE_CROSSCOMPILING that tells us whether it is cross-compiling, but I could be cross-compiling for a toolchain host (which is, in fact, what we do for ARM64). So, really, the best way to handle this is to explicitly require that you build the install target if you want to install the macros.

The plugin (DLL) approach is not really viable. The entire compiler codebase needs to be annotated with __declspec(dllimport) and __declspec(dllexport) to get that to work (including the LLVM one). Otherwise we run out of space for exporting everything (not to mention, there is a load time and runtime memory penalty for doing that). Executables are the way forward for the foreseeable future (we could try to invasively fork LLVM for Swift and annotate LLVM and split it up of course).

bnbarham · 2024-07-23T21:03:53Z

The plugin (DLL) approach is not really viable. The entire compiler codebase needs to be annotated with __declspec(dllimport) and __declspec(dllexport) to get that to work (including the LLVM one). Otherwise we run out of space for exporting everything (not to mention, there is a load time and runtime memory penalty for doing that). Executables are the way forward for the foreseeable future (we could try to invasively fork LLVM for Swift and annotate LLVM and split it up of course).

We currently use libraries for the stdlib plugins. The driver isn't setup to load executables from the toolchain at all, so if we did end up going down that route we'd need updates there. But I'm not sure I understand why we have to. Why does LLVM matter at all here?

compnerd · 2024-07-24T04:10:56Z

We currently use libraries for the stdlib plugins. The driver isn't setup to load executables from the toolchain at all, so if we did end up going down that route we'd need updates there. But I'm not sure I understand why we have to. Why does LLVM matter at all here?

The plugin host needs to expose interfaces to the plugin (i.e. -rdynamic). If the plugins have no assumed library interface, then this should be possible.

bnbarham · 2024-07-24T18:34:45Z

The plugin host needs to expose interfaces to the plugin (i.e. -rdynamic). If the plugins have no assumed library interface, then this should be possible.

There's an interface, but it uses swift-syntax APIs and they are in SwiftSyntax.dll.

Just to re-iterate, the stdlib macro plugins are both libraries today and as far as I'm aware they don't have any issues. This seems like an identical case to me.

hyp · 2024-07-29T19:39:02Z

Added additional changes to make this also work for the android swift-foundation building from build.ps1 windows host:
compnerd#1
swiftlang/swift#75549

jmschonfeld · 2024-08-08T23:53:29Z

Sources/CMakeLists.txt

+  set(_SwiftFoundation_ExpressionMacro "${BINARY_DIR}/FoundationMacros.dll#ExpressionMacro")
+else()
+  set(_SwiftFoundation_PredicateMacro "${BINARY_DIR}/FoundationMacros#PredicateMacro")
+  set(_SwiftFoundation_ExpressionMacro "${BINARY_DIR}/FoundationMacros#ExpressionMacro")


I think I'm still confused on what the intention of these settings are? Are they supposed to be paths that we provide as -plugin-path to the FoundationEssentials build? If so, I think I'd expect them to be in the library directory on non-windows and also look like libFoundationMacros.so rather than FoundationMacros#ExpressionMacro.

Yes, this was the expected parameter for the macro. Although, after the discussion with @rintaro yesterday, I don't think it matters. Once we have the custom option for the macro, this will no longer be needed.

jmschonfeld · 2024-08-08T23:54:56Z

Sources/CMakeLists.txt

+if(CMAKE_HOST_WIN32)
+  set(_FoundationMacrosSwiftFlags "-use-ld=lld")
+endif()
+ExternalProject_Add(FoundationMacros


Just to clarify with this approach, I know that the build script will need to invoke the macro build separately. Is the goal that it needs to invoke the macro build once for the current host (and pass that path to the swift-foundation build to use when building itself) and once for each target host (and that it will actually install instead of just building)? In FoundationEssentials/CMakeLists.txt we have a FoundationEssentials --> FoundationMacros dependency added on non-Windows, I think we'll need to adjust that accordingly if there's no longer a target built in this same project

Yes, the idea is that we would need at least two builds:

build: for use by the compiler building swift-foundation (not installed)

host: for distribution only

The path to the build build of the macros would be passed in as a parameter for the build as we discussed.

Sources/CMakeLists.txt

CMakeLists.txt

Sources/FoundationMacros/CMakeLists.txt

Use `ExternalProject` to switch FoundationMacros to cross-compilation. This allows us to build the macros for the right OS/architecture when cross-compiling Foundation for other environments. Co-authored-by: Alex Lorenz <[email protected]>

jmschonfeld · 2024-08-10T04:55:37Z

@swift-ci please test

jmschonfeld

This LGTM now - approved to merge, but let's try to merge it with both of our swiftlang/swift changes and your swift-installer-script change just so that everything switches to the new build style at once

) Use `ExternalProject` to switch FoundationMacros to cross-compilation. This allows us to build the macros for the right OS/architecture when cross-compiling Foundation for other environments. Co-authored-by: Alex Lorenz <[email protected]>

Use `ExternalProject` to switch FoundationMacros to cross-compilation. This allows us to build the macros for the right OS/architecture when cross-compiling Foundation for other environments. Co-authored-by: Alex Lorenz <[email protected]>

) Use `ExternalProject` to switch FoundationMacros to cross-compilation. This allows us to build the macros for the right OS/architecture when cross-compiling Foundation for other environments. Co-authored-by: Alex Lorenz <[email protected]>

compnerd requested review from parkera, jmschonfeld and iCharlesHu July 3, 2024 15:39

etcwilde reviewed Jul 3, 2024

View reviewed changes

Sources/FoundationMacros/CMakeLists.txt Show resolved Hide resolved

Sources/CMakeLists.txt Outdated Show resolved Hide resolved

Sources/CMakeLists.txt Outdated Show resolved Hide resolved

jmschonfeld reviewed Jul 3, 2024

View reviewed changes

Sources/CMakeLists.txt Outdated Show resolved Hide resolved

Sources/CMakeLists.txt Outdated Show resolved Hide resolved

Sources/CMakeLists.txt Outdated Show resolved Hide resolved

compnerd force-pushed the crossed-macros branch from 56d99c0 to a1c39d7 Compare July 3, 2024 16:56

compnerd force-pushed the crossed-macros branch from a1c39d7 to a332b7b Compare July 3, 2024 17:59

compnerd mentioned this pull request Jul 17, 2024

utils: build a few extra copies of swift-syntax swiftlang/swift#75206

Closed

compnerd mentioned this pull request Jul 25, 2024

[android] fix the android build #757

Open

hyp mentioned this pull request Aug 6, 2024

Add android support, that uses new swift-foundation for Android swiftlang/swift-corelibs-foundation#5024

Merged

kateinoigakukun mentioned this pull request Aug 7, 2024

Repair swift-corelibs-foundation build swiftwasm/swift#5586

Closed

33 tasks

compnerd force-pushed the crossed-macros branch 3 times, most recently from 8a9c602 to 6c3ffcb Compare August 8, 2024 21:29

compnerd mentioned this pull request Aug 8, 2024

utils: build Foundation macros for distribution swiftlang/swift#75785

Merged

compnerd force-pushed the crossed-macros branch from 6c3ffcb to 890309d Compare August 8, 2024 22:53

compnerd mentioned this pull request Aug 8, 2024

Get Swift-Foundation building for Musl/Static SDK #838

Merged

jmschonfeld reviewed Aug 8, 2024

View reviewed changes

kateinoigakukun reviewed Aug 9, 2024

View reviewed changes

Sources/CMakeLists.txt Outdated Show resolved Hide resolved

compnerd force-pushed the crossed-macros branch from 890309d to ed0dd0f Compare August 9, 2024 15:06

jmschonfeld reviewed Aug 9, 2024

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

jmschonfeld reviewed Aug 9, 2024

View reviewed changes

Sources/FoundationMacros/CMakeLists.txt Outdated Show resolved Hide resolved

compnerd force-pushed the crossed-macros branch from ed0dd0f to 52a0ee3 Compare August 10, 2024 02:45

jmschonfeld mentioned this pull request Aug 10, 2024

build-script: Invoke Foundation macros build separately swiftlang/swift#75816

Merged

jmschonfeld approved these changes Aug 11, 2024

View reviewed changes

compnerd merged commit aee18e4 into swiftlang:main Aug 12, 2024
3 checks passed

compnerd deleted the crossed-macros branch August 12, 2024 16:41

This was referenced Aug 14, 2024

FoundationMacros: use cross-compilation to build for host (#714) #849

Merged

utils: build Foundation macros for distribution swiftlang/swift#75869

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FoundationMacros: use cross-compilation to build for host #714

FoundationMacros: use cross-compilation to build for host #714

compnerd commented Jul 3, 2024

etcwilde left a comment

compnerd commented Jul 3, 2024

shahmishal commented Jul 10, 2024

shahmishal commented Jul 10, 2024

compnerd commented Jul 17, 2024

compnerd commented Jul 18, 2024

jmschonfeld commented Jul 18, 2024

etcwilde commented Jul 19, 2024

compnerd commented Jul 19, 2024

bnbarham commented Jul 19, 2024

compnerd commented Jul 19, 2024

bnbarham commented Jul 19, 2024

compnerd commented Jul 20, 2024

jmschonfeld commented Jul 22, 2024

jmschonfeld commented Jul 22, 2024

compnerd commented Jul 23, 2024

bnbarham commented Jul 23, 2024

compnerd commented Jul 24, 2024

bnbarham commented Jul 24, 2024

hyp commented Jul 29, 2024

jmschonfeld Aug 8, 2024

compnerd Aug 9, 2024

jmschonfeld Aug 8, 2024

compnerd Aug 9, 2024

jmschonfeld commented Aug 10, 2024

jmschonfeld left a comment

FoundationMacros: use cross-compilation to build for host #714

FoundationMacros: use cross-compilation to build for host #714

Conversation

compnerd commented Jul 3, 2024

etcwilde left a comment

Choose a reason for hiding this comment

compnerd commented Jul 3, 2024

shahmishal commented Jul 10, 2024

shahmishal commented Jul 10, 2024

compnerd commented Jul 17, 2024

compnerd commented Jul 18, 2024

jmschonfeld commented Jul 18, 2024

etcwilde commented Jul 19, 2024

compnerd commented Jul 19, 2024

bnbarham commented Jul 19, 2024

compnerd commented Jul 19, 2024

bnbarham commented Jul 19, 2024

compnerd commented Jul 20, 2024

jmschonfeld commented Jul 22, 2024

jmschonfeld commented Jul 22, 2024

compnerd commented Jul 23, 2024

bnbarham commented Jul 23, 2024

compnerd commented Jul 24, 2024

bnbarham commented Jul 24, 2024

hyp commented Jul 29, 2024

jmschonfeld Aug 8, 2024

Choose a reason for hiding this comment

compnerd Aug 9, 2024

Choose a reason for hiding this comment

jmschonfeld Aug 8, 2024

Choose a reason for hiding this comment

compnerd Aug 9, 2024

Choose a reason for hiding this comment

jmschonfeld commented Aug 10, 2024

jmschonfeld left a comment

Choose a reason for hiding this comment