Skip to content

Remove the portable-simd unpublished crate #216

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

Closed
wants to merge 1 commit into from

Conversation

Kerollmops
Copy link
Member

In order to be able to publish the v0.9.0 on crates.io, a crate must have all its dependencies available on crates.io. Unfortunately, the portable-simd crate is not published but is part of the core library, we can try to use this instead.

I am encountering only one final issue with the above-defined library switch. The Mask<i16, 8_usize>::to_bitmask doesn't exist in the core library. I must find a way to reproduce the behavior of mask.to_bitmask()[0] as we only need that.

The internal implementations of the to_bitmask method do not look too complex:

@saik0
Copy link
Contributor

saik0 commented Feb 25, 2022

For context

In summary

to_bitmask is not present in std because it requires the generic_const_exprs and std is compiled with no features.

It's a common enough operation that supporting int/byte bitmasks without generic_const_exprs is being actively worked on.

Next steps

We're fortunate in that we use u16x8s. The masks were interested in happen to map perfectly onto a u8.

I think you're suggesting we write a specialized implementation for 8 lane width bitmasks(?). Trouble is: we don't have access to intrinsics::simd_bitmask and would need to reimplement it. I think we'd be better off contributing to the upstream effort.

@saik0
Copy link
Contributor

saik0 commented Feb 26, 2022

Demo of changes we'd need to make if merged. (The git dep could be removed in favor of std::simd, the Cargo.toml changes are only to demo that there are no features)

saik0#8

@Kerollmops
Copy link
Member Author

Kerollmops commented Feb 28, 2022

Hey @saik0,

I have another idea about this issue, why can't we just release this version but we remove the simd feature from the Cargo.toml, this way no one will be able to enable it and see that it doesn't compile. We keep the core::simd dependency to be able to cartes.io.

I will, obviously, need to change the release notes, the parts that talk about performance improvements thanks to your SIMD-related work. All of that while waiting for the core::simd module to be updated.

What do you think about that?

@saik0
Copy link
Contributor

saik0 commented Feb 28, 2022

The ToBitmask branch has been merged into portable-simd/master and is being merged into rust nightly

If there's nothing time sensitive about the release I think we should wait. I can open a PR similar to saik0#8 but based nightly once it's merged.

@saik0
Copy link
Contributor

saik0 commented Mar 2, 2022

Merged. See #217

@bors bors bot closed this in fa246d7 Mar 2, 2022
@bors bors bot closed this in #217 Mar 2, 2022
@Kerollmops Kerollmops deleted the depends-on-std-simd branch March 2, 2022 09:25
not-jan pushed a commit to not-jan/roaring-rs that referenced this pull request Aug 31, 2022
217: Replace portable-simd create with core::simd r=Kerollmops a=saik0

Closes RoaringBitmap#216

Co-authored-by: saik0 <[email protected]>
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.

2 participants