search

rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.14k stars 318 forks source link

portable-simd: add test for non-power-of-2 bitmask #3655

Closed RalfJung closed 3 weeks ago

RalfJung commented 3 weeks ago

@calebzulawski is that the intended behavior? Specifically for arrays, the bitmask [1, 0, 0, 1, 0, 0, 1, 0, 1, 0] becomes

calebzulawski commented 3 weeks ago

Yes, I think that's expected (we reverse the bitmasks in std::simd)

FYI, we actually just removed use of that variation of the intrinsic in https://github.com/rust-lang/portable-simd/pull/423

RalfJung commented 3 weeks ago

FYI, we actually just removed use of that variation of the intrinsic in https://github.com/rust-lang/portable-simd/pull/423

The intrinsic is still called in other places though. In which sense is it "removed"?

RalfJung commented 3 weeks ago

Yes, I think that's expected (we reverse the bitmasks in std::simd)

Great, thanks. :) @bors r+

bors commented 3 weeks ago

:pushpin: Commit 2ecd6ed21996ba4ce5f000988ddf7170907fc423 has been approved by RalfJung

It is now in the queue for this repository.

bors commented 3 weeks ago

:hourglass: Testing commit 2ecd6ed21996ba4ce5f000988ddf7170907fc423 with merge 813c0bfa0dcf13e22a9b7af1e0496fa0dfc16638...

calebzulawski commented 3 weeks ago

FYI, we actually just removed use of that variation of the intrinsic in rust-lang/portable-simd#423

The intrinsic is still called in other places though. In which sense is it "removed"?

We still use the intrinsic, but currently (not yet synced to rust-lang/rust) we only use integer return types

RalfJung commented 3 weeks ago

Ah, this seems to fail on big-endian...

We still use the intrinsic, but currently (not yet synced to rust-lang/rust) we only use integer return types

I see. If the array support ever gets removed entirely, please ping me.

RalfJung commented 3 weeks ago

The docs say

No matter whether the output is an array or an unsigned integer, it is treated as a single contiguous list of bits.

On big-endian, the bitlist 0b1001001010u16 transmutes to the array [0b10u8, 0b01001010u8]. So either the docs are wrong or my examples listed above are wrong.

Maybe that explains why you had trouble with this in the non-power-of-2 PR?

RalfJung commented 3 weeks ago

@bors r+

bors commented 3 weeks ago

:pushpin: Commit 7a30c8e392346b1d9d4ef5b3bb57a6a5d12d6d79 has been approved by RalfJung

It is now in the queue for this repository.

bors commented 3 weeks ago

:hourglass: Testing commit 7a30c8e392346b1d9d4ef5b3bb57a6a5d12d6d79 with merge d46c2e7a73c34a2f03132f67afeb1d9406601aa3...

bors commented 3 weeks ago

:sunny: Test successful - checks-actions Approved by: RalfJung Pushing d46c2e7a73c34a2f03132f67afeb1d9406601aa3 to master...