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

counts/offsets shouldn't reuse the Self type from the trait #11

Open
ekmett opened this issue Feb 8, 2020 · 1 comment
Open

counts/offsets shouldn't reuse the Self type from the trait #11

ekmett opened this issue Feb 8, 2020 · 1 comment

Comments

@ekmett
Copy link

ekmett commented Feb 8, 2020

popcnt, tzcnt, bzhi, bextr and a few others should probably take/return a u32 (or similar) to match the register types of the instructions, rather than the Self type of the key argument when they talk about positions. Otherwise the calling convention of these combinators is rather different than the underlying assembly opcodes, and any desugaring into those instructions is rather unlikely.

As a reference from the standard library consider that .count_ones() returns a u32 for a u64 argument.

@gnzlbg
Copy link
Owner

gnzlbg commented Feb 8, 2020

Thank you for the issue. I'm a bit low on time, but will keep this in mind for the next time I'll do a maintenance pass over the library. PRs are obviously welcome in case somebody wants to fix this.

A future version of this library will probably only expose "software fallbacks" for the core::arch intrinsics, leaving the decision of when to use the core::arch intrinsic or the software fallback up to the user, so it might be worth it for you to consider to just start doing that already in your application (that would obviously be a semver breaking change, so if you use current versions of this library whatever semantics they expose will be obviously supported forever).

We also have an RFC that's about to get merged in the pipeline (target-feature 1.1 RFC) that will more clearly express what is exactly unsafe about the core::arch intrinsics, so if your concerns about core::arch are the amount of unsafe code required, that might improve "soonish".

This crate predates core::arch and its design by a couple of years (although what was learned here certainly influenced its design). Right now, I'm more of the opinion that without a proper effect-system for target-features and a way to be generic about them this crate cannot really make a meaningful decision about how to generate code for these intrinsics. For example,

  • should the safe wrappers do compile-time feature detection?
  • run-time feature detection?
  • no feature detection at all because the wrapper is used in an execution path that always has the feature enabled?

While we are slowly making strides towards a good effect system for Rust (e.g. for handling const-effects, see RFC2632), a really a good effect system for Rust is unfortunately still a couple of PhD thesis away at this point (see rust-effects/target-feature-1.1).

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

No branches or pull requests

2 participants