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

Bye bye MMX! #890

Merged
merged 1 commit into from
Sep 3, 2020
Merged

Bye bye MMX! #890

merged 1 commit into from
Sep 3, 2020

Conversation

mati865
Copy link
Contributor

@mati865 mati865 commented Sep 2, 2020

Fixes #823

@rust-highfive
Copy link

r? @Amanieu

(rust_highfive has picked a reviewer for you, use r? to override)

@bjorn3
Copy link
Member

bjorn3 commented Sep 2, 2020

There is also special casing in the compiler to give the mmx vector type the right LLVM type. That should also be removed.

@mati865
Copy link
Contributor Author

mati865 commented Sep 2, 2020

@bjorn3 sure, after this is merged I'm going to grep mmx on Rust repo.

@Amanieu
Copy link
Member

Amanieu commented Sep 2, 2020

I'm a bit concerned about the number of functions we're removing from SSE, SSE2, etc. Are we sure that all of these are MMX-only?

EDIT: Apparently just removing anything that uses __m64 seems to be correct. See MSVC docs:

The __m64 data type is not supported on x64 processors. Applications that use __m64 as part of MMX intrinsics must be rewritten to use equivalent SSE and SSE2 intrinsics.

@mati865
Copy link
Contributor Author

mati865 commented Sep 3, 2020

I'm a bit concerned about the number of functions we're removing from SSE, SSE2, etc. Are we sure that all of these are MMX-only?

EDIT: Apparently just removing anything that uses __m64 seems to be correct. See MSVC docs:

The __m64 data type is not supported on x64 processors. Applications that use __m64 as part of MMX intrinsics must be rewritten to use equivalent SSE and SSE2 intrinsics.

I wasn't sure whether we want to just remove MMX instructions or all the instructions involving MMX registers.
Went for the latter but I can bring back __m64 and SSE{,2} instructions using MMX registers.

@Amanieu Amanieu merged commit 078ff4f into rust-lang:master Sep 3, 2020
@Amanieu
Copy link
Member

Amanieu commented Sep 3, 2020

Make sure to tag with relnotes when updating the submodule in rust-lang/rust.

@Amanieu
Copy link
Member

Amanieu commented Sep 3, 2020

Hmm actually this is only unstable code, so we don't want it in the release notes.

@danieldjewell
Copy link

danieldjewell commented Sep 19, 2020

It would appear that this change has broken compilation of rust-lang/packed_simd (see referenced issues). I assume that it was this pull that broke things as the "_mm_movemask_pi8" macro appears to have been removed?

Could someone who is more versed in this than I perhaps take a look?

@Amanieu It would appear that your concerns were well founded.

Edit:

MSVC docs

@Amanieu I'm not sure it's a good idea for a programming language to reference another programming language's documentation unless it's written in that language.... (Correct me if I'm wrong but Rust is written in Rust?) Perhaps the best reference on what is and is not supported by x86 would be: https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html

PDF: https://software.intel.com/content/www/us/en/develop/download/intel-64-and-ia-32-architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4.html (5000! pages! wow!)

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2020
Remove MMX from Rust

Follow-up to rust-lang/stdarch#890
This removes most of MMX from Rust (tests pass with small changes), keeping stable `is_x86_feature_detected!("mmx")` working.
Stupremee added a commit to Stupremee/packed_simd that referenced this pull request Sep 22, 2020
This has to be done, because `_mm_movemask_pi8` got removed
from stdarch in rust-lang/stdarch#890
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.

Remove the MMX intrinsics
5 participants