1
Fork 0

Self review suggestions

- add back accidentally removed new lines
- try to deref in patterns, rather than in expressions
  (maybe this was the reason of perf regression?...)
This commit is contained in:
Maybe Waffle 2023-01-14 06:47:49 +00:00
parent 8d3c90ae13
commit c21b1f742e
4 changed files with 36 additions and 29 deletions

View file

@ -151,50 +151,50 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Also see https://github.com/rust-lang/rust/issues/68364. // Also see https://github.com/rust-lang/rust/issues/68364.
use rustc_middle::mir::Rvalue::*; use rustc_middle::mir::Rvalue::*;
match rvalue { match *rvalue {
ThreadLocalRef(did) => { ThreadLocalRef(did) => {
let ptr = M::thread_local_static_base_pointer(self, *did)?; let ptr = M::thread_local_static_base_pointer(self, did)?;
self.write_pointer(ptr, &dest)?; self.write_pointer(ptr, &dest)?;
} }
Use(operand) => { Use(ref operand) => {
// Avoid recomputing the layout // Avoid recomputing the layout
let op = self.eval_operand(operand, Some(dest.layout))?; let op = self.eval_operand(operand, Some(dest.layout))?;
self.copy_op(&op, &dest, /*allow_transmute*/ false)?; self.copy_op(&op, &dest, /*allow_transmute*/ false)?;
} }
CopyForDeref(place) => { CopyForDeref(place) => {
let op = self.eval_place_to_op(*place, Some(dest.layout))?; let op = self.eval_place_to_op(place, Some(dest.layout))?;
self.copy_op(&op, &dest, /* allow_transmute*/ false)?; self.copy_op(&op, &dest, /* allow_transmute*/ false)?;
} }
BinaryOp(bin_op, box (left, right)) => { BinaryOp(bin_op, box (ref left, ref right)) => {
let layout = binop_left_homogeneous(*bin_op).then_some(dest.layout); let layout = binop_left_homogeneous(bin_op).then_some(dest.layout);
let left = self.read_immediate(&self.eval_operand(left, layout)?)?; let left = self.read_immediate(&self.eval_operand(left, layout)?)?;
let layout = binop_right_homogeneous(*bin_op).then_some(left.layout); let layout = binop_right_homogeneous(bin_op).then_some(left.layout);
let right = self.read_immediate(&self.eval_operand(right, layout)?)?; let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
self.binop_ignore_overflow(*bin_op, &left, &right, &dest)?; self.binop_ignore_overflow(bin_op, &left, &right, &dest)?;
} }
CheckedBinaryOp(bin_op, box (left, right)) => { CheckedBinaryOp(bin_op, box (ref left, ref right)) => {
// Due to the extra boolean in the result, we can never reuse the `dest.layout`. // Due to the extra boolean in the result, we can never reuse the `dest.layout`.
let left = self.read_immediate(&self.eval_operand(left, None)?)?; let left = self.read_immediate(&self.eval_operand(left, None)?)?;
let layout = binop_right_homogeneous(*bin_op).then_some(left.layout); let layout = binop_right_homogeneous(bin_op).then_some(left.layout);
let right = self.read_immediate(&self.eval_operand(right, layout)?)?; let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
self.binop_with_overflow( self.binop_with_overflow(
*bin_op, /*force_overflow_checks*/ false, &left, &right, &dest, bin_op, /*force_overflow_checks*/ false, &left, &right, &dest,
)?; )?;
} }
UnaryOp(un_op, operand) => { UnaryOp(un_op, ref operand) => {
// The operand always has the same type as the result. // The operand always has the same type as the result.
let val = self.read_immediate(&self.eval_operand(operand, Some(dest.layout))?)?; let val = self.read_immediate(&self.eval_operand(operand, Some(dest.layout))?)?;
let val = self.unary_op(*un_op, &val)?; let val = self.unary_op(un_op, &val)?;
assert_eq!(val.layout, dest.layout, "layout mismatch for result of {:?}", un_op); assert_eq!(val.layout, dest.layout, "layout mismatch for result of {:?}", un_op);
self.write_immediate(*val, &dest)?; self.write_immediate(*val, &dest)?;
} }
Aggregate(box kind, operands) => { Aggregate(box ref kind, ref operands) => {
assert!(matches!(kind, mir::AggregateKind::Array(..))); assert!(matches!(kind, mir::AggregateKind::Array(..)));
for (field_index, operand) in operands.iter().enumerate() { for (field_index, operand) in operands.iter().enumerate() {
@ -204,7 +204,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} }
} }
Repeat(operand, _) => { Repeat(ref operand, _) => {
let src = self.eval_operand(operand, None)?; let src = self.eval_operand(operand, None)?;
assert!(src.layout.is_sized()); assert!(src.layout.is_sized());
let dest = self.force_allocation(&dest)?; let dest = self.force_allocation(&dest)?;
@ -241,14 +241,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} }
Len(place) => { Len(place) => {
let src = self.eval_place(*place)?; let src = self.eval_place(place)?;
let op = self.place_to_op(&src)?; let op = self.place_to_op(&src)?;
let len = op.len(self)?; let len = op.len(self)?;
self.write_scalar(Scalar::from_machine_usize(len, self), &dest)?; self.write_scalar(Scalar::from_machine_usize(len, self), &dest)?;
} }
Ref(_, borrow_kind, place) => { Ref(_, borrow_kind, place) => {
let src = self.eval_place(*place)?; let src = self.eval_place(place)?;
let place = self.force_allocation(&src)?; let place = self.force_allocation(&src)?;
let val = ImmTy::from_immediate(place.to_ref(self), dest.layout); let val = ImmTy::from_immediate(place.to_ref(self), dest.layout);
// A fresh reference was created, make sure it gets retagged. // A fresh reference was created, make sure it gets retagged.
@ -274,7 +274,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
false false
}; };
let src = self.eval_place(*place)?; let src = self.eval_place(place)?;
let place = self.force_allocation(&src)?; let place = self.force_allocation(&src)?;
let mut val = ImmTy::from_immediate(place.to_ref(self), dest.layout); let mut val = ImmTy::from_immediate(place.to_ref(self), dest.layout);
if !place_base_raw { if !place_base_raw {
@ -285,7 +285,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} }
NullaryOp(null_op, ty) => { NullaryOp(null_op, ty) => {
let ty = self.subst_from_current_frame_and_normalize_erasing_regions(*ty)?; let ty = self.subst_from_current_frame_and_normalize_erasing_regions(ty)?;
let layout = self.layout_of(ty)?; let layout = self.layout_of(ty)?;
if layout.is_unsized() { if layout.is_unsized() {
// FIXME: This should be a span_bug (#80742) // FIXME: This should be a span_bug (#80742)
@ -302,21 +302,21 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.write_scalar(Scalar::from_machine_usize(val, self), &dest)?; self.write_scalar(Scalar::from_machine_usize(val, self), &dest)?;
} }
ShallowInitBox(operand, _) => { ShallowInitBox(ref operand, _) => {
let src = self.eval_operand(operand, None)?; let src = self.eval_operand(operand, None)?;
let v = self.read_immediate(&src)?; let v = self.read_immediate(&src)?;
self.write_immediate(*v, &dest)?; self.write_immediate(*v, &dest)?;
} }
Cast(cast_kind, operand, cast_ty) => { Cast(cast_kind, ref operand, cast_ty) => {
let src = self.eval_operand(operand, None)?; let src = self.eval_operand(operand, None)?;
let cast_ty = let cast_ty =
self.subst_from_current_frame_and_normalize_erasing_regions(*cast_ty)?; self.subst_from_current_frame_and_normalize_erasing_regions(cast_ty)?;
self.cast(&src, *cast_kind, cast_ty, &dest)?; self.cast(&src, cast_kind, cast_ty, &dest)?;
} }
Discriminant(place) => { Discriminant(place) => {
let op = self.eval_place_to_op(*place, None)?; let op = self.eval_place_to_op(place, None)?;
let discr_val = self.read_discriminant(&op)?.0; let discr_val = self.read_discriminant(&op)?.0;
self.write_scalar(discr_val, &dest)?; self.write_scalar(discr_val, &dest)?;
} }

View file

@ -35,6 +35,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Branch to the `otherwise` case by default, if no match is found. // Branch to the `otherwise` case by default, if no match is found.
let mut target_block = targets.otherwise(); let mut target_block = targets.otherwise();
for (const_int, target) in targets.iter() { for (const_int, target) in targets.iter() {
// Compare using MIR BinOp::Eq, to also support pointer values. // Compare using MIR BinOp::Eq, to also support pointer values.
// (Avoiding `self.binary_op` as that does some redundant layout computation.) // (Avoiding `self.binary_op` as that does some redundant layout computation.)
@ -50,6 +51,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
break; break;
} }
} }
self.go_to_block(target_block); self.go_to_block(target_block);
} }
@ -66,11 +68,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let old_loc = self.frame().loc; let old_loc = self.frame().loc;
let func = self.eval_operand(func, None)?; let func = self.eval_operand(func, None)?;
let args = self.eval_operands(args)?; let args = self.eval_operands(args)?;
let fn_sig_binder = func.layout.ty.fn_sig(*self.tcx); let fn_sig_binder = func.layout.ty.fn_sig(*self.tcx);
let fn_sig = let fn_sig =
self.tcx.normalize_erasing_late_bound_regions(self.param_env, fn_sig_binder); self.tcx.normalize_erasing_late_bound_regions(self.param_env, fn_sig_binder);
let extra_args = &args[fn_sig.inputs().len()..]; let extra_args = &args[fn_sig.inputs().len()..];
let extra_args = self.tcx.mk_type_list(extra_args.iter().map(|arg| arg.layout.ty)); let extra_args = self.tcx.mk_type_list(extra_args.iter().map(|arg| arg.layout.ty));
let (fn_val, fn_abi, with_caller_location) = match *func.layout.ty.kind() { let (fn_val, fn_abi, with_caller_location) = match *func.layout.ty.kind() {
ty::FnPtr(_sig) => { ty::FnPtr(_sig) => {
let fn_ptr = self.read_pointer(&func)?; let fn_ptr = self.read_pointer(&func)?;
@ -144,6 +148,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Abort => { Abort => {
M::abort(self, "the program aborted execution".to_owned())?; M::abort(self, "the program aborted execution".to_owned())?;
} }
// When we encounter Resume, we've finished unwinding // When we encounter Resume, we've finished unwinding
// cleanup for the current stack frame. We pop it in order // cleanup for the current stack frame. We pop it in order
// to continue unwinding the next frame // to continue unwinding the next frame
@ -154,8 +159,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.pop_stack_frame(/* unwinding */ true)?; self.pop_stack_frame(/* unwinding */ true)?;
return Ok(()); return Ok(());
} }
// It is UB to ever encounter this. // It is UB to ever encounter this.
Unreachable => throw_ub!(Unreachable), Unreachable => throw_ub!(Unreachable),
// These should never occur for MIR we actually run. // These should never occur for MIR we actually run.
DropAndReplace { .. } DropAndReplace { .. }
| FalseEdge { .. } | FalseEdge { .. }

View file

@ -483,8 +483,8 @@ macro_rules! make_value_visitor {
// Visit the fields of this value. // Visit the fields of this value.
match &v.layout().fields { match &v.layout().fields {
FieldsShape::Primitive => {} FieldsShape::Primitive => {}
FieldsShape::Union(fields) => { &FieldsShape::Union(fields) => {
self.visit_union(v, *fields)?; self.visit_union(v, fields)?;
} }
FieldsShape::Arbitrary { offsets, .. } => { FieldsShape::Arbitrary { offsets, .. } => {
// FIXME: We collect in a vec because otherwise there are lifetime // FIXME: We collect in a vec because otherwise there are lifetime

View file

@ -2881,13 +2881,13 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let opt_self_ty = maybe_qself.as_ref().map(|qself| self.ast_ty_to_ty(qself)); let opt_self_ty = maybe_qself.as_ref().map(|qself| self.ast_ty_to_ty(qself));
self.res_to_ty(opt_self_ty, path, false) self.res_to_ty(opt_self_ty, path, false)
} }
hir::TyKind::OpaqueDef(item_id, lifetimes, in_trait) => { &hir::TyKind::OpaqueDef(item_id, lifetimes, in_trait) => {
let opaque_ty = tcx.hir().item(*item_id); let opaque_ty = tcx.hir().item(item_id);
let def_id = item_id.owner_id.to_def_id(); let def_id = item_id.owner_id.to_def_id();
match opaque_ty.kind { match opaque_ty.kind {
hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => { hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => {
self.impl_trait_ty_to_ty(def_id, lifetimes, origin, *in_trait) self.impl_trait_ty_to_ty(def_id, lifetimes, origin, in_trait)
} }
ref i => bug!("`impl Trait` pointed to non-opaque type?? {:#?}", i), ref i => bug!("`impl Trait` pointed to non-opaque type?? {:#?}", i),
} }