Skip to content

core::arch::x86_64::_mm_shuffle_ps input doesn't match official docs #62490

Closed
@Lokathor

Description

@Lokathor
Contributor

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.

Activity

hellow554

hellow554 commented on Jul 8, 2019

@hellow554
Contributor

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

Lokathor commented on Jul 8, 2019

@Lokathor
ContributorAuthor

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

nikic commented on Jul 8, 2019

@nikic
Contributor

I believe this was changed recently because Intel's definition is wrong/inconsistent. See rust-lang/stdarch#522.

Lokathor

Lokathor commented on Jul 8, 2019

@Lokathor
ContributorAuthor

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.

added
A-docsArea: Documentation for any part of the project, including the compiler, standard library, and tools
E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
on Aug 6, 2019
afnanenayet

afnanenayet commented on Jan 4, 2020

@afnanenayet
Contributor

I could take this!

@rustbot claim

self-assigned this
on Jan 4, 2020
added
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.
on Mar 6, 2020
JohnTitor

JohnTitor commented on Jul 24, 2020

@JohnTitor
Member

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

removed their assignment
on Jul 24, 2020
georgio

georgio commented on Jul 31, 2020

@georgio
Contributor

@rustbot claim

self-assigned this
on Jul 31, 2020

6 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsA-intrinsicsArea: IntrinsicsE-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @nikic@Centril@hellow554@afnanenayet@jonas-schievink

    Issue actions

      `core::arch::x86_64::_mm_shuffle_ps` input doesn't match official docs · Issue #62490 · rust-lang/rust