SILOptimizer: Allow inlining of transparent functions in @backDeployed thunks
#76135
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tshortli
force-pushed
the
backdeployed-ios-apps-on-macos
branch
from
4197cef to
f7816cf
Compare
In the standard library shipped in Apple's SDKs and OSes, the implementation of `_stdlib_isOSVersionAtLeast()` has diverged in order to solve some tricky issues related to supporting iOS applications running on macOS. It's now time to bring that change upstream in order to unblock further changes that depend on it. Originally introduced to resolve rdar://83378814.
tshortli
force-pushed
the
backdeployed-ios-apps-on-macos
branch
from
f7816cf to
42a68fd
Compare
tshortli
changed the title
_stdlib_isOSVersionAtLeast_AEIC() for availability checks@backDeployed thunks
|
@swift-ci please test |
tshortli
commented
Aug 29, 2024
tshortli
requested review from
jckarter,
rjmccall,
a team,
xymus and
eeckstein
as code owners
|
@swift-ci please test Windows |
|
@swift-ci benchmark |
|
Run the benchmarks to be on the safe side (I don't expect any changes) |
…d thunks. In order for availability checks in iOS apps to be evaluated correctly when running on macOS, the application binary must call a copy of `_stdlib_isOSVersionAtLeast_AEIC()` that was emitted into the app, instead of calling the `_stdlib_isOSVersionAtLeast()` function provided by the standard library. This is because the call to the underlying compiler-rt function `__isPlatformVersionAtLeast()` must be given the correct platform identifier argument; if the call is not emitted into the client, then the macOS platform identifier is used and the iOS version number will be mistakenly interpreted as a macOS version number at runtime. The `_stdlib_isOSVersionAtLeast()` function in the standard library is marked `@_transparent` on iOS so that its call to `_stdlib_isOSVersionAtLeast_AEIC()` is always inlined into the client. This works for the code generated by normal `if #available` checks, but for the `@backDeployed` function thunks, the calls to `_stdlib_isOSVersionAtLeast()` were not being inlined and that was causing calls to `@backDeployed` functions to crash in iOS apps running on macOS since their availability checks were being misevaluated. The SIL optimizer has a heuristic which inhibits mandatory inlining in functions that are classified as thunks, in order to save code size. This heuristic needs to be relaxed in `@backDeployed` thunks, so that mandatory inlining of `_stdlib_isOSVersionAtLeast()` can behave as expected. The change should be safe since the only `@_transparent` function a `@backDeployed` thunk is ever expected to call is `_stdlib_isOSVersionAtLeast()`. Resolves rdar://134793410.
tshortli
force-pushed
the
backdeployed-ios-apps-on-macos
branch
from
42a68fd to
789b795
Compare
|
@swift-ci please test |
We lost the link to these benchmark results since I had to rerun the tests after fixing some broken ones. Here are the original results: https://ci.swift.org/job/swift-PR-macos-perf/1001/ |
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Aug 30, 2024
As promised in the FIXME, update the SIL printer and parser to handle the new thunk kind for `@backDeployed` thunks. Follow up to swiftlang#76135.
This was referenced Aug 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order for availability checks in iOS apps to be evaluated correctly when running on macOS, the application binary must call a copy of
_stdlib_isOSVersionAtLeast_AEIC()that was emitted into the app, instead of calling the_stdlib_isOSVersionAtLeast()function provided by the standard library. This is because the call to the underlying compiler-rt function__isPlatformVersionAtLeast()must be given the correct platform identifier argument; if the call is not emitted into the client, then the macOS platform identifier is used and the iOS version number will be mistakenly interpreted as a macOS version number at runtime.The
_stdlib_isOSVersionAtLeast()function in the standard library is marked@_transparenton iOS so that its call to_stdlib_isOSVersionAtLeast_AEIC()is always inlined into the client. This works for the code generated by normalif #availablechecks, but for the@backDeployedfunction thunks, the calls to_stdlib_isOSVersionAtLeast()were not being inlined and that was causing calls to@backDeployedfunctions to crash in iOS apps running on macOS since their availability checks were being misevaluated.The SIL optimizer has a heuristic which inhibits mandatory inlining in functions that are classified as thunks, in order to save code size. This heuristic needs to be relaxed in
@backDeployedthunks, so that mandatory inlining of_stdlib_isOSVersionAtLeast()can behave as expected. The change should be safe since the only@_transparentfunction a@backDeployedthunk is ever expected to call is_stdlib_isOSVersionAtLeast().Resolves rdar://134793410.