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

Standard: Distribution<__m128i> features requirements are undocumented #1410

Closed
dragomang87 opened this issue Mar 13, 2024 · 7 comments
Closed
Assignees
Labels
C-docs Documentation

Comments

@dragomang87
Copy link

Common issues

Problem: rand::distributions::Standard for __m128i

Quick solution: do not have one

Details: Using version 0.8.5 and the documentation reports that there is impl Distribution<__m128i> for Standard .
However the following code

use rand::Rng;
use rand::distributions::Distribution;

use std::arch::x86_64::*;


pub fn generate_m128i(n: usize) -> Vec<__m128i> {
    // Set up generator
    let mut rng = rand::thread_rng();
    // Set up random variable
    let random_variable: rand::distributions::Standard;
    // Set up vector
    let mut vector: Vec<__m128i>  = Vec::new();
    // Generate vector
    for _ in 0..n {
        //vector.push(rng.gen::<__m128i>());
        //vector.push(rng.sample(Standard));
        vector.push(random_variable.sample(&mut rng));
    }
    return vector;
}

gives the error

error[E0277]: the trait bound `Standard: Distribution<std::arch::x86_64::__m128i>` is not satisfied
  --> src/random_generators.rs:18:21
   |
18 |         vector.push(random_variable.sample(&mut rng));
   |                     ^^^^^^^^^^^^^^^ ------ required by a bound introduced by this call
   |                     |
   |                     the trait `Distribution<std::arch::x86_64::__m128i>` is not implemented for `Standard`
   |
   = help: the following other types implement trait `Distribution<T>`:
             <Standard as Distribution<()>>
             <Standard as Distribution<(A, B)>>
             <Standard as Distribution<(A, B, C)>>
             <Standard as Distribution<(A, B, C, D)>>
             <Standard as Distribution<(A, B, C, D, E)>>
             <Standard as Distribution<(A, B, C, D, E, F)>>
             <Standard as Distribution<(A, B, C, D, E, F, G)>>
             <Standard as Distribution<(A, B, C, D, E, F, G, H)>>
           and 62 others

What am I doing wrong?

@TheIronBorn
Copy link
Collaborator

Do you have the simd_support crate feature enabled?

We seem to have overlooked that in documentation

@TheIronBorn
Copy link
Collaborator

TheIronBorn commented Mar 14, 2024

To be clear: Standard: Distribution<__m128i> requires the simd_support crate feature.

However, in v0.8.5 simd_support is broken due to packed_simd and it seems rather permanent this time. If you still need the trait impl, you can try the 0.9 alpha

@TheIronBorn TheIronBorn changed the title "impl Distribution<__m128i> for Standard" is in 0.8.5 but get "the trait Distribution<std::arch::x86_64::__m128i> is not implemented for Standard" Standard: Distribution<__m128i> features requirements are undocumented Mar 14, 2024
@TheIronBorn
Copy link
Collaborator

Is anyone aware of a way to fix the documentation if the simd_support feature won't compile?

@dragomang87
Copy link
Author

I hadn't notice the simd_support as I was not expecting nightly features to be needed. This would be the std::simd nightly crate right?
I guess the easiest at this point is to keep using rust stable, generate two i64 and use _mm_set_epi64x.
It seems the code for Distribution<__m128i> generates __m128i indirectly by first creating u8x16 using .fill_bytes() anyway.

Bitwise speaking, does it makes a difference if the __m128i is constructed by first randomly generating u8x16, i64, u64, i32, u32, ... ? If I read the code correctly they all use .fill_bytes() so it should make no difference.

Also, looking at `rand/distributions/integer.rs:13:

#[cfg(all(target_arch = "x86", feature = "simd_support"))]
use core::arch::x86::{__m128i, __m256i};
#[cfg(all(target_arch = "x86_64", feature = "simd_support"))]
use core::arch::x86_64::{__m128i, __m256i};

it seems that the __m128i is indeed the one from core::arch so the simd_support is only needed because the implementation goes through generating u8x16 first. Wouldn't it have made more sense to indeed generate a pair of u64 or i64 instead and avoid needing simd? That way Distribution<__m128i> could go stable independently of simd.

PS: why does integer.rs:118 use from_bits of the FromBits trait? The only place where I found it is in the Ubuntu nightly documentation from web.mit.edu which is Version 1.26.0 (a77568041 2018-05-07) from 2018.
Both stable and nightly define From trait with the from method. What's the reason for the different implementation?

@TheIronBorn
Copy link
Collaborator

Yes we are dropping the feature requirement in 0.9 as well as the from_bits along with the whole packed_simd crate

@dhardy dhardy added the C-docs Documentation label Jul 26, 2024
@dhardy
Copy link
Member

dhardy commented Jul 26, 2024

This is about documentation of feature flags. May be affected by #1450. There's still no obvious doc here:
image

@dhardy
Copy link
Member

dhardy commented Oct 16, 2024

The above is the expected doc for 0.9: this feature requires x86 or x86-64 but does not require simd_support.

@dhardy dhardy closed this as completed Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-docs Documentation
Projects
None yet
Development

No branches or pull requests

4 participants