Skip to content

Conversation

@scottmcm
Copy link
Member

For things that only change the valid ranges, we can just return the input, rather than making the LLVMBuildBitCast call and having it then do nothing.

I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.

For things that only change the valid ranges, we can just skip the `LLVMBuildBitCast` call.

I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

r? @WaffleLapkin

rustbot has assigned @WaffleLapkin.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@WaffleLapkin
Copy link
Member

@bors r+ rollup

@WaffleLapkin
Copy link
Member

did not mean to clone PR...
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 24, 2025

📌 Commit 5d16a7b has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 24, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 25, 2025
…=WaffleLapkin

Avoid a bitcast FFI call in transmuting

For things that only change the valid ranges, we can just return the input, rather than making the `LLVMBuildBitCast` call and having *it* then do nothing.

I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
bors added a commit that referenced this pull request Jun 25, 2025
Rollup of 15 pull requests

Successful merges:

 - #135731 (Implement parsing of pinned borrows)
 - #138780 (Add `#[loop_match]` for improved DFA codegen)
 - #142453 (Windows: make `read_dir` stop iterating after the first error is encountered)
 - #142633 (Error on invalid signatures for interrupt ABIs)
 - #142768 (Avoid a bitcast FFI call in transmuting)
 - #142825 (Port `#[track_caller]` to the new attribute system)
 - #142844 (Enable short-ice for Windows)
 - #142934 (Tweak `-Zmacro-stats` measurement.)
 - #142955 (Couple of test suite fixes for cg_clif)
 - #142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠)
 - #142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect)
 - #142982 (Corrected spelling mistake in c_str.rs)
 - #142983 (Taint body on invalid call ABI)
 - #142988 (Update wasm-component-ld to 0.5.14)
 - #142993 (Update cargo)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit adaf340 into rust-lang:master Jun 25, 2025
20 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jun 25, 2025
rust-timer added a commit that referenced this pull request Jun 25, 2025
Rollup merge of #142768 - scottmcm:avoid-unneeded-bitcast, r=WaffleLapkin

Avoid a bitcast FFI call in transmuting

For things that only change the valid ranges, we can just return the input, rather than making the `LLVMBuildBitCast` call and having *it* then do nothing.

I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
@scottmcm scottmcm deleted the avoid-unneeded-bitcast branch June 26, 2025 01:04
@scottmcm
Copy link
Member Author

Huh, I wasn't expecting anything material from this, but something in #142997 was, so out of curiosity
@rust-timer build 4e95f4a

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4e95f4a): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.4% [-1.4%, -1.4%] 1
Improvements ✅
(secondary)
-1.4% [-1.4%, -1.4%] 1
All ❌✅ (primary) -1.4% [-1.4%, -1.4%] 1

Max RSS (memory usage)

Results (primary -1.3%, secondary -1.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
6.0% [4.2%, 7.8%] 2
Improvements ✅
(primary)
-1.3% [-1.3%, -1.3%] 1
Improvements ✅
(secondary)
-4.1% [-7.6%, -2.5%] 5
All ❌✅ (primary) -1.3% [-1.3%, -1.3%] 1

Cycles

Results (secondary -5.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.0% [-5.1%, -4.8%] 3
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 689.044s -> 689.011s (-0.00%)
Artifact size: 372.03 MiB -> 372.01 MiB (-0.00%)

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 26, 2025
Rollup of 15 pull requests

Successful merges:

 - rust-lang/rust#135731 (Implement parsing of pinned borrows)
 - rust-lang/rust#138780 (Add `#[loop_match]` for improved DFA codegen)
 - rust-lang/rust#142453 (Windows: make `read_dir` stop iterating after the first error is encountered)
 - rust-lang/rust#142633 (Error on invalid signatures for interrupt ABIs)
 - rust-lang/rust#142768 (Avoid a bitcast FFI call in transmuting)
 - rust-lang/rust#142825 (Port `#[track_caller]` to the new attribute system)
 - rust-lang/rust#142844 (Enable short-ice for Windows)
 - rust-lang/rust#142934 (Tweak `-Zmacro-stats` measurement.)
 - rust-lang/rust#142955 (Couple of test suite fixes for cg_clif)
 - rust-lang/rust#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠)
 - rust-lang/rust#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect)
 - rust-lang/rust#142982 (Corrected spelling mistake in c_str.rs)
 - rust-lang/rust#142983 (Taint body on invalid call ABI)
 - rust-lang/rust#142988 (Update wasm-component-ld to 0.5.14)
 - rust-lang/rust#142993 (Update cargo)

r? `@ghost`
`@rustbot` modify labels: rollup
@sayantn
Copy link
Contributor

sayantn commented Jun 27, 2025

Idk how, but this is creating miscompilations when casting from T to Tx1 (single-element vector), causing stdarch CI to fail

Which brings up the question, are these transmutes even valid? Because they are used quite a bit in stdarch (mainly in ARM, which has 1-element SIMD types)

Comment on lines -1126 to 1128
// the Rust type but not the underlying layout/niche.
if from_scalar == to_scalar && from_backend_ty == to_backend_ty {
if from_scalar == to_scalar {
return imm;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this change is incorrect. We can't just implicitly convert between types that have the same scalar representation, but a different backend type. For instance

https://godbolt.org/z/PaaGWTv5a

#![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) }
}

generates broken IR:

define noundef i64 @single_element_simd(<1 x i64> %b) unnamed_addr {
start:
  ret <1 x i64> %b
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very strange that a function that is supposed to work on all "immediates" just takes abi::Scalar. That doesn't fully describe an immediate. Either the function should be renamed to transmute_scalar (and never be called for non-scalar types), or it should take the full BackendRepr so that it has a clue what the two immediates are.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 29, 2025
…-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`
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 1, 2025
…-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`
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 2, 2025
…-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``
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 2, 2025
…-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```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 2, 2025
…-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````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 2, 2025
…-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`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 2, 2025
…-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``````
rust-timer added a commit that referenced this pull request Jul 2, 2025
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 &#96;T&#96; to &#96;Tx1&#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``````
tautschnig pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 3, 2025
…kingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#135731 (Implement parsing of pinned borrows)
 - rust-lang#138780 (Add `#[loop_match]` for improved DFA codegen)
 - rust-lang#142453 (Windows: make `read_dir` stop iterating after the first error is encountered)
 - rust-lang#142633 (Error on invalid signatures for interrupt ABIs)
 - rust-lang#142768 (Avoid a bitcast FFI call in transmuting)
 - rust-lang#142825 (Port `#[track_caller]` to the new attribute system)
 - rust-lang#142844 (Enable short-ice for Windows)
 - rust-lang#142934 (Tweak `-Zmacro-stats` measurement.)
 - rust-lang#142955 (Couple of test suite fixes for cg_clif)
 - rust-lang#142977 (rustdoc: Don't mark `#[target_feature]` functions as ⚠)
 - rust-lang#142980 (Reduce mismatched-lifetime-syntaxes suggestions to MaybeIncorrect)
 - rust-lang#142982 (Corrected spelling mistake in c_str.rs)
 - rust-lang#142983 (Taint body on invalid call ABI)
 - rust-lang#142988 (Update wasm-component-ld to 0.5.14)
 - rust-lang#142993 (Update cargo)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants