Change the compiler ABI of keypaths. #20493
Merged
Conversation
This file contains hidden or 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
Previously, the stdlib provided: - getters for AnyKeyPath and PartialKeyPath, which have remained; - a getter for KeyPath, which still exists alongside a new read coroutine; and - a pair of owned mutable addressors that provided modify-like behavior for WritableKeyPath and ReferenceWritableKeyPath, which have been replaced with modify coroutines and augmented with dedicated setters. SILGen then uses the most efficient accessor available for the access it's been asked to do: for example, if it's been asked to produce a borrowed r-value, it uses the read accessor. Providing a broad spectrum of accessor functions here seems acceptable because the code-size hit is fixed-size: we don't need to generate extra code per storage declaration to support more alternatives for key paths. Note that this is just the compiler ABI; the implementation is still basically what it was. That means the implementation of the setters and the read accessor is pretty far from optimal. But we can improve the implementation later; we can't improve the ABI. The coroutine accessors have to be implemented in C++ and used via hand-rolled declarations in SILGen because it's not currently possible to declare independent coroutine accessors in Swift.
|
@swift-ci Please test. |
|
@swift-ci Please benchmark |
|
@swift-ci Please test source compatibility. |
Build comment file:No performance and code size changes |
|
Build failed |
|
Random LLDB failures? |
|
Source compatibility failures are just due to the introduction of |
|
@swift-ci Please smoke test |
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.
Previously, the stdlib provided:
getters for AnyKeyPath and PartialKeyPath, which have remained;
a getter for KeyPath, which still exists alongside a new read coroutine; and
a pair of owned mutable addressors that provided modify-like behavior for WritableKeyPath and ReferenceWritableKeyPath, which have been replaced with modify coroutines and augmented with dedicated setters.
SILGen then uses the most efficient accessor available for the access it's been asked to do: for example, if it's been asked to produce a borrowed r-value, it uses the read accessor.
Providing a broad spectrum of accessor functions here seems acceptable because the code-size hit is fixed-size: we don't need to generate extra code per storage declaration to support more alternatives for key paths.
Note that this is just the compiler ABI; the implementation is still basically what it was. That means the implementation of the setters and the read accessor is pretty far from optimal. But we can improve the implementation later; we can't improve the ABI.
The coroutine accessors have to be implemented in C++ and used via hand-rolled declarations in SILGen because it's not currently possible to declare independent coroutine accessors in Swift.