-
Notifications
You must be signed in to change notification settings - Fork 13.5k
fix bitcast of single-element SIMD vectors #143194
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
fix bitcast of single-element SIMD vectors #143194
Conversation
Some changes occurred in compiler/rustc_codegen_ssa |
Thanks. @bors r+ rollup |
…-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts rust-lang#142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 `@single_element_simd(<1` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a &rust-lang#96;T&rust-lang#96; to &rust-lang#96;Tx1&rust-lang#96; (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc `@sayantn` r? `@scottmcm`
Rollup of 7 pull requests Successful merges: - #142429 (`tests/ui`: A New Order [13/N]) - #143066 (Use let chains in the new solver) - #143156 (inherit `#[align]` from trait method prototypes) - #143176 (fix typos and improve clarity in documentation) - #143187 (Add my work email to mailmap) - #143194 (fix bitcast of single-element SIMD vectors) - #143199 (Re-disable `tests/run-make/short-ice` on Windows MSVC again) r? `@ghost` `@rustbot` modify labels: rollup
My bad. Thanks for adding more test coverage. |
Considering the error message in the CI log, do we need to add something like this? #![cfg_attr(target_family = "wasm", target_feature(enable = "simd128"))] |
@bors r- |
51e1666
to
e86fddc
Compare
Turns out you need a bunch of @rustbot ready |
stdarch CI is currently broken because of this issue, it would be nice to get this merged. |
Oh, sorry! @bors r+ rollup |
@@ -1123,7 +1123,7 @@ pub(super) fn transmute_immediate<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( | |||
// While optimizations will remove no-op transmutes, they might still be | |||
// there in debug or things that aren't no-op in MIR because they change | |||
// the Rust type but not the underlying layout/niche. | |||
if from_scalar == to_scalar { | |||
if from_scalar == to_scalar && from_backend_ty == to_backend_ty { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add some more comments on why both of these must obtain. That can happen after this merges and the discussion on Zulip settles though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The entire function is cursed. It talks about transmuting "immediates" but then takes Scalar
information as arguments. This needs some refactoring figuring out which class of types it is meant for, and aligning name and signature and comments.
…-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts rust-lang#142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 `@single_element_simd(<1` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a &rust-lang#96;T&rust-lang#96; to &rust-lang#96;Tx1&rust-lang#96; (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc `@sayantn` r? `@scottmcm`
Rollup of 12 pull requests Successful merges: - #141829 (Specialize sleep_until implementation for unix (except mac)) - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142568 (Use the .drectve section for exporting symbols from dlls on Windows) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143206 (Align attr fixes) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts rust-lang#142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 ``@single_element_simd(<1`` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a &rust-lang#96;T&rust-lang#96; to &rust-lang#96;Tx1&rust-lang#96; (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc ``@sayantn`` r? ``@scottmcm``
Rollup of 11 pull requests Successful merges: - #141829 (Specialize sleep_until implementation for unix (except mac)) - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143206 (Align attr fixes) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts rust-lang#142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 ```@single_element_simd(<1``` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a &rust-lang#96;T&rust-lang#96; to &rust-lang#96;Tx1&rust-lang#96; (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc ```@sayantn``` r? ```@scottmcm```
Rollup of 12 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143206 (Align attr fixes) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts rust-lang#142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 ````@single_element_simd(<1```` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a &rust-lang#96;T&rust-lang#96; to &rust-lang#96;Tx1&rust-lang#96; (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc ````@sayantn```` r? ````@scottmcm````
Rollup of 11 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143038 (avoid suggesting traits from private dependencies) - #143194 (fix bitcast of single-element SIMD vectors) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
…-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts rust-lang#142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 `````@single_element_simd(<1````` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a &rust-lang#96;T&rust-lang#96; to &rust-lang#96;Tx1&rust-lang#96; (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc `````@sayantn````` r? `````@scottmcm`````
Rollup of 10 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143194 (fix bitcast of single-element SIMD vectors) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143260 (Use the correct export kind for __rust_alloc_error_handler_should_panic) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 9 pull requests Successful merges: - #141847 (Explain `TOCTOU` on the top of `std::fs`, and reference it in functions) - #142138 (Add `Vec::into_chunks`) - #142321 (Expose elf abi on ppc64 targets) - #142886 (ci: aarch64-gnu: Stop skipping `panic_abort_doc_tests`) - #143194 (fix bitcast of single-element SIMD vectors) - #143231 (Suggest use another lifetime specifier instead of underscore lifetime) - #143232 ([COMPILETEST-UNTANGLE 3/N] Use "directives" consistently within compiletest) - #143258 (Don't recompute `DisambiguatorState` for every RPITIT in trait definition) - #143274 (ci: support optional jobs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143194 - folkertdev:fix-single-element-simd-bitcast, r=workingjubilee fix bitcast of single-element SIMD vectors in effect this reverts #142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR. https://godbolt.org/z/PaaGWTv5a ```rust #![feature(repr_simd)] #[repr(simd)] #[derive(Clone, Copy)] struct S([i64; 1]); #[no_mangle] pub extern "C" fn single_element_simd(b: S) -> i64 { unsafe { std::mem::transmute(b) } } ``` at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type. ```llvm define noundef i64 ``````@single_element_simd(<1`````` x i64> %b) unnamed_addr { start: ret <1 x i64> %b } ``` The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general. zullip thread: [#t-compiler > Is transmuting a `T` to `Tx1` (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799) cc ``````@sayantn`````` r? ``````@scottmcm``````
This issue can happen in specific conditions even without nightly features, right? |
in effect this reverts #142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR.
https://godbolt.org/z/PaaGWTv5a
at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type.
The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general.
zullip thread: #t-compiler > Is transmuting a `T` to `Tx1` (one-element SIMD vector) UB?
cc @sayantn
r? @scottmcm