Merge pull request #146 from oli-obk/peephole1

[Peephole Optimization 1/n] Don't allocate for structs with a single primval field

Author: solson

src/eval_context.rs

@@ -317,14 +317,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                 match global_value.value {
                     Value::ByRef(ptr) => self.memory.mark_static_initalized(ptr.alloc_id, mutable)?,
                     Value::ByVal(val) => if let PrimVal::Ptr(ptr) = val {
-                        self.memory.mark_static_initalized(ptr.alloc_id, mutable)?;
+                        self.memory.mark_inner_allocation(ptr.alloc_id, mutable)?;
                     },
                     Value::ByValPair(val1, val2) => {
                         if let PrimVal::Ptr(ptr) = val1 {
-                            self.memory.mark_static_initalized(ptr.alloc_id, mutable)?;
+                            self.memory.mark_inner_allocation(ptr.alloc_id, mutable)?;
                         }
                         if let PrimVal::Ptr(ptr) = val2 {
-                            self.memory.mark_static_initalized(ptr.alloc_id, mutable)?;
+                            self.memory.mark_inner_allocation(ptr.alloc_id, mutable)?;
                         }
                     },
                 }
@@ -374,7 +374,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         discr_val: u128,
         variant_idx: usize,
         discr_size: u64,
-    ) -> EvalResult<'tcx> {
+    ) -> EvalResult<'tcx>
+        where J::IntoIter: ExactSizeIterator,
+    {
         // FIXME(solson)
         let dest_ptr = self.force_allocation(dest)?.to_ptr();
 
@@ -397,7 +399,19 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         dest: Lvalue<'tcx>,
         dest_ty: Ty<'tcx>,
         operands: J,
-    ) -> EvalResult<'tcx> {
+    ) -> EvalResult<'tcx>
+        where J::IntoIter: ExactSizeIterator,
+    {
+        if self.type_size(dest_ty)? == Some(0) {
+            // zst assigning is a nop
+            return Ok(());
+        }
+        if self.ty_to_primval_kind(dest_ty).is_ok() {
+            let mut iter = operands.into_iter();
+            assert_eq!(iter.len(), 1);
+            let (value, value_ty) = iter.next().unwrap().into_val_ty_pair(self)?;
+            return self.write_value(value, dest, value_ty);
+        }
         for (field_index, operand) in operands.into_iter().enumerate() {
             let (value, value_ty) = operand.into_val_ty_pair(self)?;
             let field_dest = self.lvalue_field(dest, field_index, dest_ty, value_ty)?;
@@ -795,7 +809,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         }
     }
 
-    fn get_field_count(&self, ty: Ty<'tcx>) -> EvalResult<'tcx, usize> {
+    pub fn get_field_count(&self, ty: Ty<'tcx>) -> EvalResult<'tcx, usize> {
         let layout = self.type_layout(ty)?;
 
         use rustc::ty::layout::Layout::*;
@@ -1052,8 +1066,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         a: PrimVal,
         b: PrimVal,
         ptr: Pointer,
-        ty: Ty<'tcx>
+        mut ty: Ty<'tcx>
     ) -> EvalResult<'tcx> {
+        while self.get_field_count(ty)? == 1 {
+            ty = self.get_field_ty(ty, 0)?;
+        }
         assert_eq!(self.get_field_count(ty)?, 2);
         let field_0 = self.get_field_offset(ty, 0)?.bytes();
         let field_1 = self.get_field_offset(ty, 1)?.bytes();
@@ -1109,7 +1126,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
             ty::TyAdt(ref def, _) if def.is_box() => PrimValKind::Ptr,
 
-            ty::TyAdt(..) => {
+            ty::TyAdt(ref def, substs) => {
                 use rustc::ty::layout::Layout::*;
                 match *self.type_layout(ty)? {
                     CEnum { discr, signed, .. } => {
@@ -1132,6 +1149,18 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                         }
                     }
 
+                    // represent single field structs as their single field
+                    Univariant { .. } => {
+                        // enums with just one variant are no different, but `.struct_variant()` doesn't work for enums
+                        let variant = &def.variants[0];
+                        // FIXME: also allow structs with only a single non zst field
+                        if variant.fields.len() == 1 {
+                            return self.ty_to_primval_kind(variant.fields[0].ty(self.tcx, substs));
+                        } else {
+                            return Err(EvalError::TypeNotPrimitive(ty));
+                        }
+                    }
+
                     _ => return Err(EvalError::TypeNotPrimitive(ty)),
                 }
             }
@@ -1320,8 +1349,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     }
                     return self.unsize_into_ptr(src, src_ty, dest, dest_ty, src_ty.boxed_ty(), dest_ty.boxed_ty());
                 }
-                // FIXME(solson)
-                let dest = self.force_allocation(dest)?.to_ptr();
+                if self.ty_to_primval_kind(src_ty).is_ok() {
+                    let sty = self.get_field_ty(src_ty, 0)?;
+                    let dty = self.get_field_ty(dest_ty, 0)?;
+                    return self.unsize_into(src, sty, dest, dty);
+                }
                 // unsizing of generic struct with pointer fields
                 // Example: `Arc<T>` -> `Arc<Trait>`
                 // here we need to increase the size of every &T thin ptr field to a fat ptr
@@ -1338,6 +1370,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     _ => bug!("expected pointer, got {:?}", src),
                 };
 
+                // FIXME(solson)
+                let dest = self.force_allocation(dest)?.to_ptr();
                 let iter = src_fields.zip(dst_fields).enumerate();
                 for (i, (src_f, dst_f)) in iter {
                     let src_fty = monomorphize_field_ty(self.tcx, src_f, substs_a);

src/lvalue.rs

@@ -219,14 +219,24 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                     (ptr, LvalueExtra::None)
                 },
                 Value::ByVal(PrimVal::Undef) => {
-                    // FIXME: add some logic for when to not allocate
-                    (self.force_allocation(base)?.to_ptr(), LvalueExtra::None)
+                    // FIXME: allocate in fewer cases
+                    if self.ty_to_primval_kind(base_ty).is_ok() {
+                        return Ok(base);
+                    } else {
+                        (self.force_allocation(base)?.to_ptr(), LvalueExtra::None)
+                    }
                 },
                 Value::ByVal(_) => {
-                    assert_eq!(offset.bytes(), 0, "ByVal can only have 1 non zst field with offset 0");
+                    assert_eq!(field_index, 0, "ByVal can only have 1 non zst field with offset 0");
                     return Ok(base);
                 },
                 Value::ByValPair(_, _) => {
+                    let field_count = self.get_field_count(base_ty)?;
+                    if field_count == 1 {
+                        assert_eq!(field_index, 0, "{:?} has only one field", base_ty);
+                        return Ok(base);
+                    }
+                    assert_eq!(field_count, 2);
                     assert!(field_index < 2);
                     return Ok(Lvalue::Local {
                         frame,

src/memory.rs

@@ -658,13 +658,24 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
 impl<'a, 'tcx> Memory<'a, 'tcx> {
     /// mark an allocation as being the entry point to a static (see `static_alloc` field)
     pub fn mark_static(&mut self, alloc_id: AllocId) {
+        trace!("mark_static: {:?}", alloc_id);
         if alloc_id != NEVER_ALLOC_ID && alloc_id != ZST_ALLOC_ID && !self.static_alloc.insert(alloc_id) {
             bug!("tried to mark an allocation ({:?}) as static twice", alloc_id);
         }
     }
 
+    /// mark an allocation pointed to by a static as static and initialized
+    pub fn mark_inner_allocation(&mut self, alloc: AllocId, mutable: bool) -> EvalResult<'tcx> {
+        // relocations into other statics are not "inner allocations"
+        if !self.static_alloc.contains(&alloc) {
+            self.mark_static_initalized(alloc, mutable)?;
+        }
+        Ok(())
+    }
+
     /// mark an allocation as static and initialized, either mutable or not
     pub fn mark_static_initalized(&mut self, alloc_id: AllocId, mutable: bool) -> EvalResult<'tcx> {
+        trace!("mark_static_initialized {:?}, mutable: {:?}", alloc_id, mutable);
         // do not use `self.get_mut(alloc_id)` here, because we might have already marked a
         // sub-element or have circular pointers (e.g. `Rc`-cycles)
         let relocations = match self.alloc_map.get_mut(&alloc_id) {
@@ -684,10 +695,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
         };
         // recurse into inner allocations
         for &alloc in relocations.values() {
-            // relocations into other statics are not "inner allocations"
-            if !self.static_alloc.contains(&alloc) {
-                self.mark_static_initalized(alloc, mutable)?;
-            }
+            self.mark_inner_allocation(alloc, mutable)?;
         }
         // put back the relocations
         self.alloc_map.get_mut(&alloc_id).expect("checked above").relocations = relocations;

src/terminator/mod.rs

@@ -551,7 +551,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
                                 args.push((undef, field_ty));
                             }
                         },
-                        _ => bug!("rust-call ABI tuple argument was {:?}", last),
+                        _ => bug!("rust-call ABI tuple argument was {:?}, but {:?} were expected", last, fields),
                     }
                 }
                 ty => bug!("expected tuple as last argument in function with 'rust-call' ABI, got {:?}", ty),