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

Remove the MMX intrinsics #823

Closed
gnzlbg opened this issue Oct 22, 2019 · 6 comments · Fixed by #890
Closed

Remove the MMX intrinsics #823

gnzlbg opened this issue Oct 22, 2019 · 6 comments · Fixed by #890

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 22, 2019

This will need some motivation and rationale, but the TL;DR is that they are very hard to use correctly, and they cause subtle bugs that take a lot of people a lot of time to debug. They aren't in the path towards stabilization either, so those who really need them can migrate towards using inline assembly instead, which is just as unstable.

We should probably phase in a deprecation warning first, and remove them some time later.

@hanna-kruppe
Copy link

I would even argue they are impossible to use correctly in several circumstances:

So if you're on one of these targets, and use certain operations on floats anywhere in the program, you're basically entirely at the mercy of LLVM not getting too aggressive with reordering code.

@mati865
Copy link
Contributor

mati865 commented Sep 2, 2020

LLVM developers are wondering about removing MMX: https://www.phoronix.com/scan.php?page=news_item&px=LLVM-Goodbye-MMX-Maybe

@Amanieu
Copy link
Member

Amanieu commented Sep 2, 2020

It seems that LLVM's main concern is backwards compatibility with existing code that uses __m64 and MMX intrinsics. We don't have this concern in Rust because there is no existing (stable) code using MMX.

I propose simply removing the MMX intrinsics and MMX types. I don't think there is much value in emulating them on top of SSE2 since anyone writing new code should not be using MMX intrinsics.

Note that there is still an issue with inline asm in that it can't properly express clobbers of the FP/MMX registers, so you currently can't use it with MMX instructions.

@lu-zero
Copy link
Contributor

lu-zero commented Sep 2, 2020

Sounds a good idea given the potential harm.

@mati865
Copy link
Contributor

mati865 commented Sep 2, 2020

I could prepare PR but what to do with is_x86_feature_detected!("mmx")?
It's currently works on stable: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=41534dce6ad51df68df2e79323adfd44

@Amanieu
Copy link
Member

Amanieu commented Sep 2, 2020

It's fine to have feature detection for a feature even if we don't support codegen for it.

@mati865 mati865 mentioned this issue Sep 2, 2020
vks added a commit to vks/rand that referenced this issue Oct 8, 2020
This is necessary, because support for `__m64` was removed from nighly
Rust [1].

Fixes rust-random#1050.

[1] rust-lang/stdarch#823
vks added a commit to rust-random/rand that referenced this issue Oct 15, 2020
This is necessary, because support for `__m64` was removed from nighly
Rust [1].

Fixes #1050.

[1] rust-lang/stdarch#823
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 a pull request may close this issue.

5 participants