Closed
Description
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.
Metadata
Metadata
Assignees
Labels
Area: Documentation for any part of the project, including the compiler, standard library, and toolsArea: IntrinsicsCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.Relevant to the compiler team, which will review and decide on the PR/issue.Relevant to the library API team, which will review and decide on the PR/issue.
Activity
hellow554 commentedon Jul 8, 2019
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 commentedon Jul 8, 2019
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 commentedon Jul 8, 2019
I believe this was changed recently because Intel's definition is wrong/inconsistent. See rust-lang/stdarch#522.
Lokathor commentedon Jul 8, 2019
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 commentedon Jan 4, 2020
I could take this!
@rustbot claim
JohnTitor commentedon Jul 24, 2020
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 commentedon Jul 31, 2020
@rustbot claim
6 remaining items