Skip to content

Ban projecting into SIMD types [MCP838] #143833

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions compiler/rustc_codegen_cranelift/example/float-minmax-pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
19 changes: 5 additions & 14 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_mir_transform/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,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);
Expand Down
22 changes: 14 additions & 8 deletions src/tools/miri/tests/pass/intrinsics/portable-simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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]);
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -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() {
Expand Down
160 changes: 160 additions & 0 deletions tests/auxiliary/minisimd.rs
Original file line number Diff line number Diff line change
@@ -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()
}
}
38 changes: 18 additions & 20 deletions tests/codegen/const-vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"))]
Expand All @@ -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]) });
}
}
Loading
Loading