From 86158f581d20948507bef65e6162e3bbf5f4fa91 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Mon, 20 May 2024 01:09:29 -0400 Subject: [PATCH 1/5] Make repr(packed) vectors work with SIMD intrinsics --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 49 +++++++++++++++++++- tests/ui/simd/repr_packed.rs | 18 ++----- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index c0a1208a8c7d0..83a71752ffd8d 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -480,8 +480,55 @@ impl<'ll, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'_, 'll, 'tcx> { } _ if name.as_str().starts_with("simd_") => { + // Unpack non-power-of-2 #[repr(packed)] + let mut loaded_args = Vec::new(); + for (ty, arg) in arg_tys.iter().zip(args) { + loaded_args.push( + if ty.is_simd() + && let OperandValue::Ref(place) = arg.val + { + let (size, elem_ty) = ty.simd_size_and_type(self.tcx()); + let elem_ll_ty = match elem_ty.kind() { + ty::Float(f) => self.type_float_from_ty(*f), + ty::Int(i) => self.type_int_from_ty(*i), + ty::Uint(u) => self.type_uint_from_ty(*u), + ty::RawPtr(_, _) => self.type_ptr(), + _ => unreachable!(), + }; + let loaded = + self.load_from_place(self.type_vector(elem_ll_ty, size), place); + OperandRef::from_immediate_or_packed_pair(self, loaded, arg.layout) + } else { + *arg + }, + ); + } + + let llret_ty = if ret_ty.is_simd() + && let abi::Abi::Aggregate { .. } = self.layout_of(ret_ty).layout.abi + { + let (size, elem_ty) = ret_ty.simd_size_and_type(self.tcx()); + let elem_ll_ty = match elem_ty.kind() { + ty::Float(f) => self.type_float_from_ty(*f), + ty::Int(i) => self.type_int_from_ty(*i), + ty::Uint(u) => self.type_uint_from_ty(*u), + ty::RawPtr(_, _) => self.type_ptr(), + _ => unreachable!(), + }; + self.type_vector(elem_ll_ty, size) + } else { + llret_ty + }; + match generic_simd_intrinsic( - self, name, callee_ty, fn_args, args, ret_ty, llret_ty, span, + self, + name, + callee_ty, + fn_args, + &loaded_args, + ret_ty, + llret_ty, + span, ) { Ok(llval) => llval, Err(()) => return Ok(()), diff --git a/tests/ui/simd/repr_packed.rs b/tests/ui/simd/repr_packed.rs index 411bba3454ebe..52c794563de6d 100644 --- a/tests/ui/simd/repr_packed.rs +++ b/tests/ui/simd/repr_packed.rs @@ -6,9 +6,6 @@ #[repr(simd, packed)] struct Simd([T; N]); -#[repr(simd)] -struct FullSimd([T; N]); - fn check_size_align() { use std::mem; assert_eq!(mem::size_of::>(), mem::size_of::<[T; N]>()); @@ -44,16 +41,9 @@ fn main() { simd_add(Simd::([0., 1., 2., 3.]), Simd::([2., 2., 2., 2.])); assert_eq!(std::mem::transmute::<_, [f64; 4]>(x), [2., 3., 4., 5.]); - // non-powers-of-two have padding and need to be expanded to full vectors - fn load(v: Simd) -> FullSimd { - unsafe { - let mut tmp = core::mem::MaybeUninit::>::uninit(); - std::ptr::copy_nonoverlapping(&v as *const _, tmp.as_mut_ptr().cast(), 1); - tmp.assume_init() - } - } - let x: FullSimd = - simd_add(load(Simd::([0., 1., 2.])), load(Simd::([2., 2., 2.]))); - assert_eq!(x.0, [2., 3., 4.]); + // non-powers-of-two have padding and lesser alignment, but the intrinsic handles it + let x: Simd = simd_add(Simd::([0., 1., 2.]), Simd::([2., 2., 2.])); + let arr: [f64; 3] = x.0; + assert_eq!(arr, [2., 3., 4.]); } } From 190a96f9d347363ac9e3d3ac3d3821cf35e3b0a9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 1 Jun 2024 10:29:45 +0200 Subject: [PATCH 2/5] Migrate `run-make/emit-named-files` to `rmake.rs` --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/emit-named-files/Makefile | 33 ------------------- tests/run-make/emit-named-files/rmake.rs | 25 ++++++++++++++ 3 files changed, 25 insertions(+), 34 deletions(-) delete mode 100644 tests/run-make/emit-named-files/Makefile create mode 100644 tests/run-make/emit-named-files/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 009200aca15de..95fc30cb77571 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -38,7 +38,6 @@ run-make/dump-ice-to-disk/Makefile run-make/dump-mono-stats/Makefile run-make/duplicate-output-flavors/Makefile run-make/dylib-chain/Makefile -run-make/emit-named-files/Makefile run-make/emit-path-unhashed/Makefile run-make/emit-shared-files/Makefile run-make/emit-stack-sizes/Makefile diff --git a/tests/run-make/emit-named-files/Makefile b/tests/run-make/emit-named-files/Makefile deleted file mode 100644 index 2b97b841fc00d..0000000000000 --- a/tests/run-make/emit-named-files/Makefile +++ /dev/null @@ -1,33 +0,0 @@ -include ../tools.mk - -OUT=$(TMPDIR)/emit - -all: asm llvm-bc llvm-ir obj metadata link dep-info mir - -asm: $(OUT) - $(RUSTC) --emit asm=$(OUT)/libfoo.s foo.rs - test -f $(OUT)/libfoo.s -llvm-bc: $(OUT) - $(RUSTC) --emit llvm-bc=$(OUT)/libfoo.bc foo.rs - test -f $(OUT)/libfoo.bc -llvm-ir: $(OUT) - $(RUSTC) --emit llvm-ir=$(OUT)/libfoo.ll foo.rs - test -f $(OUT)/libfoo.ll -obj: $(OUT) - $(RUSTC) --emit obj=$(OUT)/libfoo.o foo.rs - test -f $(OUT)/libfoo.o -metadata: $(OUT) - $(RUSTC) --emit metadata=$(OUT)/libfoo.rmeta foo.rs - test -f $(OUT)/libfoo.rmeta -link: $(OUT) - $(RUSTC) --emit link=$(OUT)/libfoo.rlib foo.rs - test -f $(OUT)/libfoo.rlib -dep-info: $(OUT) - $(RUSTC) --emit dep-info=$(OUT)/libfoo.d foo.rs - test -f $(OUT)/libfoo.d -mir: $(OUT) - $(RUSTC) --emit mir=$(OUT)/libfoo.mir foo.rs - test -f $(OUT)/libfoo.mir - -$(OUT): - mkdir -p $(OUT) diff --git a/tests/run-make/emit-named-files/rmake.rs b/tests/run-make/emit-named-files/rmake.rs new file mode 100644 index 0000000000000..068f9796d0e9e --- /dev/null +++ b/tests/run-make/emit-named-files/rmake.rs @@ -0,0 +1,25 @@ +use std::fs::create_dir; +use std::path::Path; + +use run_make_support::{rustc, tmp_dir}; + +fn emit_and_check(out_dir: &Path, out_file: &str, format: &str) { + let out_file = out_dir.join(out_file); + rustc().input("foo.rs").emit(&format!("{format}={}", out_file.display())).run(); + assert!(out_file.is_file()); +} + +fn main() { + let out_dir = tmp_dir().join("emit"); + + create_dir(&out_dir).unwrap(); + + emit_and_check(&out_dir, "libfoo.s", "asm"); + emit_and_check(&out_dir, "libfoo.bc", "llvm-bc"); + emit_and_check(&out_dir, "libfoo.ll", "llvm-ir"); + emit_and_check(&out_dir, "libfoo.o", "obj"); + emit_and_check(&out_dir, "libfoo.rmeta", "metadata"); + emit_and_check(&out_dir, "libfoo.rlib", "link"); + emit_and_check(&out_dir, "libfoo.d", "dep-info"); + emit_and_check(&out_dir, "libfoo.mir", "mir"); +} From 9bdc5b2455bbd8d71e912b5ceaeb390abb987c91 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sat, 1 Jun 2024 14:17:16 -0400 Subject: [PATCH 3/5] Improve documentation --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 7 ++++++- tests/ui/simd/repr_packed.rs | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 83a71752ffd8d..87098566f6b1d 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -480,10 +480,15 @@ impl<'ll, 'tcx> IntrinsicCallMethods<'tcx> for Builder<'_, 'll, 'tcx> { } _ if name.as_str().starts_with("simd_") => { - // Unpack non-power-of-2 #[repr(packed)] + // Unpack non-power-of-2 #[repr(packed, simd)] arguments. + // This gives them the expected layout of a regular #[repr(simd)] vector. let mut loaded_args = Vec::new(); for (ty, arg) in arg_tys.iter().zip(args) { loaded_args.push( + // #[repr(packed, simd)] vectors are passed like arrays (as references, + // with reduced alignment and no padding) rather than as immediates. + // We can use a vector load to fix the layout and turn the argument + // into an immediate. if ty.is_simd() && let OperandValue::Ref(place) = arg.val { diff --git a/tests/ui/simd/repr_packed.rs b/tests/ui/simd/repr_packed.rs index 52c794563de6d..1ba15bda98dd2 100644 --- a/tests/ui/simd/repr_packed.rs +++ b/tests/ui/simd/repr_packed.rs @@ -36,12 +36,13 @@ fn main() { check_ty::(); unsafe { - // powers-of-two have no padding and work as usual + // powers-of-two have no padding and have the same layout as #[repr(simd)] let x: Simd = simd_add(Simd::([0., 1., 2., 3.]), Simd::([2., 2., 2., 2.])); assert_eq!(std::mem::transmute::<_, [f64; 4]>(x), [2., 3., 4., 5.]); - // non-powers-of-two have padding and lesser alignment, but the intrinsic handles it + // non-powers-of-two should have padding (which is removed by #[repr(packed)]), + // but the intrinsic handles it let x: Simd = simd_add(Simd::([0., 1., 2.]), Simd::([2., 2., 2.])); let arr: [f64; 3] = x.0; assert_eq!(arr, [2., 3., 4.]); From 11d6f18bf6a853ccae06371b67a43a41a4dd8af2 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 1 Jun 2024 02:36:55 -0700 Subject: [PATCH 4/5] Add some more specific checks to the MIR validator None of the `PointerCoercion`s had any, so while there's probably more that could be done here, hopefully these are better than the previous nothing. --- compiler/rustc_mir_transform/src/validate.rs | 101 +++++++++++++++++-- 1 file changed, 94 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs index 5e83a6f373aad..3b4d4c9387710 100644 --- a/compiler/rustc_mir_transform/src/validate.rs +++ b/compiler/rustc_mir_transform/src/validate.rs @@ -8,6 +8,7 @@ use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; +use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::{ self, CoroutineArgsExt, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitableExt, Variance, }; @@ -1134,9 +1135,76 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { // FIXME(dyn-star): make sure nothing needs to be done here. } // FIXME: Add Checks for these - CastKind::PointerWithExposedProvenance - | CastKind::PointerExposeProvenance - | CastKind::PointerCoercion(_) => {} + CastKind::PointerWithExposedProvenance | CastKind::PointerExposeProvenance => {} + CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => { + // FIXME: check signature compatibility. + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a fn item, not {:?}", + ty::FnDef(..) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a fn pointer, not {:?}", + ty::FnPtr(..) + ); + } + CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer) => { + // FIXME: check safety and signature compatibility. + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a fn pointer, not {:?}", + ty::FnPtr(..) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a fn pointer, not {:?}", + ty::FnPtr(..) + ); + } + CastKind::PointerCoercion(PointerCoercion::ClosureFnPointer(..)) => { + // FIXME: check safety, captures, and signature compatibility. + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a closure, not {:?}", + ty::Closure(..) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a fn pointer, not {:?}", + ty::FnPtr(..) + ); + } + CastKind::PointerCoercion(PointerCoercion::MutToConstPointer) => { + // FIXME: check same pointee? + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a raw mut pointer, not {:?}", + ty::RawPtr(_, Mutability::Mut) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a raw const pointer, not {:?}", + ty::RawPtr(_, Mutability::Not) + ); + } + CastKind::PointerCoercion(PointerCoercion::ArrayToPointer) => { + // FIXME: Check pointee types + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a raw pointer, not {:?}", + ty::RawPtr(..) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a raw pointer, not {:?}", + ty::RawPtr(..) + ); + } + CastKind::PointerCoercion(PointerCoercion::Unsize) => { + // This is used for all `CoerceUnsized` types, + // not just pointers/references, so is hard to check. + } CastKind::IntToInt | CastKind::IntToFloat => { let input_valid = op_ty.is_integral() || op_ty.is_char() || op_ty.is_bool(); let target_valid = target_type.is_numeric() || target_type.is_char(); @@ -1147,10 +1215,29 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ); } } - CastKind::FnPtrToPtr | CastKind::PtrToPtr => { - if !(op_ty.is_any_ptr() && target_type.is_unsafe_ptr()) { - self.fail(location, "Can't cast {op_ty} into 'Ptr'"); - } + CastKind::FnPtrToPtr => { + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a fn pointer, not {:?}", + ty::FnPtr(..) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a raw pointer, not {:?}", + ty::RawPtr(..) + ); + } + CastKind::PtrToPtr => { + check_kinds!( + op_ty, + "CastKind::{kind:?} input must be a raw pointer, not {:?}", + ty::RawPtr(..) + ); + check_kinds!( + target_type, + "CastKind::{kind:?} output must be a raw pointer, not {:?}", + ty::RawPtr(..) + ); } CastKind::FloatToFloat | CastKind::FloatToInt => { if !op_ty.is_floating_point() || !target_type.is_numeric() { From 5c32f84048f12b4cdbb404c0c543d38dec622d26 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 2 Jun 2024 01:03:51 -0700 Subject: [PATCH 5/5] Test codegen for repr(packed,simd) --- tests/codegen/simd/packed-simd-alignment.rs | 44 +++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/codegen/simd/packed-simd-alignment.rs diff --git a/tests/codegen/simd/packed-simd-alignment.rs b/tests/codegen/simd/packed-simd-alignment.rs new file mode 100644 index 0000000000000..53e88d8e5cf11 --- /dev/null +++ b/tests/codegen/simd/packed-simd-alignment.rs @@ -0,0 +1,44 @@ +//@ compile-flags: -Cno-prepopulate-passes + +#![crate_type = "lib"] +#![feature(repr_simd, core_intrinsics)] +// make sure that codegen emits correctly-aligned loads and stores for repr(packed, simd) types +// the alignment of a load should be no less than T, and no more than the size of the vector type +use std::intrinsics::simd as intrinsics; + +#[derive(Copy, Clone)] +#[repr(packed, simd)] +struct f32x3([f32; 3]); + +#[derive(Copy, Clone)] +#[repr(packed, simd)] +struct f32x4([f32; 4]); + +// CHECK-LABEL: load_f32x3 +#[no_mangle] +pub fn load_f32x3(floats: &f32x3) -> f32x3 { + // FIXME: Is a memcpy really the best we can do? + // CHECK: @llvm.memcpy.{{.*}}ptr align 4 {{.*}}ptr align 4 + *floats +} + +// CHECK-LABEL: load_f32x4 +#[no_mangle] +pub fn load_f32x4(floats: &f32x4) -> f32x4 { + // CHECK: load <4 x float>, ptr %{{[a-z0-9_]*}}, align {{4|8|16}} + *floats +} + +// CHECK-LABEL: add_f32x3 +#[no_mangle] +pub fn add_f32x3(x: f32x3, y: f32x3) -> f32x3 { + // CHECK: load <3 x float>, ptr %{{[a-z0-9_]*}}, align 4 + unsafe { intrinsics::simd_add(x, y) } +} + +// CHECK-LABEL: add_f32x4 +#[no_mangle] +pub fn add_f32x4(x: f32x4, y: f32x4) -> f32x4 { + // CHECK: load <4 x float>, ptr %{{[a-z0-9_]*}}, align {{4|8|16}} + unsafe { intrinsics::simd_add(x, y) } +}