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

fix CTFE/Miri simd_insert/extract on array-style repr(simd) types #90999

Merged
merged 2 commits into from
Nov 20, 2021
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
47 changes: 16 additions & 31 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,48 +413,33 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
sym::simd_insert => {
let index = u64::from(self.read_scalar(&args[1])?.to_u32()?);
let elem = &args[2];
let input = &args[0];
let (len, e_ty) = input.layout.ty.simd_size_and_type(*self.tcx);
let (input, input_len) = self.operand_to_simd(&args[0])?;
let (dest, dest_len) = self.place_to_simd(dest)?;
assert_eq!(input_len, dest_len, "Return vector length must match input length");
assert!(
index < len,
"Index `{}` must be in bounds of vector type `{}`: `[0, {})`",
index < dest_len,
"Index `{}` must be in bounds of vector with length {}`",
index,
e_ty,
len
);
assert_eq!(
input.layout, dest.layout,
"Return type `{}` must match vector type `{}`",
dest.layout.ty, input.layout.ty
);
assert_eq!(
elem.layout.ty, e_ty,
"Scalar element type `{}` must match vector element type `{}`",
elem.layout.ty, e_ty
dest_len
);

for i in 0..len {
let place = self.place_index(dest, i)?;
let value = if i == index { *elem } else { self.operand_index(input, i)? };
self.copy_op(&value, &place)?;
for i in 0..dest_len {
let place = self.mplace_index(&dest, i)?;
let value =
if i == index { *elem } else { self.mplace_index(&input, i)?.into() };
self.copy_op(&value, &place.into())?;
Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW, this will assert that the types are equal, thus taking care of the remaining assertions that I removed.

}
}
sym::simd_extract => {
let index = u64::from(self.read_scalar(&args[1])?.to_u32()?);
let (len, e_ty) = args[0].layout.ty.simd_size_and_type(*self.tcx);
let (input, input_len) = self.operand_to_simd(&args[0])?;
assert!(
index < len,
"index `{}` is out-of-bounds of vector type `{}` with length `{}`",
index < input_len,
"index `{}` must be in bounds of vector with length `{}`",
index,
e_ty,
len
);
assert_eq!(
e_ty, dest.layout.ty,
"Return type `{}` must match vector element type `{}`",
dest.layout.ty, e_ty
input_len
);
self.copy_op(&self.operand_index(&args[0], index)?, dest)?;
self.copy_op(&self.mplace_index(&input, index)?.into(), dest)?;
}
sym::likely | sym::unlikely | sym::black_box => {
// These just return their argument
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
})
}

/// Converts a repr(simd) operand into an operand where `place_index` accesses the SIMD elements.
/// Also returns the number of elements.
pub fn operand_to_simd(
&self,
base: &OpTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, (MPlaceTy<'tcx, M::PointerTag>, u64)> {
// Basically we just transmute this place into an array following simd_size_and_type.
// This only works in memory, but repr(simd) types should never be immediates anyway.
assert!(base.layout.ty.is_simd());
self.mplace_to_simd(&base.assert_mem_place())
}

/// Read from a local. Will not actually access the local if reading from a ZST.
/// Will not access memory, instead an indirect `Operand` is returned.
///
Expand Down
28 changes: 27 additions & 1 deletion compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> {
}
} else {
// Go through the layout. There are lots of types that support a length,
// e.g., SIMD types.
// e.g., SIMD types. (But not all repr(simd) types even have FieldsShape::Array!)
match self.layout.fields {
FieldsShape::Array { count, .. } => Ok(count),
_ => bug!("len not supported on sized type {:?}", self.layout.ty),
Expand Down Expand Up @@ -533,6 +533,22 @@ where
})
}

/// Converts a repr(simd) place into a place where `place_index` accesses the SIMD elements.
/// Also returns the number of elements.
pub fn mplace_to_simd(
&self,
base: &MPlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, (MPlaceTy<'tcx, M::PointerTag>, u64)> {
// Basically we just transmute this place into an array following simd_size_and_type.
// (Transmuting is okay since this is an in-memory place. We also double-check the size
// stays the same.)
let (len, e_ty) = base.layout.ty.simd_size_and_type(*self.tcx);
let array = self.tcx.mk_array(e_ty, len);
let layout = self.layout_of(array)?;
assert_eq!(layout.size, base.layout.size);
Ok((MPlaceTy { layout, ..*base }, len))
}

/// Gets the place of a field inside the place, and also the field's type.
/// Just a convenience function, but used quite a bit.
/// This is the only projection that might have a side-effect: We cannot project
Expand Down Expand Up @@ -594,6 +610,16 @@ where
})
}

/// Converts a repr(simd) place into a place where `place_index` accesses the SIMD elements.
/// Also returns the number of elements.
pub fn place_to_simd(
&mut self,
base: &PlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, (MPlaceTy<'tcx, M::PointerTag>, u64)> {
let mplace = self.force_allocation(base)?;
self.mplace_to_simd(&mplace)
}

/// Computes a place. You should only use this if you intend to write into this
/// place; for reading, a more efficient alternative is `eval_place_for_read`.
pub fn eval_place(
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1805,17 +1805,22 @@ impl<'tcx> TyS<'tcx> {
pub fn simd_size_and_type(&self, tcx: TyCtxt<'tcx>) -> (u64, Ty<'tcx>) {
match self.kind() {
Adt(def, substs) => {
assert!(def.repr.simd(), "`simd_size_and_type` called on non-SIMD type");
let variant = def.non_enum_variant();
let f0_ty = variant.fields[0].ty(tcx, substs);

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_usize(tcx, ParamEnv::empty()) as u64, 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),
}
}
Expand Down
22 changes: 16 additions & 6 deletions src/test/ui/consts/const-eval/simd/insert_extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

#[repr(simd)] struct i8x1(i8);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you check whether #[repr(simd)] struct i8x1([i8; 1]); won't end up with an immediate? Not that this type makes much sense, but probably good to be exhaustive.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I have added that case to the test as well.

#[repr(simd)] struct u16x2(u16, u16);
#[repr(simd)] struct f32x4(f32, f32, f32, f32);
// Make some of them array types to ensure those also work.
#[repr(simd)] struct i8x1_arr([i8; 1]);
#[repr(simd)] struct f32x4([f32; 4]);

extern "platform-intrinsic" {
#[rustc_const_stable(feature = "foo", since = "1.3.37")]
Expand All @@ -25,6 +27,14 @@ fn main() {
assert_eq!(X0, 42);
assert_eq!(Y0, 42);
}
{
const U: i8x1_arr = i8x1_arr([13]);
const V: i8x1_arr = unsafe { simd_insert(U, 0_u32, 42_i8) };
const X0: i8 = V.0[0];
const Y0: i8 = unsafe { simd_extract(V, 0) };
assert_eq!(X0, 42);
assert_eq!(Y0, 42);
}
{
const U: u16x2 = u16x2(13, 14);
const V: u16x2 = unsafe { simd_insert(U, 1_u32, 42_u16) };
Expand All @@ -38,12 +48,12 @@ fn main() {
assert_eq!(Y1, 42);
}
{
const U: f32x4 = f32x4(13., 14., 15., 16.);
const U: f32x4 = f32x4([13., 14., 15., 16.]);
const V: f32x4 = unsafe { simd_insert(U, 1_u32, 42_f32) };
const X0: f32 = V.0;
const X1: f32 = V.1;
const X2: f32 = V.2;
const X3: f32 = V.3;
const X0: f32 = V.0[0];
const X1: f32 = V.0[1];
const X2: f32 = V.0[2];
const X3: f32 = V.0[3];
const Y0: f32 = unsafe { simd_extract(V, 0) };
const Y1: f32 = unsafe { simd_extract(V, 1) };
const Y2: f32 = unsafe { simd_extract(V, 2) };
Expand Down