Skip to content
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

Ban non-array SIMD #129403

Merged
merged 3 commits into from
Sep 11, 2024
Merged
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
12 changes: 6 additions & 6 deletions compiler/rustc_codegen_cranelift/example/float-minmax-pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@

#[repr(simd)]
#[derive(Copy, Clone, PartialEq, Debug)]
struct f32x4(pub f32, pub f32, pub f32, pub f32);
struct f32x4(pub [f32; 4]);

use std::intrinsics::simd::*;

fn main() {
let x = f32x4(1.0, 2.0, 3.0, 4.0);
let y = f32x4(2.0, 1.0, 4.0, 3.0);
let x = f32x4([1.0, 2.0, 3.0, 4.0]);
let y = f32x4([2.0, 1.0, 4.0, 3.0]);

#[cfg(not(any(target_arch = "mips", target_arch = "mips64")))]
let nan = f32::NAN;
Expand All @@ -24,13 +24,13 @@ fn main() {
#[cfg(any(target_arch = "mips", target_arch = "mips64"))]
let nan = f32::from_bits(f32::NAN.to_bits() - 1);

let n = f32x4(nan, nan, nan, nan);
let n = f32x4([nan, nan, nan, nan]);

unsafe {
let min0 = simd_fmin(x, y);
let min1 = simd_fmin(y, x);
assert_eq!(min0, min1);
let e = f32x4(1.0, 1.0, 3.0, 3.0);
let e = f32x4([1.0, 1.0, 3.0, 3.0]);
assert_eq!(min0, e);
let minn = simd_fmin(x, n);
assert_eq!(minn, x);
Expand All @@ -40,7 +40,7 @@ fn main() {
let max0 = simd_fmax(x, y);
let max1 = simd_fmax(y, x);
assert_eq!(max0, max1);
let e = f32x4(2.0, 2.0, 4.0, 4.0);
let e = f32x4([2.0, 2.0, 4.0, 4.0]);
assert_eq!(max0, e);
let maxn = simd_fmax(x, n);
assert_eq!(maxn, x);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/example/std_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ fn main() {
enum Never {}
}

foo(I64X2(0, 0));
foo(I64X2([0, 0]));

transmute_fat_pointer();

Expand Down Expand Up @@ -204,7 +204,7 @@ fn rust_call_abi() {
}

#[repr(simd)]
struct I64X2(i64, i64);
struct I64X2([i64; 2]);

#[allow(improper_ctypes_definitions)]
extern "C" fn foo(_a: I64X2) {}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0074.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ This will cause an error:
#![feature(repr_simd)]
#[repr(simd)]
struct Bad<T>(T, T, T, T);
struct Bad<T>([T; 4]);
```

This will not:
Expand All @@ -20,5 +20,5 @@ This will not:
#![feature(repr_simd)]
#[repr(simd)]
struct Good(u32, u32, u32, u32);
struct Good([u32; 4]);
```
18 changes: 12 additions & 6 deletions compiler/rustc_error_codes/src/error_codes/E0075.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
A `#[simd]` attribute was applied to an empty tuple struct.
A `#[simd]` attribute was applied to an empty or multi-field struct.

Erroneous code example:
Erroneous code examples:

```compile_fail,E0075
#![feature(repr_simd)]
Expand All @@ -9,15 +9,21 @@ Erroneous code example:
struct Bad; // error!
```

The `#[simd]` attribute can only be applied to non empty tuple structs, because
it doesn't make sense to try to use SIMD operations when there are no values to
operate on.
```compile_fail,E0075
#![feature(repr_simd)]
#[repr(simd)]
struct Bad([u32; 1], [u32; 1]); // error!
```

The `#[simd]` attribute can only be applied to a single-field struct, because
the one field must be the array of values in the vector.

Fixed example:

```
#![feature(repr_simd)]
#[repr(simd)]
struct Good(u32); // ok!
struct Good([u32; 2]); // ok!
```
10 changes: 5 additions & 5 deletions compiler/rustc_error_codes/src/error_codes/E0076.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
All types in a tuple struct aren't the same when using the `#[simd]`
The type of the field in a tuple struct isn't an array when using the `#[simd]`
attribute.

Erroneous code example:
Expand All @@ -7,18 +7,18 @@ Erroneous code example:
#![feature(repr_simd)]
#[repr(simd)]
struct Bad(u16, u32, u32 u32); // error!
struct Bad(u16); // error!
```

When using the `#[simd]` attribute to automatically use SIMD operations in tuple
struct, the types in the struct must all be of the same type, or the compiler
will trigger this error.
structs, if you want a single-lane vector then the field must be a 1-element
array, or the compiler will trigger this error.

Fixed example:

```
#![feature(repr_simd)]
#[repr(simd)]
struct Good(u32, u32, u32, u32); // ok!
struct Good([u16; 1]); // ok!
```
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0077.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Erroneous code example:
#![feature(repr_simd)]
#[repr(simd)]
struct Bad(String); // error!
struct Bad([String; 2]); // error!
```

When using the `#[simd]` attribute on a tuple struct, the elements in the tuple
Expand All @@ -19,5 +19,5 @@ Fixed example:
#![feature(repr_simd)]
#[repr(simd)]
struct Good(u32, u32, u32, u32); // ok!
struct Good([u32; 4]); // ok!
```
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0511.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ The generic type has to be a SIMD type. Example:
#[repr(simd)]
#[derive(Copy, Clone)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);
extern "rust-intrinsic" {
fn simd_add<T>(a: T, b: T) -> T;
}
unsafe { simd_add(i32x2(0, 0), i32x2(1, 2)); } // ok!
unsafe { simd_add(i32x2([0, 0]), i32x2([1, 2])); } // ok!
```
42 changes: 22 additions & 20 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,20 +1064,29 @@ fn check_simd(tcx: TyCtxt<'_>, sp: Span, def_id: LocalDefId) {
struct_span_code_err!(tcx.dcx(), sp, E0075, "SIMD vector cannot be empty").emit();
return;
}
let e = fields[FieldIdx::ZERO].ty(tcx, args);
if !fields.iter().all(|f| f.ty(tcx, args) == e) {
struct_span_code_err!(tcx.dcx(), sp, E0076, "SIMD vector should be homogeneous")
.with_span_label(sp, "SIMD elements must have the same type")

let array_field = &fields[FieldIdx::ZERO];
let array_ty = array_field.ty(tcx, args);
let ty::Array(element_ty, len_const) = array_ty.kind() else {
struct_span_code_err!(
tcx.dcx(),
sp,
E0076,
"SIMD vector's only field must be an array"
)
.with_span_label(tcx.def_span(array_field.did), "not an array")
.emit();
return;
};

if let Some(second_field) = fields.get(FieldIdx::from_u32(1)) {
struct_span_code_err!(tcx.dcx(), sp, E0075, "SIMD vector cannot have multiple fields")
.with_span_label(tcx.def_span(second_field.did), "excess field")
.emit();
return;
}

let len = if let ty::Array(_ty, c) = e.kind() {
c.try_eval_target_usize(tcx, tcx.param_env(def.did()))
} else {
Some(fields.len() as u64)
};
if let Some(len) = len {
if let Some(len) = len_const.try_eval_target_usize(tcx, tcx.param_env(def.did())) {
if len == 0 {
struct_span_code_err!(tcx.dcx(), sp, E0075, "SIMD vector cannot be empty").emit();
return;
Expand All @@ -1097,16 +1106,9 @@ fn check_simd(tcx: TyCtxt<'_>, sp: Span, def_id: LocalDefId) {
// These are scalar types which directly match a "machine" type
// Yes: Integers, floats, "thin" pointers
// No: char, "fat" pointers, compound types
match e.kind() {
ty::Param(_) => (), // pass struct<T>(T, T, T, T) through, let monomorphization catch errors
ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::RawPtr(_, _) => (), // struct(u8, u8, u8, u8) is ok
ty::Array(t, _) if matches!(t.kind(), ty::Param(_)) => (), // pass struct<T>([T; N]) through, let monomorphization catch errors
ty::Array(t, _clen)
if matches!(
t.kind(),
ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::RawPtr(_, _)
) =>
{ /* struct([f32; 4]) is ok */ }
match element_ty.kind() {
ty::Param(_) => (), // pass struct<T>([T; 4]) through, let monomorphization catch errors
ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::RawPtr(_, _) => (), // struct([u8; 4]) is ok
_ => {
struct_span_code_err!(
tcx.dcx(),
Expand Down
38 changes: 15 additions & 23 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,29 +1091,21 @@ impl<'tcx> Ty<'tcx> {
}

pub fn simd_size_and_type(self, tcx: TyCtxt<'tcx>) -> (u64, Ty<'tcx>) {
match self.kind() {
Adt(def, args) => {
assert!(def.repr().simd(), "`simd_size_and_type` called on non-SIMD type");
let variant = def.non_enum_variant();
let f0_ty = variant.fields[FieldIdx::ZERO].ty(tcx, args);

match f0_ty.kind() {
// If the first field is an array, we assume it is the only field and its
// elements are the SIMD components.
Array(f0_elem_ty, f0_len) => {
// FIXME(repr_simd): https://github.com/rust-lang/rust/pull/78863#discussion_r522784112
// The way we evaluate the `N` in `[T; N]` here only works since we use
// `simd_size_and_type` post-monomorphization. It will probably start to ICE
// if we use it in generic code. See the `simd-array-trait` ui test.
(f0_len.eval_target_usize(tcx, ParamEnv::empty()), *f0_elem_ty)
}
// Otherwise, the fields of this Adt are the SIMD components (and we assume they
// all have the same type).
_ => (variant.fields.len() as u64, f0_ty),
}
}
_ => bug!("`simd_size_and_type` called on invalid type"),
}
let Adt(def, args) = self.kind() else {
bug!("`simd_size_and_type` called on invalid type")
};
assert!(def.repr().simd(), "`simd_size_and_type` called on non-SIMD type");
let variant = def.non_enum_variant();
assert_eq!(variant.fields.len(), 1);
let field_ty = variant.fields[FieldIdx::ZERO].ty(tcx, args);
let Array(f0_elem_ty, f0_len) = field_ty.kind() else {
bug!("Simd type has non-array field type {field_ty:?}")
};
// FIXME(repr_simd): https://github.com/rust-lang/rust/pull/78863#discussion_r522784112
// The way we evaluate the `N` in `[T; N]` here only works since we use
// `simd_size_and_type` post-monomorphization. It will probably start to ICE
// if we use it in generic code. See the `simd-array-trait` ui test.
(f0_len.eval_target_usize(tcx, ParamEnv::empty()), *f0_elem_ty)
}

#[inline]
Expand Down
4 changes: 2 additions & 2 deletions library/alloc/benches/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,10 @@ reverse!(reverse_u32, u32, |x| x as u32);
reverse!(reverse_u64, u64, |x| x as u64);
reverse!(reverse_u128, u128, |x| x as u128);
#[repr(simd)]
struct F64x4(f64, f64, f64, f64);
struct F64x4([f64; 4]);
reverse!(reverse_simd_f64x4, F64x4, |x| {
let x = x as f64;
F64x4(x, x, x, x)
F64x4([x, x, x, x])
});

macro_rules! rotate {
Expand Down
6 changes: 3 additions & 3 deletions src/tools/miri/tests/fail/intrinsics/simd-div-by-zero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use std::intrinsics::simd::simd_div;

#[repr(simd)]
#[allow(non_camel_case_types)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(1, 1);
let y = i32x2(1, 0);
let x = i32x2([1, 1]);
let y = i32x2([1, 0]);
simd_div(x, y); //~ERROR: Undefined Behavior: dividing by zero
}
}
6 changes: 3 additions & 3 deletions src/tools/miri/tests/fail/intrinsics/simd-div-overflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use std::intrinsics::simd::simd_div;

#[repr(simd)]
#[allow(non_camel_case_types)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(1, i32::MIN);
let y = i32x2(1, -1);
let x = i32x2([1, i32::MIN]);
let y = i32x2([1, -1]);
simd_div(x, y); //~ERROR: Undefined Behavior: overflow in signed division
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ use std::intrinsics::simd::simd_reduce_any;

#[repr(simd)]
#[allow(non_camel_case_types)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(0, 1);
let x = i32x2([0, 1]);
simd_reduce_any(x); //~ERROR: must be all-0-bits or all-1-bits
}
}
6 changes: 3 additions & 3 deletions src/tools/miri/tests/fail/intrinsics/simd-rem-by-zero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use std::intrinsics::simd::simd_rem;

#[repr(simd)]
#[allow(non_camel_case_types)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(1, 1);
let y = i32x2(1, 0);
let x = i32x2([1, 1]);
let y = i32x2([1, 0]);
simd_rem(x, y); //~ERROR: Undefined Behavior: calculating the remainder with a divisor of zero
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use std::intrinsics::simd::simd_select_bitmask;
#[repr(simd)]
#[allow(non_camel_case_types)]
#[derive(Copy, Clone)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(0, 1);
let x = i32x2([0, 1]);
simd_select_bitmask(0b11111111u8, x, x); //~ERROR: bitmask less than 8 bits long must be filled with 0s for the remaining bits
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use std::intrinsics::simd::simd_select;
#[repr(simd)]
#[allow(non_camel_case_types)]
#[derive(Copy, Clone)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(0, 1);
let x = i32x2([0, 1]);
simd_select(x, x, x); //~ERROR: must be all-0-bits or all-1-bits
}
}
6 changes: 3 additions & 3 deletions src/tools/miri/tests/fail/intrinsics/simd-shl-too-far.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ use std::intrinsics::simd::simd_shl;

#[repr(simd)]
#[allow(non_camel_case_types)]
struct i32x2(i32, i32);
struct i32x2([i32; 2]);

fn main() {
unsafe {
let x = i32x2(1, 1);
let y = i32x2(100, 0);
let x = i32x2([1, 1]);
let y = i32x2([100, 0]);
simd_shl(x, y); //~ERROR: overflowing shift by 100 in `simd_shl` in lane 0
}
}
Loading
Loading