Rust: `core::arch::x86_64::_mm_shuffle_ps` input doesn't match official docs

[Lokathor]

x86_64::_mm_shuffle_ps (and by extension x86::_mm_shuffle_ps) have i32 as the mask type, but the intel intrinsics guide lists unsigned int as the mask type.

This issue has been assigned to @georgio via this comment.

[hellow554]

Hey @Lokathor 👋

I (I think we all) very much appreciate the work you are doing by looking at the docs and finding unsound things. What about opening a PR and fixing them immediatly (some of them could be done by using the github editor)? It's a very great opportunity to make Rust a better programming langauge.

If you feel like it, please open a PR and reference this issue with one of the closing keywords github provides.

[Lokathor]

I am well aware of how GitHub works :D unfortunately for us all, I only have so much programming time. I've been busy at work safe wrapping all these intrinsics for my own lib, which is why I'm filing bugs on them as they come up. However, in my experience, filing even a single PR to this repo is rather involved and so I've mostly given up on doing it.

As to this _particular_ issue, it's a change to a stable API, so _at minimum_ a full crater run would have to be performed of course. The input mask value has to be a const, so you get a lot of literals of course and those can usually infer to the new type fine, but you also get a lot of const declarations too, which would break. I suspect that at least some people will get their code broken. Given that the intrinsic only uses the 8 lowest bits, it would be entirely reasonable for T-libs to say they don't care about the i32/u32 difference compared to code breaks and just document the (very minor) difference.

or they could go for absolute exactness and declare the breaks to be the fair product of a bug fix.

up to them

[nikic]

I believe this was changed recently because Intel's definition is wrong/inconsistent. See https://github.com/rust-lang-nursery/stdsimd/issues/522.

[Lokathor]

ah lovely. of course the guide is wrong :P

well, then consider this a minor documentation issue. the function should simply explain that it doesn't match the spec and why and such.

[afnanenayet]

I could take this!

@rustbot claim

[JohnTitor]

Triage: I'm going to release assignment due to inactivity.
@afnanenayet If you're still interested in this, feel free to re-claim.
@rustbot release-assignment

[georgio]

@rustbot claim

[georgio]

I have submitted https://github.com/rust-lang/stdarch/pull/879 in order to fix this. I believe we could resolve this issue once that PR is merged by updating the submodule.