Skip to content

Commit 8ed3690

Browse files
authored
Unrolled build for #143410
Rollup merge of #143410 - scottmcm:redo-transmute-again, r=RalfJung,workingjubilee Block SIMD in transmute_immediate; delete `OperandValueKind` Vectors have been causing me problems for years in this code, for example #110021 (comment) and #143194 See conversation in <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/near/526262799>. By blocking SIMD in `transmute_immediate` it can be simplified to just take the `Scalar`s involved -- the backend types can be gotten from those `Scalar`s, rather than needing to be passed. And there's an assert added to ICE it if it does get hit. Accordingly, this changes `rvalue_creates_operand` to not send SIMD transmutes through the operand path, but to always go through memory instead, like they did back before #108442. And thanks to those changes, I could also remove the `OperandValueKind` type that I added back then which `@RalfJung` rightly considers pretty sketchy. cc `@folkertdev` `@workingjubilee` from the zulip conversation too
2 parents f0b67dd + 4e61527 commit 8ed3690

File tree

7 files changed

+138
-214
lines changed

7 files changed

+138
-214
lines changed

compiler/rustc_codegen_ssa/src/mir/operand.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_session::config::OptLevel;
1313
use tracing::{debug, instrument};
1414

1515
use super::place::{PlaceRef, PlaceValue};
16-
use super::rvalue::transmute_immediate;
16+
use super::rvalue::transmute_scalar;
1717
use super::{FunctionCx, LocalRef};
1818
use crate::common::IntPredicate;
1919
use crate::traits::*;
@@ -346,14 +346,16 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
346346

347347
let val = if field.is_zst() {
348348
OperandValue::ZeroSized
349+
} else if let BackendRepr::SimdVector { .. } = self.layout.backend_repr {
350+
// codegen_transmute_operand doesn't support SIMD, but since the previous
351+
// check handled ZSTs, the only possible field access into something SIMD
352+
// is to the `non_1zst_field` that's the same SIMD. (Other things, even
353+
// just padding, would change the wrapper's representation type.)
354+
assert_eq!(field.size, self.layout.size);
355+
self.val
349356
} else if field.size == self.layout.size {
350357
assert_eq!(offset.bytes(), 0);
351-
fx.codegen_transmute_operand(bx, *self, field).unwrap_or_else(|| {
352-
bug!(
353-
"Expected `codegen_transmute_operand` to handle equal-size \
354-
field {i:?} projection from {self:?} to {field:?}"
355-
)
356-
})
358+
fx.codegen_transmute_operand(bx, *self, field)
357359
} else {
358360
let (in_scalar, imm) = match (self.val, self.layout.backend_repr) {
359361
// Extract a scalar component from a pair.
@@ -613,10 +615,8 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, Result<V, abi::Scalar>> {
613615
};
614616

615617
let mut update = |tgt: &mut Result<V, abi::Scalar>, src, from_scalar| {
616-
let from_bty = bx.cx().type_from_scalar(from_scalar);
617618
let to_scalar = tgt.unwrap_err();
618-
let to_bty = bx.cx().type_from_scalar(to_scalar);
619-
let imm = transmute_immediate(bx, src, from_scalar, from_bty, to_scalar, to_bty);
619+
let imm = transmute_scalar(bx, src, from_scalar, to_scalar);
620620
*tgt = Ok(imm);
621621
};
622622

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

Lines changed: 101 additions & 155 deletions
Large diffs are not rendered by default.

tests/codegen/intrinsics/transmute-x64.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,20 @@ use std::mem::transmute;
99
// CHECK-LABEL: @check_sse_pair_to_avx(
1010
#[no_mangle]
1111
pub unsafe fn check_sse_pair_to_avx(x: (__m128i, __m128i)) -> __m256i {
12+
// CHECK: start:
1213
// CHECK-NOT: alloca
13-
// CHECK: %0 = load <4 x i64>, ptr %x, align 16
14-
// CHECK: store <4 x i64> %0, ptr %_0, align 32
14+
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 32 %_0, ptr align 16 %x, i64 32, i1 false)
15+
// CHECK-NEXT: ret void
1516
transmute(x)
1617
}
1718

1819
// CHECK-LABEL: @check_sse_pair_from_avx(
1920
#[no_mangle]
2021
pub unsafe fn check_sse_pair_from_avx(x: __m256i) -> (__m128i, __m128i) {
22+
// CHECK: start:
2123
// CHECK-NOT: alloca
22-
// CHECK: %0 = load <4 x i64>, ptr %x, align 32
23-
// CHECK: store <4 x i64> %0, ptr %_0, align 16
24+
// CHECK-NEXT: %[[TEMP:.+]] = load <4 x i64>, ptr %x, align 32
25+
// CHECK-NEXT: store <4 x i64> %[[TEMP]], ptr %_0, align 16
26+
// CHECK-NEXT: ret void
2427
transmute(x)
2528
}

tests/codegen/intrinsics/transmute.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,28 @@ pub struct Aggregate8(u8);
2929
// CHECK-LABEL: @check_bigger_size(
3030
#[no_mangle]
3131
pub unsafe fn check_bigger_size(x: u16) -> u32 {
32-
// CHECK: call void @llvm.trap
32+
// CHECK: call void @llvm.assume(i1 false)
3333
transmute_unchecked(x)
3434
}
3535

3636
// CHECK-LABEL: @check_smaller_size(
3737
#[no_mangle]
3838
pub unsafe fn check_smaller_size(x: u32) -> u16 {
39-
// CHECK: call void @llvm.trap
39+
// CHECK: call void @llvm.assume(i1 false)
4040
transmute_unchecked(x)
4141
}
4242

4343
// CHECK-LABEL: @check_smaller_array(
4444
#[no_mangle]
4545
pub unsafe fn check_smaller_array(x: [u32; 7]) -> [u32; 3] {
46-
// CHECK: call void @llvm.trap
46+
// CHECK: call void @llvm.assume(i1 false)
4747
transmute_unchecked(x)
4848
}
4949

5050
// CHECK-LABEL: @check_bigger_array(
5151
#[no_mangle]
5252
pub unsafe fn check_bigger_array(x: [u32; 3]) -> [u32; 7] {
53-
// CHECK: call void @llvm.trap
53+
// CHECK: call void @llvm.assume(i1 false)
5454
transmute_unchecked(x)
5555
}
5656

@@ -73,9 +73,9 @@ pub unsafe fn check_to_empty_array(x: [u32; 5]) -> [u32; 0] {
7373
#[no_mangle]
7474
#[custom_mir(dialect = "runtime", phase = "optimized")]
7575
pub unsafe fn check_from_empty_array(x: [u32; 0]) -> [u32; 5] {
76-
// CHECK-NOT: trap
77-
// CHECK: call void @llvm.trap
78-
// CHECK-NOT: trap
76+
// CHECK-NOT: call
77+
// CHECK: call void @llvm.assume(i1 false)
78+
// CHECK-NOT: call
7979
mir! {
8080
{
8181
RET = CastTransmute(x);

tests/codegen/simd-intrinsic/simd-intrinsic-transmute-array.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ pub fn build_array_s(x: [f32; 4]) -> S<4> {
4040
// CHECK-LABEL: @build_array_transmute_s
4141
#[no_mangle]
4242
pub fn build_array_transmute_s(x: [f32; 4]) -> S<4> {
43-
// CHECK: %[[VAL:.+]] = load <4 x float>, ptr %x, align [[ARRAY_ALIGN]]
44-
// CHECK: store <4 x float> %[[VAL:.+]], ptr %_0, align [[VECTOR_ALIGN]]
43+
// CHECK: call void @llvm.memcpy.{{.+}}({{.*}} align [[VECTOR_ALIGN]] {{.*}} align [[ARRAY_ALIGN]] {{.*}}, [[USIZE]] 16, i1 false)
4544
unsafe { std::mem::transmute(x) }
4645
}
4746

@@ -55,7 +54,6 @@ pub fn build_array_t(x: [f32; 4]) -> T {
5554
// CHECK-LABEL: @build_array_transmute_t
5655
#[no_mangle]
5756
pub fn build_array_transmute_t(x: [f32; 4]) -> T {
58-
// CHECK: %[[VAL:.+]] = load <4 x float>, ptr %x, align [[ARRAY_ALIGN]]
59-
// CHECK: store <4 x float> %[[VAL:.+]], ptr %_0, align [[VECTOR_ALIGN]]
57+
// CHECK: call void @llvm.memcpy.{{.+}}({{.*}} align [[VECTOR_ALIGN]] {{.*}} align [[ARRAY_ALIGN]] {{.*}}, [[USIZE]] 16, i1 false)
6058
unsafe { std::mem::transmute(x) }
6159
}

tests/codegen/transmute-scalar.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,11 @@ pub fn fake_bool_unsigned_to_bool(b: FakeBoolUnsigned) -> bool {
111111
struct S([i64; 1]);
112112

113113
// CHECK-LABEL: define{{.*}}i64 @single_element_simd_to_scalar(<1 x i64> %b)
114-
// CHECK: bitcast <1 x i64> %b to i64
115-
// CHECK: ret i64
114+
// CHECK-NEXT: start:
115+
// CHECK-NEXT: %[[RET:.+]] = alloca [8 x i8]
116+
// CHECK-NEXT: store <1 x i64> %b, ptr %[[RET]]
117+
// CHECK-NEXT: %[[TEMP:.+]] = load i64, ptr %[[RET]]
118+
// CHECK-NEXT: ret i64 %[[TEMP]]
116119
#[no_mangle]
117120
#[cfg_attr(target_family = "wasm", target_feature(enable = "simd128"))]
118121
#[cfg_attr(target_arch = "arm", target_feature(enable = "neon"))]
@@ -124,8 +127,11 @@ pub extern "C" fn single_element_simd_to_scalar(b: S) -> i64 {
124127
}
125128

126129
// CHECK-LABEL: define{{.*}}<1 x i64> @scalar_to_single_element_simd(i64 %b)
127-
// CHECK: bitcast i64 %b to <1 x i64>
128-
// CHECK: ret <1 x i64>
130+
// CHECK-NEXT: start:
131+
// CHECK-NEXT: %[[RET:.+]] = alloca [8 x i8]
132+
// CHECK-NEXT: store i64 %b, ptr %[[RET]]
133+
// CHECK-NEXT: %[[TEMP:.+]] = load <1 x i64>, ptr %[[RET]]
134+
// CHECK-NEXT: ret <1 x i64> %[[TEMP]]
129135
#[no_mangle]
130136
#[cfg_attr(target_family = "wasm", target_feature(enable = "simd128"))]
131137
#[cfg_attr(target_arch = "arm", target_feature(enable = "neon"))]

tests/codegen/vec-in-place.rs

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ pub fn vec_iterator_cast_primitive(vec: Vec<i8>) -> Vec<u8> {
4141
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
4242
// CHECK-NOT: loop
4343
// CHECK-NOT: call
44-
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
45-
// CHECK-NOT: loop
46-
// CHECK-NOT: call
4744
vec.into_iter().map(|e| e as u8).collect()
4845
}
4946

@@ -55,9 +52,6 @@ pub fn vec_iterator_cast_wrapper(vec: Vec<u8>) -> Vec<Wrapper<u8>> {
5552
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
5653
// CHECK-NOT: loop
5754
// CHECK-NOT: call
58-
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
59-
// CHECK-NOT: loop
60-
// CHECK-NOT: call
6155
vec.into_iter().map(|e| Wrapper(e)).collect()
6256
}
6357

@@ -86,9 +80,6 @@ pub fn vec_iterator_cast_unwrap(vec: Vec<Wrapper<u8>>) -> Vec<u8> {
8680
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
8781
// CHECK-NOT: loop
8882
// CHECK-NOT: call
89-
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
90-
// CHECK-NOT: loop
91-
// CHECK-NOT: call
9283
vec.into_iter().map(|e| e.0).collect()
9384
}
9485

@@ -100,9 +91,6 @@ pub fn vec_iterator_cast_aggregate(vec: Vec<[u64; 4]>) -> Vec<Foo> {
10091
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
10192
// CHECK-NOT: loop
10293
// CHECK-NOT: call
103-
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
104-
// CHECK-NOT: loop
105-
// CHECK-NOT: call
10694
vec.into_iter().map(|e| unsafe { std::mem::transmute(e) }).collect()
10795
}
10896

@@ -114,9 +102,6 @@ pub fn vec_iterator_cast_deaggregate_tra(vec: Vec<Bar>) -> Vec<[u64; 4]> {
114102
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
115103
// CHECK-NOT: loop
116104
// CHECK-NOT: call
117-
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
118-
// CHECK-NOT: loop
119-
// CHECK-NOT: call
120105

121106
// Safety: For the purpose of this test we assume that Bar layout matches [u64; 4].
122107
// This currently is not guaranteed for repr(Rust) types, but it happens to work here and
@@ -133,9 +118,6 @@ pub fn vec_iterator_cast_deaggregate_fold(vec: Vec<Baz>) -> Vec<[u64; 4]> {
133118
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
134119
// CHECK-NOT: loop
135120
// CHECK-NOT: call
136-
// CHECK: call{{.+}}void @llvm.assume(i1 %{{.+}})
137-
// CHECK-NOT: loop
138-
// CHECK-NOT: call
139121

140122
// Safety: For the purpose of this test we assume that Bar layout matches [u64; 4].
141123
// This currently is not guaranteed for repr(Rust) types, but it happens to work here and
@@ -156,12 +138,7 @@ pub fn vec_iterator_cast_unwrap_drop(vec: Vec<Wrapper<String>>) -> Vec<String> {
156138
// CHECK-NOT: call
157139
// CHECK-NOT: %{{.*}} = mul
158140
// CHECK-NOT: %{{.*}} = udiv
159-
// CHECK: call
160-
// CHECK-SAME: void @llvm.assume(i1 %{{.+}})
161-
// CHECK-NOT: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
162-
// CHECK-NOT: call
163-
// CHECK-NOT: %{{.*}} = mul
164-
// CHECK-NOT: %{{.*}} = udiv
141+
// CHECK: ret void
165142

166143
vec.into_iter().map(|Wrapper(e)| e).collect()
167144
}
@@ -178,12 +155,6 @@ pub fn vec_iterator_cast_wrap_drop(vec: Vec<String>) -> Vec<Wrapper<String>> {
178155
// CHECK-NOT: call
179156
// CHECK-NOT: %{{.*}} = mul
180157
// CHECK-NOT: %{{.*}} = udiv
181-
// CHECK: call
182-
// CHECK-SAME: void @llvm.assume(i1 %{{.+}})
183-
// CHECK-NOT: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
184-
// CHECK-NOT: call
185-
// CHECK-NOT: %{{.*}} = mul
186-
// CHECK-NOT: %{{.*}} = udiv
187158
// CHECK: ret void
188159

189160
vec.into_iter().map(Wrapper).collect()

0 commit comments

Comments
 (0)