Skip to content
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

Jump to bottom

Enable _Concurrency for Embedded Swift with WASI #79292

Merged
merged 20 commits into from
Mar 26, 2025
Merged

Enable _Concurrency for Embedded Swift with WASI #79292

merged 20 commits into from
Mar 26, 2025
+101 −69

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented Feb 11, 2025 •
edited
Loading

WASI with Embedded Swift provides WASI-libc and libc++ headers necessary to build the _Concurrency module for Wasm. We now add wasm32-unknown-wasip1-wasm triple to EMBEDDED_STDLIB_TARGET_TRIPLES when SWIFT_WASI_SYSROOT_PATH is set, which builds the necessary stdlib slice.

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov MaxDesiatov added concurrency Feature: umbrella label for concurrency language features embedded Embedded Swift WebAssembly Platform: WebAssembly labels Feb 11, 2025
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov MaxDesiatov changed the title Enable _Concurrency for Wasm in Embedded Swift Enable _Concurrency for Embedded Swift with WASI Mar 13, 2025
@MaxDesiatov MaxDesiatov added the WASI WebAssembly System Interface support label Mar 13, 2025
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov MaxDesiatov mentioned this pull request Mar 14, 2025
Closed
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov MaxDesiatov marked this pull request as ready for review March 17, 2025 11:44
@MaxDesiatov MaxDesiatov requested review from ktoso and a team as code owners March 17, 2025 11:44
@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov
Copy link
Contributor Author

@swift-ci smoke test macos

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain macos

@MaxDesiatov MaxDesiatov requested a review from etcwilde March 17, 2025 16:44
@MaxDesiatov
Merge pull request #80274 from kateinoigakukun/maxd/embedded-wasm-con…
5973bc8 
…currency

Embedded concurrency for WASI fixes
kateinoigakukun
kateinoigakukun approved these changes Mar 25, 2025
View reviewed changes
Copy link
Member

@kateinoigakukun kateinoigakukun left a comment

Choose a reason for hiding this comment

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

Thanks, seems good to me

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain

@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

@etcwilde would you mind having a look?

@MaxDesiatov
Copy link
Contributor Author

@swift-ci build toolchain macos

compnerd
compnerd approved these changes Mar 26, 2025
View reviewed changes
stdlib/public/Concurrency/CMakeLists.txt
Comment on lines +265 to +266
set(extra_c_compile_flags -D__MACH__ -D__APPLE__ -ffreestanding -stdlib=libc++)
set(extra_swift_compile_flags -Xcc -D__MACH__ -Xcc -D__APPLE__ -Xcc -ffreestanding)
Copy link
Member

Choose a reason for hiding this comment

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

Why are -D__MACH__ -D__APPLE__ needed? those should be present with the apple vendor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's unclear to me. Keeping as is, changing it doesn't seem relevant to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

These are needed for -apple-none-macho but shouldn't be needed otherwise. the __MACH__ one should only be needed for os ~= darwin but sadly __MACH__ is abused to also mean __MACHO__

etcwilde
etcwilde approved these changes Mar 26, 2025
View reviewed changes
Copy link
Contributor

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

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

I think this will work. build-script really needs a better/more general way to pass sysroots through to all of the runtime builds, but this should work.

@MaxDesiatov MaxDesiatov merged commit eb1a296 into main Mar 26, 2025
8 checks passed
@MaxDesiatov MaxDesiatov deleted the maxd/embedded-wasm-concurrency branch March 26, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd

@ktoso ktoso

@etcwilde etcwilde

@kubamracek kubamracek

@kateinoigakukun kateinoigakukun

@rauhul rauhul

Assignees
No one assigned
Labels
concurrency Feature: umbrella label for concurrency language features embedded Embedded Swift WASI WebAssembly System Interface support WebAssembly Platform: WebAssembly
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@MaxDesiatov @compnerd @ktoso @etcwilde @kubamracek @rauhul @kateinoigakukun