Auto merge of #143833 - scottmcm:final-mcp-838, r=compiler-errors

Ban projecting into SIMD types [MCP838]

Closes rust-lang/compiler-team#838

The actual compiler change here is tiny; there's just a bazillion tests to update.

~~Since I'm sure I've missed some, for now~~
~~r ghost~~

try-job: test-various
try-job: x86_64-gnu-nopt

Author: bors

compiler/rustc_codegen_cranelift/example/float-minmax-pass.rs

@@ -11,6 +11,12 @@
 #[derive(Copy, Clone, PartialEq, Debug)]
 struct f32x4(pub [f32; 4]);
 
+impl f32x4 {
+    fn into_array(self) -> [f32; 4] {
+        unsafe { std::mem::transmute(self) }
+    }
+}
+
 use std::intrinsics::simd::*;
 
 fn main() {
@@ -29,22 +35,22 @@ fn main() {
     unsafe {
         let min0 = simd_fmin(x, y);
         let min1 = simd_fmin(y, x);
-        assert_eq!(min0, min1);
+        assert_eq!(min0.into_array(), min1.into_array());
         let e = f32x4([1.0, 1.0, 3.0, 3.0]);
-        assert_eq!(min0, e);
+        assert_eq!(min0.into_array(), e.into_array());
         let minn = simd_fmin(x, n);
-        assert_eq!(minn, x);
+        assert_eq!(minn.into_array(), x.into_array());
         let minn = simd_fmin(y, n);
-        assert_eq!(minn, y);
+        assert_eq!(minn.into_array(), y.into_array());
 
         let max0 = simd_fmax(x, y);
         let max1 = simd_fmax(y, x);
-        assert_eq!(max0, max1);
+        assert_eq!(max0.into_array(), max1.into_array());
         let e = f32x4([2.0, 2.0, 4.0, 4.0]);
-        assert_eq!(max0, e);
+        assert_eq!(max0.into_array(), e.into_array());
         let maxn = simd_fmax(x, n);
-        assert_eq!(maxn, x);
+        assert_eq!(maxn.into_array(), x.into_array());
         let maxn = simd_fmax(y, n);
-        assert_eq!(maxn, y);
+        assert_eq!(maxn.into_array(), y.into_array());
     }
 }

compiler/rustc_codegen_cranelift/example/mini_core_hello_world.rs

@@ -348,7 +348,8 @@ fn main() {
     struct V([f64; 2]);
 
     let f = V([0.0, 1.0]);
-    let _a = f.0[0];
+    let fp = (&raw const f) as *const [f64; 2];
+    let _a = (unsafe { &*fp })[0];
 
     stack_val_align();
 }

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -329,20 +329,11 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
         let offset = self.layout.fields.offset(i);
 
         if !bx.is_backend_ref(self.layout) && bx.is_backend_ref(field) {
-            if let BackendRepr::SimdVector { count, .. } = self.layout.backend_repr
-                && let BackendRepr::Memory { sized: true } = field.backend_repr
-                && count.is_power_of_two()
-            {
-                assert_eq!(field.size, self.layout.size);
-                // This is being deprecated, but for now stdarch still needs it for
-                // Newtype vector of array, e.g. #[repr(simd)] struct S([i32; 4]);
-                let place = PlaceRef::alloca(bx, field);
-                self.val.store(bx, place.val.with_type(self.layout));
-                return bx.load_operand(place);
-            } else {
-                // Part of https://github.com/rust-lang/compiler-team/issues/838
-                bug!("Non-ref type {self:?} cannot project to ref field type {field:?}");
-            }
+            // Part of https://github.com/rust-lang/compiler-team/issues/838
+            span_bug!(
+                fx.mir.span,
+                "Non-ref type {self:?} cannot project to ref field type {field:?}",
+            );
         }
 
         let val = if field.is_zst() {

compiler/rustc_mir_transform/src/validate.rs

@@ -721,6 +721,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                             );
                         }
 
+                        if adt_def.repr().simd() {
+                            self.fail(
+                                location,
+                                format!(
+                                    "Projecting into SIMD type {adt_def:?} is banned by MCP#838"
+                                ),
+                            );
+                        }
+
                         let var = parent_ty.variant_index.unwrap_or(FIRST_VARIANT);
                         let Some(field) = adt_def.variant(var).fields.get(f) else {
                             fail_out_of_bounds(self, location);

src/tools/miri/tests/pass/intrinsics/portable-simd.rs

@@ -349,12 +349,15 @@ fn simd_mask() {
     // Non-power-of-2 multi-byte mask.
     #[repr(simd, packed)]
     #[allow(non_camel_case_types)]
-    #[derive(Copy, Clone, Debug, PartialEq)]
+    #[derive(Copy, Clone)]
     struct i32x10([i32; 10]);
     impl i32x10 {
         fn splat(x: i32) -> Self {
             Self([x; 10])
         }
+        fn into_array(self) -> [i32; 10] {
+            unsafe { std::mem::transmute(self) }
+        }
     }
     unsafe {
         let mask = i32x10([!0, !0, 0, !0, 0, 0, !0, 0, !0, 0]);
@@ -377,19 +380,22 @@ fn simd_mask() {
             i32x10::splat(!0), // yes
             i32x10::splat(0),  // no
         );
-        assert_eq!(selected1, mask);
-        assert_eq!(selected2, mask);
+        assert_eq!(selected1.into_array(), mask.into_array());
+        assert_eq!(selected2.into_array(), mask.into_array());
     }
 
     // Test for a mask where the next multiple of 8 is not a power of two.
     #[repr(simd, packed)]
     #[allow(non_camel_case_types)]
-    #[derive(Copy, Clone, Debug, PartialEq)]
+    #[derive(Copy, Clone)]
     struct i32x20([i32; 20]);
     impl i32x20 {
         fn splat(x: i32) -> Self {
             Self([x; 20])
         }
+        fn into_array(self) -> [i32; 20] {
+            unsafe { std::mem::transmute(self) }
+        }
     }
     unsafe {
         let mask = i32x20([!0, !0, 0, !0, 0, 0, !0, 0, !0, 0, 0, 0, 0, !0, !0, !0, !0, !0, !0, !0]);
@@ -419,8 +425,8 @@ fn simd_mask() {
             i32x20::splat(!0), // yes
             i32x20::splat(0),  // no
         );
-        assert_eq!(selected1, mask);
-        assert_eq!(selected2, mask);
+        assert_eq!(selected1.into_array(), mask.into_array());
+        assert_eq!(selected2.into_array(), mask.into_array());
     }
 }
 
@@ -708,12 +714,12 @@ fn simd_ops_non_pow2() {
     let x = SimdPacked([1u32; 3]);
     let y = SimdPacked([2u32; 3]);
     let z = unsafe { intrinsics::simd_add(x, y) };
-    assert_eq!({ z.0 }, [3u32; 3]);
+    assert_eq!(unsafe { *(&raw const z).cast::<[u32; 3]>() }, [3u32; 3]);
 
     let x = SimdPadded([1u32; 3]);
     let y = SimdPadded([2u32; 3]);
     let z = unsafe { intrinsics::simd_add(x, y) };
-    assert_eq!(z.0, [3u32; 3]);
+    assert_eq!(unsafe { *(&raw const z).cast::<[u32; 3]>() }, [3u32; 3]);
 }
 
 fn main() {

tests/auxiliary/minisimd.rs

@@ -0,0 +1,160 @@
+//! Auxiliary crate for tests that need SIMD types.
+//!
+//! Historically the tests just made their own, but projections into simd types
+//! was banned by <https://github.com/rust-lang/compiler-team/issues/838>, which
+//! breaks `derive(Clone)`, so this exists to give easily-usable types that can
+//! be used without copy-pasting the definitions of the helpers everywhere.
+//!
+//! This makes no attempt to guard against ICEs.  Using it with proper types
+//! and such is your responsibility in the tests you write.
+
+#![allow(unused)]
+#![allow(non_camel_case_types)]
+
+// The field is currently left `pub` for convenience in porting tests, many of
+// which attempt to just construct it directly. That still works; it's just the
+// `.0` projection that doesn't.
+#[repr(simd)]
+#[derive(Copy, Eq)]
+pub struct Simd<T, const N: usize>(pub [T; N]);
+
+impl<T: Copy, const N: usize> Clone for Simd<T, N> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T: PartialEq, const N: usize> PartialEq for Simd<T, N> {
+    fn eq(&self, other: &Self) -> bool {
+        self.as_array() == other.as_array()
+    }
+}
+
+impl<T: core::fmt::Debug, const N: usize> core::fmt::Debug for Simd<T, N> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
+        <[T; N] as core::fmt::Debug>::fmt(self.as_array(), f)
+    }
+}
+
+impl<T, const N: usize> core::ops::Index<usize> for Simd<T, N> {
+    type Output = T;
+    fn index(&self, i: usize) -> &T {
+        &self.as_array()[i]
+    }
+}
+
+impl<T, const N: usize> Simd<T, N> {
+    pub const fn from_array(a: [T; N]) -> Self {
+        Simd(a)
+    }
+    pub fn as_array(&self) -> &[T; N] {
+        let p: *const Self = self;
+        unsafe { &*p.cast::<[T; N]>() }
+    }
+    pub fn into_array(self) -> [T; N]
+    where
+        T: Copy,
+    {
+        *self.as_array()
+    }
+}
+
+pub type u8x2 = Simd<u8, 2>;
+pub type u8x4 = Simd<u8, 4>;
+pub type u8x8 = Simd<u8, 8>;
+pub type u8x16 = Simd<u8, 16>;
+pub type u8x32 = Simd<u8, 32>;
+pub type u8x64 = Simd<u8, 64>;
+
+pub type u16x2 = Simd<u16, 2>;
+pub type u16x4 = Simd<u16, 4>;
+pub type u16x8 = Simd<u16, 8>;
+pub type u16x16 = Simd<u16, 16>;
+pub type u16x32 = Simd<u16, 32>;
+
+pub type u32x2 = Simd<u32, 2>;
+pub type u32x4 = Simd<u32, 4>;
+pub type u32x8 = Simd<u32, 8>;
+pub type u32x16 = Simd<u32, 16>;
+
+pub type u64x2 = Simd<u64, 2>;
+pub type u64x4 = Simd<u64, 4>;
+pub type u64x8 = Simd<u64, 8>;
+
+pub type u128x2 = Simd<u128, 2>;
+pub type u128x4 = Simd<u128, 4>;
+
+pub type i8x2 = Simd<i8, 2>;
+pub type i8x4 = Simd<i8, 4>;
+pub type i8x8 = Simd<i8, 8>;
+pub type i8x16 = Simd<i8, 16>;
+pub type i8x32 = Simd<i8, 32>;
+pub type i8x64 = Simd<i8, 64>;
+
+pub type i16x2 = Simd<i16, 2>;
+pub type i16x4 = Simd<i16, 4>;
+pub type i16x8 = Simd<i16, 8>;
+pub type i16x16 = Simd<i16, 16>;
+pub type i16x32 = Simd<i16, 32>;
+
+pub type i32x2 = Simd<i32, 2>;
+pub type i32x4 = Simd<i32, 4>;
+pub type i32x8 = Simd<i32, 8>;
+pub type i32x16 = Simd<i32, 16>;
+
+pub type i64x2 = Simd<i64, 2>;
+pub type i64x4 = Simd<i64, 4>;
+pub type i64x8 = Simd<i64, 8>;
+
+pub type i128x2 = Simd<i128, 2>;
+pub type i128x4 = Simd<i128, 4>;
+
+pub type f32x2 = Simd<f32, 2>;
+pub type f32x4 = Simd<f32, 4>;
+pub type f32x8 = Simd<f32, 8>;
+pub type f32x16 = Simd<f32, 16>;
+
+pub type f64x2 = Simd<f64, 2>;
+pub type f64x4 = Simd<f64, 4>;
+pub type f64x8 = Simd<f64, 8>;
+
+// The field is currently left `pub` for convenience in porting tests, many of
+// which attempt to just construct it directly. That still works; it's just the
+// `.0` projection that doesn't.
+#[repr(simd, packed)]
+#[derive(Copy)]
+pub struct PackedSimd<T, const N: usize>(pub [T; N]);
+
+impl<T: Copy, const N: usize> Clone for PackedSimd<T, N> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T: PartialEq, const N: usize> PartialEq for PackedSimd<T, N> {
+    fn eq(&self, other: &Self) -> bool {
+        self.as_array() == other.as_array()
+    }
+}
+
+impl<T: core::fmt::Debug, const N: usize> core::fmt::Debug for PackedSimd<T, N> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> {
+        <[T; N] as core::fmt::Debug>::fmt(self.as_array(), f)
+    }
+}
+
+impl<T, const N: usize> PackedSimd<T, N> {
+    pub const fn from_array(a: [T; N]) -> Self {
+        PackedSimd(a)
+    }
+    pub fn as_array(&self) -> &[T; N] {
+        let p: *const Self = self;
+        unsafe { &*p.cast::<[T; N]>() }
+    }
+    pub fn into_array(self) -> [T; N]
+    where
+        T: Copy,
+    {
+        *self.as_array()
+    }
+}

tests/codegen/const-vector.rs

@@ -16,18 +16,9 @@
 #![feature(mips_target_feature)]
 #![allow(non_camel_case_types)]
 
-// Setting up structs that can be used as const vectors
-#[repr(simd)]
-#[derive(Clone)]
-pub struct i8x2([i8; 2]);
-
-#[repr(simd)]
-#[derive(Clone)]
-pub struct f32x2([f32; 2]);
-
-#[repr(simd, packed)]
-#[derive(Copy, Clone)]
-pub struct Simd<T, const N: usize>([T; N]);
+#[path = "../auxiliary/minisimd.rs"]
+mod minisimd;
+use minisimd::{PackedSimd as Simd, f32x2, i8x2};
 
 // The following functions are required for the tests to ensure
 // that they are called with a const vector
@@ -45,7 +36,7 @@ extern "unadjusted" {
 
 // Ensure the packed variant of the simd struct does not become a const vector
 // if the size is not a power of 2
-// CHECK: %"Simd<i32, 3>" = type { [3 x i32] }
+// CHECK: %"minisimd::PackedSimd<i32, 3>" = type { [3 x i32] }
 
 #[cfg_attr(target_family = "wasm", target_feature(enable = "simd128"))]
 #[cfg_attr(target_arch = "arm", target_feature(enable = "neon"))]
@@ -54,27 +45,34 @@ extern "unadjusted" {
 pub fn do_call() {
     unsafe {
         // CHECK: call void @test_i8x2(<2 x i8> <i8 32, i8 64>
-        test_i8x2(const { i8x2([32, 64]) });
+        test_i8x2(const { i8x2::from_array([32, 64]) });
 
         // CHECK: call void @test_i8x2_two_args(<2 x i8> <i8 32, i8 64>, <2 x i8> <i8 8, i8 16>
-        test_i8x2_two_args(const { i8x2([32, 64]) }, const { i8x2([8, 16]) });
+        test_i8x2_two_args(
+            const { i8x2::from_array([32, 64]) },
+            const { i8x2::from_array([8, 16]) },
+        );
 
         // CHECK: call void @test_i8x2_mixed_args(<2 x i8> <i8 32, i8 64>, i32 43, <2 x i8> <i8 8, i8 16>
-        test_i8x2_mixed_args(const { i8x2([32, 64]) }, 43, const { i8x2([8, 16]) });
+        test_i8x2_mixed_args(
+            const { i8x2::from_array([32, 64]) },
+            43,
+            const { i8x2::from_array([8, 16]) },
+        );
 
         // CHECK: call void @test_i8x2_arr(<2 x i8> <i8 32, i8 64>
-        test_i8x2_arr(const { i8x2([32, 64]) });
+        test_i8x2_arr(const { i8x2::from_array([32, 64]) });
 
         // CHECK: call void @test_f32x2(<2 x float> <float 0x3FD47AE140000000, float 0x3FE47AE140000000>
-        test_f32x2(const { f32x2([0.32, 0.64]) });
+        test_f32x2(const { f32x2::from_array([0.32, 0.64]) });
 
         // CHECK: void @test_f32x2_arr(<2 x float> <float 0x3FD47AE140000000, float 0x3FE47AE140000000>
-        test_f32x2_arr(const { f32x2([0.32, 0.64]) });
+        test_f32x2_arr(const { f32x2::from_array([0.32, 0.64]) });
 
         // CHECK: call void @test_simd(<4 x i32> <i32 2, i32 4, i32 6, i32 8>
         test_simd(const { Simd::<i32, 4>([2, 4, 6, 8]) });
 
-        // CHECK: call void @test_simd_unaligned(%"Simd<i32, 3>" %1
+        // CHECK: call void @test_simd_unaligned(%"minisimd::PackedSimd<i32, 3>" %1
         test_simd_unaligned(const { Simd::<i32, 3>([2, 4, 6]) });
     }
 }