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.

Author: JamieCunliffe

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 <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),
         }
     }

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 }),
         }

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"),

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 })

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`")

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 })

compiler/rustc_target/src/abi/call/x86_win64.rs

@@ -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();

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)))?,