Skip to content

[SUA][IRGen] Change IRGen to emit calls to swift_coroFrameAlloc #79384

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 5 commits into from
Feb 21, 2025

Conversation

t-rasmud
Copy link
Contributor

When TMO is enabled, change IRGen to pass the newly introduced runtime function swift_coroFrameAlloc (and pass an additional argument — the hash value) instead of malloc when it inserts calls to coro_id_retcon_once. The hashValue is computed based on the current function name (computed in getDiscriminatorForString)

rdar://141235957

When TMO is enabled, change IRGen to pass the newly introduced runtime function `swift_coroFrameAlloc` (and pass an additional argument — the hash value) instead of `malloc` when it inserts calls to `coro_id_retcon_once`.
The hashValue is computed based on the  current function name (computed in `getDiscriminatorForString`)

rdar://141235957
@t-rasmud t-rasmud marked this pull request as ready for review February 14, 2025 05:14
@t-rasmud t-rasmud requested a review from rjmccall as a code owner February 14, 2025 05:14
@aschwaighofer
Copy link
Contributor

aschwaighofer commented Feb 14, 2025

You need to cherry-pick swiftlang/llvm-project#10015 to the llvm-project branch that main builds with (stable/20240723) before merging this PR.
You can then test this PR with the llvm PR using cross repo testing: https://github.com/swiftlang/swift/blob/main/docs/ContinuousIntegration.md#cross-repository-testing

For example, something like the following should work:

Please test with following pull request:
https://github.com/swiftlang//llvm-project/pull/22222

@swift-ci test

Please put this behind an IRGen flag -enable/disable-use-type-malloc-for-coro-frame and have it default to not use this feature yet. You can use this commit b49e30c as a model.

For staging/back-deployment purposes, we will need to call through a stub that checks for the presence of swift_coroFrameAlloc in the runtime library (use a weak symbol ref that it checks for null).

The stub functionality can be added in a later commit.

@t-rasmud t-rasmud requested review from aschwaighofer, fbigarella and yln and removed request for rjmccall February 14, 2025 16:43
@t-rasmud
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10041

@swift-ci test

@t-rasmud
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10041

@swift-ci test

@t-rasmud
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10041

@swift-ci test

Copy link
Contributor

@aschwaighofer aschwaighofer left a comment

Choose a reason for hiding this comment

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

This looks good to me.

Things to fix in followups:

1.) For staging/back-deployment purposes, we will need to call through a stub that checks for the presence of swift_coroFrameAlloc in the runtime library (use a weak symbol ref that it checks for null).
2.) Compute a 32bit hash (the function used computes a 16bit truncated value, we should add a new one for this purpose)
3.) Pass a properly constructed malloc_type_id value to the runtime function

@t-rasmud
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10041

@swift-ci test

@t-rasmud
Copy link
Contributor Author

Please test with following pull request:
swiftlang/llvm-project#10041

@swift-ci test

@t-rasmud t-rasmud merged commit 0b8efc9 into swiftlang:main Feb 21, 2025
5 checks passed
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.

3 participants