Merge pull request rust-lang#223 from oli-obk/lvalue_read

Reinstate `eval_and_read_lvalue` optimizations

Author: oli-obk

src/error.rs

@@ -65,6 +65,7 @@ pub enum EvalError<'tcx> {
     Panic,
     NeedsRfc(String),
     NotConst(String),
+    ReadFromReturnPointer,
 }
 
 pub type EvalResult<'tcx, T = ()> = Result<T, EvalError<'tcx>>;
@@ -162,6 +163,8 @@ impl<'tcx> Error for EvalError<'tcx> {
                 "this feature needs an rfc before being allowed inside constants",
             EvalError::NotConst(_) =>
                 "this feature is not compatible with constant evaluation",
+            EvalError::ReadFromReturnPointer =>
+                "tried to read from the return pointer",
         }
     }
 

src/eval_context.rs

@@ -821,7 +821,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         Ok(())
     }
 
-    fn type_is_fat_ptr(&self, ty: Ty<'tcx>) -> bool {
+    pub(super) fn type_is_fat_ptr(&self, ty: Ty<'tcx>) -> bool {
         match ty.sty {
             ty::TyRawPtr(ref tam) |
             ty::TyRef(_, ref tam) => !self.type_is_sized(tam.ty),
@@ -882,6 +882,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
     pub fn get_field_ty(&self, ty: Ty<'tcx>, field_index: usize) -> EvalResult<'tcx, Ty<'tcx>> {
         match ty.sty {
             ty::TyAdt(adt_def, _) if adt_def.is_box() => self.get_fat_field(ty.boxed_ty(), field_index),
+            ty::TyAdt(adt_def, substs) if adt_def.is_enum() => {
+                use rustc::ty::layout::Layout::*;
+                match *self.type_layout(ty)? {
+                    RawNullablePointer { nndiscr, .. } |
+                    StructWrappedNullablePointer { nndiscr, .. } => Ok(adt_def.variants[nndiscr as usize].fields[field_index].ty(self.tcx, substs)),
+                    _ => Err(EvalError::Unimplemented(format!("get_field_ty can't handle enum type: {:?}, {:?}", ty, ty.sty))),
+                }
+            }
             ty::TyAdt(adt_def, substs) => {
                 Ok(adt_def.struct_variant().fields[field_index].ty(self.tcx, substs))
             }
@@ -890,6 +898,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
 
             ty::TyRef(_, ref tam) |
             ty::TyRawPtr(ref tam) => self.get_fat_field(tam.ty, field_index),
+
+            ty::TyArray(ref inner, _) => Ok(inner),
+
             _ => Err(EvalError::Unimplemented(format!("can't handle type: {:?}, {:?}", ty, ty.sty))),
         }
     }
@@ -916,14 +927,17 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
         }
     }
 
-    pub fn get_field_count(&self, ty: Ty<'tcx>) -> EvalResult<'tcx, usize> {
+    pub fn get_field_count(&self, ty: Ty<'tcx>) -> EvalResult<'tcx, u64> {
         let layout = self.type_layout(ty)?;
 
         use rustc::ty::layout::Layout::*;
         match *layout {
-            Univariant { ref variant, .. } => Ok(variant.offsets.len()),
+            Univariant { ref variant, .. } => Ok(variant.offsets.len() as u64),
             FatPointer { .. } => Ok(2),
-            StructWrappedNullablePointer { ref nonnull, .. } => Ok(nonnull.offsets.len()),
+            StructWrappedNullablePointer { ref nonnull, .. } => Ok(nonnull.offsets.len() as u64),
+            Vector { count , .. } |
+            Array { count, .. } => Ok(count),
+            Scalar { .. } => Ok(0),
             _ => {
                 let msg = format!("can't handle type: {:?}, with layout: {:?}", ty, layout);
                 Err(EvalError::Unimplemented(msg))

src/lvalue.rs

@@ -126,8 +126,63 @@ impl<'tcx> Global<'tcx> {
 }
 
 impl<'a, 'tcx> EvalContext<'a, 'tcx> {
+    /// Reads a value from the lvalue without going through the intermediate step of obtaining
+    /// a `miri::Lvalue`
+    pub fn try_read_lvalue(&mut self, lvalue: &mir::Lvalue<'tcx>) -> EvalResult<'tcx, Option<Value>> {
+        use rustc::mir::Lvalue::*;
+        match *lvalue {
+            // Might allow this in the future, right now there's no way to do this from Rust code anyway
+            Local(mir::RETURN_POINTER) => Err(EvalError::ReadFromReturnPointer),
+            // Directly reading a local will always succeed
+            Local(local) => self.frame().get_local(local).map(Some),
+            // Directly reading a static will always succeed
+            Static(ref static_) => {
+                let instance = ty::Instance::mono(self.tcx, static_.def_id);
+                let cid = GlobalId { instance, promoted: None };
+                Ok(Some(self.globals.get(&cid).expect("global not cached").value))
+            },
+            Projection(ref proj) => self.try_read_lvalue_projection(proj),
+        }
+    }
+
+    fn try_read_lvalue_projection(&mut self, proj: &mir::LvalueProjection<'tcx>) -> EvalResult<'tcx, Option<Value>> {
+        use rustc::mir::ProjectionElem::*;
+        let base = match self.try_read_lvalue(&proj.base)? {
+            Some(base) => base,
+            None => return Ok(None),
+        };
+        let base_ty = self.lvalue_ty(&proj.base);
+        match proj.elem {
+            Field(field, _) => match (field.index(), base) {
+                // the only field of a struct
+                (0, Value::ByVal(val)) => Ok(Some(Value::ByVal(val))),
+                // split fat pointers, 2 element tuples, ...
+                (0...1, Value::ByValPair(a, b)) if self.get_field_count(base_ty)? == 2 => {
+                    let val = [a, b][field.index()];
+                    Ok(Some(Value::ByVal(val)))
+                },
+                // the only field of a struct is a fat pointer
+                (0, Value::ByValPair(..)) => Ok(Some(base)),
+                _ => Ok(None),
+            },
+            // The NullablePointer cases should work fine, need to take care for normal enums
+            Downcast(..) |
+            Subslice { .. } |
+            // reading index 0 or index 1 from a ByVal or ByVal pair could be optimized
+            ConstantIndex { .. } | Index(_) |
+            // No way to optimize this projection any better than the normal lvalue path
+            Deref => Ok(None),
+        }
+    }
+
     pub(super) fn eval_and_read_lvalue(&mut self, lvalue: &mir::Lvalue<'tcx>) -> EvalResult<'tcx, Value> {
         let ty = self.lvalue_ty(lvalue);
+        // Shortcut for things like accessing a fat pointer's field,
+        // which would otherwise (in the `eval_lvalue` path) require moving a `ByValPair` to memory
+        // and returning an `Lvalue::Ptr` to it
+        if let Some(val) = self.try_read_lvalue(lvalue)? {
+            return Ok(val);
+        }
         let lvalue = self.eval_lvalue(lvalue)?;
 
         if ty.is_never() {
@@ -233,7 +288,30 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> {
             _ => bug!("field access on non-product type: {:?}", base_layout),
         };
 
-        let (base_ptr, base_extra) = self.force_allocation(base)?.to_ptr_and_extra();
+        // Do not allocate in trivial cases
+        let (base_ptr, base_extra) = match base {
+            Lvalue::Ptr { ptr, extra } => (ptr, extra),
+            Lvalue::Local { frame, local } => match self.stack[frame].get_local(local)? {
+                // in case the type has a single field, just return the value
+                Value::ByVal(_) if self.get_field_count(base_ty).map(|c| c == 1).unwrap_or(false) => {
+                    assert_eq!(offset.bytes(), 0, "ByVal can only have 1 non zst field with offset 0");
+                    return Ok(base);
+                },
+                Value::ByRef(_) |
+                Value::ByValPair(..) |
+                Value::ByVal(_) => self.force_allocation(base)?.to_ptr_and_extra(),
+            },
+            Lvalue::Global(cid) => match self.globals.get(&cid).expect("uncached global").value {
+                // in case the type has a single field, just return the value
+                Value::ByVal(_) if self.get_field_count(base_ty).map(|c| c == 1).unwrap_or(false) => {
+                    assert_eq!(offset.bytes(), 0, "ByVal can only have 1 non zst field with offset 0");
+                    return Ok(base);
+                },
+                Value::ByRef(_) |
+                Value::ByValPair(..) |
+                Value::ByVal(_) => self.force_allocation(base)?.to_ptr_and_extra(),
+            },
+        };
 
         let offset = match base_extra {
             LvalueExtra::Vtable(tab) => {

src/memory.rs

@@ -772,7 +772,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> {
                     4 => !0u32 as u128,
                     8 => !0u64 as u128,
                     16 => !0,
-                    _ => bug!("unexpected PrimVal::Bytes size"),
+                    n => bug!("unexpected PrimVal::Bytes size: {}", n),
                 };
                 self.write_uint(dest.to_ptr()?, bytes & mask, size)
             }

src/value.rs

@@ -108,7 +108,7 @@ impl<'a, 'tcx: 'a> Value {
                 assert_eq!(len as u64 as u128, len);
                 Ok((ptr, len as u64))
             },
-            _ => unimplemented!(),
+            ByVal(_) => unimplemented!(),
         }
     }
 }