Skip to content

[stdlib] BitwiseCopyable loadUnaligned overloads. #70248

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 2 commits into from
Dec 6, 2023

Conversation

nate-chandler
Copy link
Contributor

@nate-chandler nate-chandler commented Dec 5, 2023

The new overloads will make use of the new BitwiseCopyableArchetypeTypeInfo to avoid the extra copy that is currently done.

@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@atrick
Copy link
Contributor

atrick commented Dec 5, 2023

I like this approach. The standard library team should review to be sure I'm not missing some problem.

Copy link
Contributor

@glessard glessard left a comment

Choose a reason for hiding this comment

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

Looks good. I wonder if we can use Builtin.loadRaw in the unconstrained versions as well, since every version of loadUnaligned is always-emit-into-client.

@@ -473,8 +483,6 @@ public struct UnsafeRawPointer: _Pointer {
)
return temporary.pointee
}
//FIXME: reimplement with `loadRaw` when supported in SIL (rdar://96956089)
// e.g. Builtin.loadRaw((self + offset)._rawValue)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can Builtin.loadRaw be used for the version that doesn't have the generic constraint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately no--we need to generate a direct call to memcpy rather than to a value witness; the compiler needs to know the type is BitwiseCopyable in order to know doing so is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Nice to know!

If an archetype conforms to the protocol, use the new TypeInfo.  This
allows clients to use code in the stdlib that's built with
BitwiseCopyable.
The new overloads will make use the new BitwiseCopyableArchetypeTypeInfo
to avoid the extra copy that is currently done.
@nate-chandler
Copy link
Contributor Author

@swift-ci please test

@nate-chandler
Copy link
Contributor Author

@swift-ci please test linux platform

1 similar comment
@nate-chandler
Copy link
Contributor Author

@swift-ci please test linux platform

@nate-chandler nate-chandler marked this pull request as ready for review December 6, 2023 15:16
@nate-chandler nate-chandler requested a review from a team as a code owner December 6, 2023 15:16
@nate-chandler nate-chandler merged commit 684739e into swiftlang:main Dec 6, 2023
@nate-chandler nate-chandler deleted the rdar96956089 branch December 6, 2023 20:13
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