-
Notifications
You must be signed in to change notification settings - Fork 69
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 multi-field repr(simd)
#621
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed. cc @rust-lang/compiler @rust-lang/compiler-contributors |
@rustbot second |
…e-asm, r=workingjubilee allow array-style simd in inline asm Required for [MCP#621](rust-lang/compiler-team#621) to be implemented. r? `@workingjubilee`
…asm, r=workingjubilee allow array-style simd in inline asm Required for [MCP#621](rust-lang/compiler-team#621) to be implemented. r? `@workingjubilee`
…rkingjubilee allow array-style simd in inline asm Required for [MCP#621](rust-lang/compiler-team#621) to be implemented. r? `@workingjubilee`
@rustbot label -final-comment-period +major-change-accepted |
Originally, repr(simd) supported only multi-field form. An array based version was later added and it's likely to become the only supported way (rust-lang/compiler-team#621). The array-based version is currently used in the standard library, and it is used to implement `portable-simd`. This change adds support to instantiating and using the array-based version.
…rkingjubilee allow array-style simd in inline asm Required for [MCP#621](rust-lang/compiler-team#621) to be implemented. r? `@workingjubilee`
…rkingjubilee allow array-style simd in inline asm Required for [MCP#621](rust-lang/compiler-team#621) to be implemented. r? `@workingjubilee`
Update stdarch submodule To pick up rust-lang/stdarch#1624 and unblock removing support for non-array-based-simd (rust-lang/compiler-team#621). try-job: dist-aarch64-linux try-job: dist-armv7-linux try-job: dist-x86_64-linux
Update stdarch submodule To pick up rust-lang/stdarch#1624 and unblock removing support for non-array-based-simd (rust-lang/compiler-team#621). try-job: dist-aarch64-linux try-job: dist-armv7-linux try-job: dist-x86_64-linux
Update stdarch submodule To pick up rust-lang/stdarch#1624 and unblock removing support for non-array-based-simd (rust-lang/compiler-team#621).
…-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
…-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
…er-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
…er-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
…-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
…-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
…-errors Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang#129400 ~~rust-lang#129481 for windows.
Ban non-array SIMD Nearing the end of rust-lang/compiler-team#621 ! Currently blocked on ~~rust-lang/compiler-builtins#673 ~~rust-lang/compiler-builtins#674 ~~rust-lang/rust#129400 ~~rust-lang/rust#129481 for windows.
Proposal
Originally,
repr(simd)
supported only multi-field form:https://github.com/rust-lang/stdarch/blob/5f5ddf8f9a404d1f12b531095b13bccce97357ea/crates/core_arch/src/x86/mod.rs#L324-L329
But with const generics showing up (and because counting fields like that isn't the most fun thing in the world), an array-based version was added:
Unsurprisingly, portable-simd uses that form:
https://github.com/rust-lang/portable-simd/blob/ceb26115928c5c69b10268fd2f9e500865c142d6/crates/core_simd/src/vector.rs#L80
That was a great addition, but came with the cost that we now need to handle both kinds everywhere that looks at simd.
For example, this test needs to check both
https://github.com/rust-lang/rust/blob/0fd50f3e019dddc47d1d6dbe35c4c1542098d9c5/tests/codegen/simd-intrinsic/simd-intrinsic-transmute-array.rs#L14-L20
And, even more interestingly, it demonstrates that things don't necessarily work the same way between the different approaches.
I propose that we remove the multi-field form.
repr(simd)
is not on a stabilization track, the array form is more readable, and nightly users (including core arch) can easily migrate to the array-based form.By only having one way to do it, we'll simplify things that need to handle or test simd types, like this code that currently has to check for empty simd types twice:
https://github.com/rust-lang/rust/blob/0fd50f3e019dddc47d1d6dbe35c4c1542098d9c5/compiler/rustc_hir_analysis/src/check/check.rs#L890-L910
Mentors or Reviewers
Removing code is generally the easier side of things, so no mentoring should be needed.
@workingjubilee or other portable-simd folks would likely make good reviewers.
Process
The main points of the Major Change Process are as follows:
@rustbot second
.-C flag
, then full team check-off is required.@rfcbot fcp merge
on either the MCP or the PR.You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
The text was updated successfully, but these errors were encountered: