rust-lang / portable-simd

The testing ground for the future of portable SIMD in Rust
Apache License 2.0
903 stars 81 forks source link

from_bitmask_vector on big-endian calls simd_select_bitmask with the mask at the wrong end of the byte #379

Closed RalfJung closed 9 months ago

RalfJung commented 11 months ago

This code, running on big-endian

    let bitmask = u8x4::from_array([0b00001101, 0, 0, 0]);
    assert_eq!(
        mask32x4::from_bitmask_vector(bitmask),
        mask32x4::from_array([true, false, true, true]),
    );

ends up calling simd_select_bitmask with the mask being 0b10110000. I think that's wrong; for simd_bitmask the mask lives in the least-sigificant bits (see https://github.com/rust-lang/portable-simd/pull/378) so I would expect that simd_select_bitmask should be called with 0b00001011.

This issue is reminiscent of https://github.com/rust-lang/portable-simd/pull/267.

RalfJung commented 11 months ago

I am assuming that simd_select_bitmask is supposed to pass this testcase:

    unsafe {
        let selected1 = simd_select_bitmask::<u16, _>(
            if cfg!(target_endian = "little") { 0b1010001101001001 } else { 0b1001001011000101 },
            i32x16::splat(1), // yes
            i32x16::splat(0), // no
        );
        let selected2 = simd_select_bitmask::<[u8; 2], _>(
            if cfg!(target_endian = "little") {
                [0b01001001, 0b10100011]
            } else {
                [0b10010010, 0b11000101]
            },
            i32x16::splat(1), // yes
            i32x16::splat(0), // no
        );
        assert_eq!(selected1, i32x16::from_array([1, 0, 0, 1, 0, 0, 1, 0, 1, 1, 0, 0, 0, 1, 0, 1]));
        assert_eq!(selected2, selected1);
        // Also try masks less than a byte long.
        let selected1 = simd_select_bitmask::<u8, _>(
            if cfg!(target_endian = "little") { 0b1000 } else { 0b0001 },
            i32x4::splat(1), // yes
            i32x4::splat(0), // no
        );
        let selected2 = simd_select_bitmask::<[u8; 1], _>(
            if cfg!(target_endian = "little") { [0b1000] } else { [0b0001] },
            i32x4::splat(1), // yes
            i32x4::splat(0), // no
        );
        assert_eq!(selected1, i32x4::from_array([0, 0, 0, 1]));
        assert_eq!(selected2, selected1);
    }

That matches the testcase for simd_bitmask mentioned in https://github.com/rust-lang/portable-simd/pull/378.