Skip to content

Commit

Permalink
Fix up style review comments.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
JamieCunliffe committed Apr 26, 2024
1 parent 351bec3 commit a371ac9
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 27 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <vscale x 16 x i8>, 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),
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
}
Expand Down
37 changes: 20 additions & 17 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/arm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_target/src/abi/call/loongarch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`")
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/powerpc64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/call/x86_win64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn compute_abi_info<Ty>(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();
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))?,
Expand Down

0 comments on commit a371ac9

Please sign in to comment.