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

add SIMD swap_bytes/to_le #1

Closed

Conversation

TheIronBorn
Copy link

This also includes's dhardy's recommended mem::size_of::<$ty>() rust-random#523 (comment).

Note: builds currently fail with --features simd_support. Should we also enable all nightly features when simd_support is enabled?


// bulk impl for a shuffle intrinsic/vector width
($vec8:ident, $shuf:ident, $indices:expr, $($ty:ident,)+) => ($(
impl_swap_bytes! { $ty, $vec8, $shuf, $indices }
Copy link

Choose a reason for hiding this comment

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

Changing the argument order here is confusing. Keep the type at the end?

Copy link
Author

Choose a reason for hiding this comment

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

It makes the macro invitation prettier but yeah I see how it could trip people up

#[cfg(feature = "simd_support")]
extern "platform-intrinsic" {
fn simd_shuffle2<T, U>(a: T, b: T, indices: [u32; 2]) -> U;
fn simd_shuffle4<T, U>(a: T, b: T, indices: [u32; 4]) -> U;
Copy link

Choose a reason for hiding this comment

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

Where's the documentation on these?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

@TheIronBorn TheIronBorn Jun 29, 2018

Choose a reason for hiding this comment

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

You could look at the planned API documentation rust-lang/stdarch#387 (we should mention this PR explicitly)

@pitdicker
Copy link
Owner

Nice! A lot of code... Wasn't stdsimd open for endianness conversion functions?

Builds currently fail with --features simd_support. Should we also enable all nightly features when simd_support is enabled?

This is a kind of messy compromise I ended up with for i128_support, to make it work on both nightly and stable. Better suggestions welcome, but it would be nice if the solution keeps working when stdsimd is stabilized.

O, and why is this PR opened against my repro? To get it into the PR that I half neglected (sorry)?

@TheIronBorn
Copy link
Author

TheIronBorn commented Jun 29, 2018

@dhardy recommended I make this pull against your branch. Not sure if this is what they meant, but it felt better than breaking reproducibility for a moment then making my own pull.

@TheIronBorn
Copy link
Author

Wasn't stdsimd open for endianness conversion functions?

I’m not sure what you mean by this

@TheIronBorn
Copy link
Author

I should probably make a swap_bytes_horizontally stdsimd PR

@TheIronBorn
Copy link
Author

Sent a PR rust-lang/stdarch#509

@TheIronBorn
Copy link
Author

Fixed a couple nits

@pitdicker pitdicker closed this Jul 20, 2018
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