Skip to content

[stdlib] Apply @_transparent consistently for (U)Int128 #74523

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 7 commits into from
Jun 21, 2024

Conversation

Azoy
Copy link
Contributor

@Azoy Azoy commented Jun 18, 2024

Also adds a concrete implementation of _lowWord for both Int128 and UInt128. I've sprinkled available attributes everywhere because I personally dislike the implicit availability from the extension declaration because a new API introduced in an existing extension may forget to add availability which would be incorrect, so littering this everywhere hopefully makes it more obvious.

Resolves: #74481

@Azoy Azoy requested a review from a team as a code owner June 18, 2024 17:49
@Azoy
Copy link
Contributor Author

Azoy commented Jun 18, 2024

@swift-ci please test

@Azoy
Copy link
Contributor Author

Azoy commented Jun 18, 2024

@swift-ci please benchmark

@benrimmington

This comment was marked as outdated.

#elseif _pointerBitWidth(_32)
UInt(Builtin.trunc_Int64_Int32(_low._value))
#else
UInt(truncatingIfNeeded: _low)
Copy link
Contributor

Choose a reason for hiding this comment

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

Formally I think the #else branch here should probably hit the default implementation instead, but that requires refactoring this a bit, so I'm ok with this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this implementation is technically incorrect for words larger than 64 bits, so I just wrote this as truncating self.

@inline(__always)

@available(SwiftStdlib 6.0, *)
@_transparent
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not certain that we want these to be @_transparent. These are potentially pretty heavyweight operations to drop into the caller (though we might reap benefits in the case of constant divisors).

@Azoy
Copy link
Contributor Author

Azoy commented Jun 18, 2024

@stephentyrone how do we feel about making _low and _high public? Considering that the underlying builtin _value property is public (for good reason) it seems like we should just make these other accessors public as well. Thoughts?

@stephentyrone
Copy link
Contributor

It's a little bit weird since we don't have them for any other integer types, but other than that I'm OK with it.

@Azoy
Copy link
Contributor Author

Azoy commented Jun 18, 2024

@swift-ci please test

@Azoy
Copy link
Contributor Author

Azoy commented Jun 18, 2024

@swift-ci please benchmark

@Azoy Azoy force-pushed the int128-attributes branch from 52ac1e6 to 87a8edb Compare June 18, 2024 23:08
@Azoy
Copy link
Contributor Author

Azoy commented Jun 18, 2024

@swift-ci please test

@Azoy
Copy link
Contributor Author

Azoy commented Jun 20, 2024

@swift-ci please test

@benrimmington
Copy link
Contributor

Do you also want to @swift-ci test WebAssembly for the _pointerBitWidth(_32) parts?

@Azoy
Copy link
Contributor Author

Azoy commented Jun 20, 2024

@swift-ci test WebAssembly

@Azoy
Copy link
Contributor Author

Azoy commented Jun 20, 2024

Sure!

@Azoy Azoy merged commit dd8a21e into swiftlang:main Jun 21, 2024
6 checks passed
@Azoy Azoy deleted the int128-attributes branch June 21, 2024 01:28
Azoy added a commit to Azoy/swift that referenced this pull request Jun 25, 2024
[stdlib] Apply @_transparent consistently for (U)Int128
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.

[SE-0425] Int128 and UInt128 APIs have different attributes
3 participants