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

Miscellaneous changes #83

Merged
merged 3 commits into from Aug 11, 2022
Merged

Miscellaneous changes #83

merged 3 commits into from Aug 11, 2022

Conversation

glandium
Copy link
Contributor

No description provided.

It hasn't been useful for Firefox for a while.
As of rustc 1.59.0, all the simd intrinsics in use in the crate are
stable, but two of the types aren't, for some reason. However, if we
let type inference do its job via IntoBits, we can avoid having to
import those unstable types and leave all unstable uses for simd
purpose to packed_simd.

For consistency, we use IntoBits everywhere, rather than use FromBits as
well, which would make it necessary to figure out on which specific
configurations FromBits is used.
For some reason, it's marked as unsafe to mimic the intrinsic, but the
intrinsic appears to always have been safe: rust-lang/rust@b778f7f
@glandium
Copy link
Contributor Author

I have another, more controversial change in stock, which removes the use of core intrinsics altogether, removing likely/unlikely because... guess what... they don't work. I'll submit it separately once this is merged.

@hsivonen hsivonen merged commit d4d7d2a into hsivonen:master Aug 11, 2022
@hsivonen
Copy link
Owner

Thanks!

I think it doesn't make sense to remove likely/unlikely. When I wrote the code, there were places, IIRC, UTF-8 decoding in particular, where these annotations made a measurable difference on Haswell. I might have also annotated something based on logic rather measuring, but going to the trouble of having these at all based on a feature flag was certainly motivated by measurement. Anyway, I'd like to leave these in place so that they can be eventually migrated to something stable that works instead of being lost in the mean time.

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.

None yet

2 participants