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

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

Merged
merged 1 commit into from
Jun 8, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jun 8, 2024

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

  • [0b01001001, 0b01] on little endian
  • [0b10010010, 0b10] on big endian

@calebzulawski
Copy link
Member

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 rust-lang/portable-simd#423

@RalfJung
Copy link
Member Author

RalfJung commented Jun 8, 2024

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"?

@RalfJung
Copy link
Member Author

RalfJung commented Jun 8, 2024

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

Great, thanks. :)
@bors r+

@bors
Copy link
Contributor

bors commented Jun 8, 2024

📌 Commit 2ecd6ed has been approved by RalfJung

It is now in the queue for this repository.

bors added a commit that referenced this pull request Jun 8, 2024
portable-simd: add test for non-power-of-2 bitmask

`@calebzulawski` is that the intended behavior? Specifically for arrays, the bitmask `[1, 0, 0, 1, 0, 0, 1, 0, 1, 0]` becomes
- `[0b01001001, 0b01]` on little endian
- `[0b10010010, 0b10]` on big endian
@bors
Copy link
Contributor

bors commented Jun 8, 2024

⌛ Testing commit 2ecd6ed with merge 813c0bf...

@calebzulawski
Copy link
Member

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
Copy link
Member Author

RalfJung commented Jun 8, 2024

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
Copy link
Member Author

RalfJung commented Jun 8, 2024

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
Copy link
Member Author

RalfJung commented Jun 8, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jun 8, 2024

📌 Commit 7a30c8e has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 8, 2024

⌛ Testing commit 7a30c8e with merge d46c2e7...

@bors
Copy link
Contributor

bors commented Jun 8, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing d46c2e7 to master...

@bors bors merged commit d46c2e7 into rust-lang:master Jun 8, 2024
8 checks passed
@RalfJung RalfJung deleted the simd-bitmask branch June 8, 2024 17:50
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.

3 participants