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.

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

[embedded] Compile-time (literal) KeyPaths for Embedded Swift #72472

Merged
merged 5 commits into from
Mar 25, 2024

Conversation

kubamracek
Copy link
Contributor

@kubamracek kubamracek commented Mar 20, 2024

Enable KeyPath/AnyKeyPath/PartialKeyPath/WritableKeyPath in Embedded Swift, but for compile-time use only:

  • Add keypath optimizations into the mandatory optimizations pipeline
  • Allow keypath optimizations to look through begin_borrow, to make them work even in OSSA.
  • If a use of a KeyPath doesn't optimize away, diagnose in PerformanceDiagnostics
  • Make UnsafePointer.pointer(to:) transparent to allow the keypath optimization to happen in the callers of UnsafePointer.pointer(to:).

rdar://125363334

Enable KeyPath/AnyKeyPath/PartialKeyPath/WritableKeyPath in Embedded Swift, but
for compile-time use only:

- Add keypath optimizations into the mandatory optimizations pipeline
- Allow keypath optimizations to look through begin_borrow, to make them work
  even in OSSA.
- If a use of a KeyPath doesn't optimize away, diagnose in PerformanceDiagnostics
- Make UnsafePointer.pointer(to:) transparent to allow the keypath optimization
  to happen in the callers of UnsafePointer.pointer(to:).
@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek kubamracek added the embedded Embedded Swift label Mar 22, 2024
Copy link
Member

@eeckstein eeckstein left a comment

Choose a reason for hiding this comment

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

nice!
lgtm!

Copy link
Member

@phausler phausler left a comment

Choose a reason for hiding this comment

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

provided the transparent additions are ok, this looks great to me.

I presume this requires heap allocations since KeyPath is a reference type right?

@@ -370,7 +370,7 @@ public struct UnsafePointer<Pointee>: _Pointer {
/// by the key path, or `nil`.
@inlinable
@_alwaysEmitIntoClient
@_unavailableInEmbedded
@_transparent
Copy link
Member

Choose a reason for hiding this comment

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

since this is applicable for both embedded and non-embedded systems what impact does that have per ABI?
@Azoy do you have any objection here?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm not sure we'd want these things to be @_transparent in the non-embedded version. That seems a bit too aggressive for this API. If you truly need this for embedded and can't just rely on the @inlinable/@_alwaysEmitIntoClient declarations, then I'd suggest guarding this with the feature flag. In terms of ABI, this shouldn't be too big of a concern because it's already visible to clients.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the #if $Embedded guards.

@kubamracek
Copy link
Contributor Author

I presume this requires heap allocations since KeyPath is a reference type right?

Nope! I guess it is technically a reference type, but the compiler has an optimization to eliminate the creation of the keypath completely in the case where all you need from the keypath is to get the stored offset value, i.e. you only use the keypath to call MemoryLayout.offset(of:) or UnsafePointer.pointer(to:). That's exactly what this PR allows in Embedded Swift, and it (still) disallows actually creating keypaths objects.

@kubamracek
Copy link
Contributor Author

@swift-ci please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
embedded Embedded Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants