From a371ac9d6c7677e685f94e621382508fcb7400a1 Mon Sep 17 00:00:00 2001 From: Jamie Cunliffe Date: Fri, 26 Apr 2024 15:32:59 +0100 Subject: [PATCH] Fix up style review comments. Add some comments explaining some not so obvious things. Mark code as unreachable on architectures that don't currently have scalable vectors. Remove some unused (and incorrect) checks that were being performed. Refactor some code to improve it's readability. --- compiler/rustc_codegen_llvm/src/abi.rs | 4 ++ compiler/rustc_codegen_llvm/src/intrinsic.rs | 1 + compiler/rustc_middle/src/ty/sty.rs | 37 ++++++++++--------- compiler/rustc_target/src/abi/call/arm.rs | 2 +- .../rustc_target/src/abi/call/loongarch.rs | 3 +- .../rustc_target/src/abi/call/powerpc64.rs | 2 +- .../rustc_target/src/abi/call/x86_win64.rs | 2 +- compiler/rustc_ty_utils/src/layout.rs | 6 --- 8 files changed, 30 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 813d9d0796109..9d4e4984b2502 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -123,6 +123,10 @@ impl LlvmType for Reg { _ => bug!("unsupported float: {:?}", self), }, RegKind::Vector => cx.type_vector(cx.type_i8(), self.size.bytes()), + // Generate a LLVM type such as , like above for a non scalable + // vector. The use of 16 here is chosen as that will generate a valid type with both + // Arm SVE and RISC-V RVV. In the future with other architectures this might not be + // valid and might have to be configured by the target. RegKind::ScalableVector => cx.type_scalable_vector(cx.type_i8(), 16), } } diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 5a3ebd47a7fd5..560baad917a25 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -1316,6 +1316,7 @@ fn generic_simd_intrinsic<'ll, 'tcx>( InvalidMonomorphization::MismatchedLengths { span, name, m_len, v_len } ); match m_elem_ty.kind() { + // Arm SVE has a svbool type and we need to represent that as a bool in the type system. ty::Int(_) | ty::Bool => {} _ => return_error!(InvalidMonomorphization::MaskType { span, name, ty: m_elem_ty }), } diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 5b74f593bc499..9a9487b309d51 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1964,25 +1964,28 @@ impl<'tcx> Ty<'tcx> { let variant = def.non_enum_variant(); let f0_ty = variant.fields[FieldIdx::from_u32(0)].ty(tcx, args); - match f0_ty.kind() { - Array(_, _) if def.repr().scalable() => { - bug!("Scalable SIMD should be using a slice, not array"); + if def.repr().scalable() { + match f0_ty.kind() { + Slice(f0_elem_ty) => (def.repr().scalable.unwrap_or(0) as u64, *f0_elem_ty), + _ => { + bug!("Scalable SIMD should be using a slice"); + } } - Slice(f0_elem_ty) if def.repr().scalable() => { - (def.repr().scalable.unwrap_or(0) as u64, *f0_elem_ty) - } - // 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) + } else { + 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), } - // 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"), diff --git a/compiler/rustc_target/src/abi/call/arm.rs b/compiler/rustc_target/src/abi/call/arm.rs index 7de4796488fa2..93be382db80a4 100644 --- a/compiler/rustc_target/src/abi/call/arm.rs +++ b/compiler/rustc_target/src/abi/call/arm.rs @@ -19,7 +19,7 @@ where RegKind::Integer => false, RegKind::Float => true, RegKind::Vector => size.bits() == 64 || size.bits() == 128, - RegKind::ScalableVector => true, + RegKind::ScalableVector => unreachable!(), }; valid_unit.then_some(Uniform { unit, total: size }) diff --git a/compiler/rustc_target/src/abi/call/loongarch.rs b/compiler/rustc_target/src/abi/call/loongarch.rs index ee07eb408a650..488418dfd62c5 100644 --- a/compiler/rustc_target/src/abi/call/loongarch.rs +++ b/compiler/rustc_target/src/abi/call/loongarch.rs @@ -76,9 +76,10 @@ where } } }, - Abi::Vector { .. } | Abi::ScalableVector { .. } | Abi::Uninhabited => { + Abi::Vector { .. } | Abi::Uninhabited => { return Err(CannotUseFpConv); } + Abi::ScalableVector { .. } => unreachable!(), Abi::ScalarPair(..) | Abi::Aggregate { .. } => match arg_layout.fields { FieldsShape::Primitive => { unreachable!("aggregates can't have `FieldsShape::Primitive`") diff --git a/compiler/rustc_target/src/abi/call/powerpc64.rs b/compiler/rustc_target/src/abi/call/powerpc64.rs index 74aeffd614e8d..16e22b3c987f5 100644 --- a/compiler/rustc_target/src/abi/call/powerpc64.rs +++ b/compiler/rustc_target/src/abi/call/powerpc64.rs @@ -35,7 +35,7 @@ where RegKind::Integer => false, RegKind::Float => true, RegKind::Vector => arg.layout.size.bits() == 128, - RegKind::ScalableVector => true, + RegKind::ScalableVector => unreachable!(), }; valid_unit.then_some(Uniform { unit, total: arg.layout.size }) diff --git a/compiler/rustc_target/src/abi/call/x86_win64.rs b/compiler/rustc_target/src/abi/call/x86_win64.rs index fd42f85198197..4cf87eebfb0e7 100644 --- a/compiler/rustc_target/src/abi/call/x86_win64.rs +++ b/compiler/rustc_target/src/abi/call/x86_win64.rs @@ -18,7 +18,7 @@ pub fn compute_abi_info(fn_abi: &mut FnAbi<'_, Ty>) { // FIXME(eddyb) there should be a size cap here // (probably what clang calls "illegal vectors"). } - Abi::ScalableVector { .. } => {} + Abi::ScalableVector { .. } => unreachable!(), Abi::Scalar(_) => { if a.layout.size.bytes() > 8 { a.make_indirect(); diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 286d4f2d65386..d832188602ca4 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -516,12 +516,6 @@ fn layout_of_uncached<'tcx>( return Err(error(cx, LayoutError::Unknown(ty))); } - if def.repr().scalable() - && variants[FIRST_VARIANT].iter().all(|field| !field.0.is_zst()) - { - bug!("Fields for a Scalable vector should be a ZST"); - } - return Ok(tcx.mk_layout( cx.layout_of_union(&def.repr(), &variants) .ok_or_else(|| error(cx, LayoutError::Unknown(ty)))?,