pr feedback - document code & give variable more descriptive name

Author: Jamesbarford

compiler/rustc_codegen_llvm/src/common.rs

@@ -100,11 +100,6 @@ impl<'ll> CodegenCx<'ll, '_> {
         unsafe { llvm::LLVMConstArray2(ty, elts.as_ptr(), len) }
     }
 
-    pub fn const_vector(&self, elts: &[&'ll Value]) -> &'ll Value {
-        let len = c_uint::try_from(elts.len()).expect("LLVMConstVector elements len overflow");
-        unsafe { llvm::LLVMConstVector(elts.as_ptr(), len) }
-    }
-
     pub fn const_bytes(&self, bytes: &[u8]) -> &'ll Value {
         bytes_in_context(self.llcx, bytes)
     }
@@ -225,7 +220,8 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
     }
 
     fn const_vector(&self, elts: &[&'ll Value]) -> &'ll Value {
-        unsafe { llvm::LLVMConstVector(elts.as_ptr(), elts.len() as c_uint) }
+        let len = c_uint::try_from(elts.len()).expect("LLVMConstVector elements len overflow");
+        unsafe { llvm::LLVMConstVector(elts.as_ptr(), len) }
     }
 
     fn const_to_opt_uint(&self, v: &'ll Value) -> Option<u64> {

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -923,6 +923,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         // third argument must be constant. This is
                         // checked by the type-checker.
                         if i == 2 && intrinsic.name == sym::simd_shuffle {
+                            // FIXME: the simd_shuffle argument is actually an array,
+                            // not a vector, so we need this special hack to make sure
+                            // it is passed as an immediate. We should pass the
+                            // shuffle indices as a vector instead to avoid this hack.
                             if let mir::Operand::Constant(constant) = &arg.node {
                                 let (llval, ty) = self.immediate_const_vector(bx, constant);
                                 return OperandRef {

compiler/rustc_codegen_ssa/src/mir/constant.rs

@@ -67,6 +67,9 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     ) -> (Bx::Value, Ty<'tcx>) {
         let ty = self.monomorphize(constant.ty());
         let ty_is_simd = ty.is_simd();
+        // FIXME: ideally we'd assert that this is a SIMD type, but simd_shuffle
+        // in its current form relies on a regular array being passed as an
+        // immediate argument. This hack can be removed once that is fixed.
         let field_ty = if ty_is_simd {
             ty.simd_size_and_type(bx.tcx()).1
         } else {
@@ -79,11 +82,17 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             .map(|x| x.ok())
             .flatten()
             .map(|val| {
-                let field_iter = val.unwrap_branch();
-                let first = field_iter.get(0).unwrap();
+                // Depending on whether this is a SIMD type with an array field
+                // or a type with many fields (one for each elements), the valtree
+                // is either a single branch with N children, or a root note
+                // with exactly one child which then in turn has many children.
+                // So we look at the first child to determine whether it is a
+                // leaf or whether we have to go one more layer down.
+                let branch_or_leaf = val.unwrap_branch();
+                let first = branch_or_leaf.get(0).unwrap();
                 let field_iter = match first {
                     ValTree::Branch(_) => first.unwrap_branch().iter(),
-                    ValTree::Leaf(_) => field_iter.iter(),
+                    ValTree::Leaf(_) => branch_or_leaf.iter(),
                 };
                 let values: Vec<_> = field_iter
                     .map(|field| {