Skip to content

Refactor Miri ops (unary, binary) to have more types #63658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 17, 2019
Merged

Refactor Miri ops (unary, binary) to have more types #63658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a2a517
Add ImmTy::from_uint
RalfJung Aug 17, 2019
3edf099
make both unary_op and binary_op fully typed, including a return type
RalfJung Aug 10, 2019
5ac2045
use ImmTy::from_uint in a few more spots
RalfJung Aug 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/librustc_mir/const_eval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@ use rustc::hir::def::DefKind;
use rustc::hir::def_id::DefId;
use rustc::mir::interpret::{ConstEvalErr, ErrorHandled, ScalarMaybeUndef};
use rustc::mir;
use rustc::ty::{self, TyCtxt};
use rustc::ty::{self, Ty, TyCtxt, subst::Subst};
use rustc::ty::layout::{self, LayoutOf, VariantIdx};
use rustc::ty::subst::Subst;
use rustc::traits::Reveal;
use rustc_data_structures::fx::FxHashMap;

Expand Down Expand Up @@ -415,7 +414,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
_bin_op: mir::BinOp,
_left: ImmTy<'tcx>,
_right: ImmTy<'tcx>,
) -> InterpResult<'tcx, (Scalar, bool)> {
) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)> {
Err(
ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
)
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let l = self.read_immediate(args[0])?;
let r = self.read_immediate(args[1])?;
let is_add = intrinsic_name == "saturating_add";
let (val, overflowed) = self.binary_op(if is_add {
let (val, overflowed, _ty) = self.overflowing_binary_op(if is_add {
BinOp::Add
} else {
BinOp::Sub
Expand Down Expand Up @@ -184,7 +184,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
"unchecked_shr" => BinOp::Shr,
_ => bug!("Already checked for int ops")
};
let (val, overflowed) = self.binary_op(bin_op, l, r)?;
let (val, overflowed, _ty) = self.overflowing_binary_op(bin_op, l, r)?;
if overflowed {
let layout = self.layout_of(substs.type_at(0))?;
let r_val = r.to_scalar()?.to_bits(layout.size)?;
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/machine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::hash::Hash;

use rustc::hir::def_id::DefId;
use rustc::mir;
use rustc::ty::{self, TyCtxt};
use rustc::ty::{self, Ty, TyCtxt};

use super::{
Allocation, AllocId, InterpResult, Scalar, AllocationExtra,
Expand Down Expand Up @@ -176,7 +176,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
bin_op: mir::BinOp,
left: ImmTy<'tcx, Self::PointerTag>,
right: ImmTy<'tcx, Self::PointerTag>,
) -> InterpResult<'tcx, (Scalar<Self::PointerTag>, bool)>;
) -> InterpResult<'tcx, (Scalar<Self::PointerTag>, bool, Ty<'tcx>)>;

/// Heap allocations via the `box` keyword.
fn box_alloc(
Expand Down
17 changes: 13 additions & 4 deletions src/librustc_mir/interpret/operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'tcx, Tag> Immediate<Tag> {
// as input for binary and cast operations.
#[derive(Copy, Clone, Debug)]
pub struct ImmTy<'tcx, Tag=()> {
pub imm: Immediate<Tag>,
pub(crate) imm: Immediate<Tag>,
pub layout: TyLayout<'tcx>,
}

Expand Down Expand Up @@ -155,7 +155,7 @@ impl<Tag> Operand<Tag> {

#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub struct OpTy<'tcx, Tag=()> {
op: Operand<Tag>,
op: Operand<Tag>, // Keep this private, it helps enforce invariants
pub layout: TyLayout<'tcx>,
}

Expand Down Expand Up @@ -187,13 +187,22 @@ impl<'tcx, Tag> From<ImmTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
}
}

impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag>
{
impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> {
#[inline]
pub fn from_scalar(val: Scalar<Tag>, layout: TyLayout<'tcx>) -> Self {
ImmTy { imm: val.into(), layout }
}

#[inline]
pub fn from_uint(i: impl Into<u128>, layout: TyLayout<'tcx>) -> Self {
Self::from_scalar(Scalar::from_uint(i, layout.size), layout)
}

#[inline]
pub fn from_int(i: impl Into<i128>, layout: TyLayout<'tcx>) -> Self {
Self::from_scalar(Scalar::from_int(i, layout.size), layout)
}

#[inline]
pub fn to_bits(self) -> InterpResult<'tcx, u128> {
self.to_scalar()?.to_bits(self.layout.size)
Expand Down
123 changes: 74 additions & 49 deletions src/librustc_mir/interpret/operator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc::mir;
use rustc::ty::{self, layout::TyLayout};
use rustc::ty::{self, Ty, layout::{TyLayout, LayoutOf}};
use syntax::ast::FloatTy;
use rustc_apfloat::Float;
use rustc::mir::interpret::{InterpResult, Scalar};
Expand All @@ -17,7 +17,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
right: ImmTy<'tcx, M::PointerTag>,
dest: PlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx> {
let (val, overflowed) = self.binary_op(op, left, right)?;
let (val, overflowed, ty) = self.overflowing_binary_op(op, left, right)?;
debug_assert_eq!(
self.tcx.intern_tup(&[ty, self.tcx.types.bool]),
dest.layout.ty,
"type mismatch for result of {:?}", op,
);
let val = Immediate::ScalarPair(val.into(), Scalar::from_bool(overflowed).into());
self.write_immediate(val, dest)
}
Expand All @@ -31,7 +36,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
right: ImmTy<'tcx, M::PointerTag>,
dest: PlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx> {
let (val, _overflowed) = self.binary_op(op, left, right)?;
let (val, _overflowed, ty) = self.overflowing_binary_op(op, left, right)?;
assert_eq!(ty, dest.layout.ty, "type mismatch for result of {:?}", op);
self.write_scalar(val, dest)
}
}
Expand All @@ -42,7 +48,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
bin_op: mir::BinOp,
l: char,
r: char,
) -> (Scalar<M::PointerTag>, bool) {
) -> (Scalar<M::PointerTag>, bool, Ty<'tcx>) {
use rustc::mir::BinOp::*;

let res = match bin_op {
Expand All @@ -54,15 +60,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ge => l >= r,
_ => bug!("Invalid operation on char: {:?}", bin_op),
};
return (Scalar::from_bool(res), false);
return (Scalar::from_bool(res), false, self.tcx.types.bool);
}

fn binary_bool_op(
&self,
bin_op: mir::BinOp,
l: bool,
r: bool,
) -> (Scalar<M::PointerTag>, bool) {
) -> (Scalar<M::PointerTag>, bool, Ty<'tcx>) {
use rustc::mir::BinOp::*;

let res = match bin_op {
Expand All @@ -77,32 +83,33 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
BitXor => l ^ r,
_ => bug!("Invalid operation on bool: {:?}", bin_op),
};
return (Scalar::from_bool(res), false);
return (Scalar::from_bool(res), false, self.tcx.types.bool);
}

fn binary_float_op<F: Float + Into<Scalar<M::PointerTag>>>(
&self,
bin_op: mir::BinOp,
ty: Ty<'tcx>,
l: F,
r: F,
) -> (Scalar<M::PointerTag>, bool) {
) -> (Scalar<M::PointerTag>, bool, Ty<'tcx>) {
use rustc::mir::BinOp::*;

let val = match bin_op {
Eq => Scalar::from_bool(l == r),
Ne => Scalar::from_bool(l != r),
Lt => Scalar::from_bool(l < r),
Le => Scalar::from_bool(l <= r),
Gt => Scalar::from_bool(l > r),
Ge => Scalar::from_bool(l >= r),
Add => (l + r).value.into(),
Sub => (l - r).value.into(),
Mul => (l * r).value.into(),
Div => (l / r).value.into(),
Rem => (l % r).value.into(),
let (val, ty) = match bin_op {
Eq => (Scalar::from_bool(l == r), self.tcx.types.bool),
Ne => (Scalar::from_bool(l != r), self.tcx.types.bool),
Lt => (Scalar::from_bool(l < r), self.tcx.types.bool),
Le => (Scalar::from_bool(l <= r), self.tcx.types.bool),
Gt => (Scalar::from_bool(l > r), self.tcx.types.bool),
Ge => (Scalar::from_bool(l >= r), self.tcx.types.bool),
Add => ((l + r).value.into(), ty),
Sub => ((l - r).value.into(), ty),
Mul => ((l * r).value.into(), ty),
Div => ((l / r).value.into(), ty),
Rem => ((l % r).value.into(), ty),
_ => bug!("invalid float op: `{:?}`", bin_op),
};
return (val, false);
return (val, false, ty);
}

fn binary_int_op(
Expand All @@ -113,7 +120,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
left_layout: TyLayout<'tcx>,
r: u128,
right_layout: TyLayout<'tcx>,
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool)> {
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool, Ty<'tcx>)> {
use rustc::mir::BinOp::*;

// Shift ops can have an RHS with a different numeric type.
Expand Down Expand Up @@ -142,7 +149,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};
let truncated = self.truncate(result, left_layout);
return Ok((Scalar::from_uint(truncated, size), oflo));
return Ok((Scalar::from_uint(truncated, size), oflo, left_layout.ty));
}

// For the remaining ops, the types must be the same on both sides
Expand All @@ -167,7 +174,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if let Some(op) = op {
let l = self.sign_extend(l, left_layout) as i128;
let r = self.sign_extend(r, right_layout) as i128;
return Ok((Scalar::from_bool(op(&l, &r)), false));
return Ok((Scalar::from_bool(op(&l, &r)), false, self.tcx.types.bool));
}
let op: Option<fn(i128, i128) -> (i128, bool)> = match bin_op {
Div if r == 0 => throw_panic!(DivisionByZero),
Expand All @@ -187,7 +194,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Rem | Div => {
// int_min / -1
if r == -1 && l == (1 << (size.bits() - 1)) {
return Ok((Scalar::from_uint(l, size), true));
return Ok((Scalar::from_uint(l, size), true, left_layout.ty));
}
},
_ => {},
Expand All @@ -202,25 +209,24 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// this may be out-of-bounds for the result type, so we have to truncate ourselves
let result = result as u128;
let truncated = self.truncate(result, left_layout);
return Ok((Scalar::from_uint(truncated, size), oflo));
return Ok((Scalar::from_uint(truncated, size), oflo, left_layout.ty));
}
}

let size = left_layout.size;

// only ints left
let val = match bin_op {
Eq => Scalar::from_bool(l == r),
Ne => Scalar::from_bool(l != r),
let (val, ty) = match bin_op {
Eq => (Scalar::from_bool(l == r), self.tcx.types.bool),
Ne => (Scalar::from_bool(l != r), self.tcx.types.bool),

Lt => Scalar::from_bool(l < r),
Le => Scalar::from_bool(l <= r),
Gt => Scalar::from_bool(l > r),
Ge => Scalar::from_bool(l >= r),
Lt => (Scalar::from_bool(l < r), self.tcx.types.bool),
Le => (Scalar::from_bool(l <= r), self.tcx.types.bool),
Gt => (Scalar::from_bool(l > r), self.tcx.types.bool),
Ge => (Scalar::from_bool(l >= r), self.tcx.types.bool),

BitOr => Scalar::from_uint(l | r, size),
BitAnd => Scalar::from_uint(l & r, size),
BitXor => Scalar::from_uint(l ^ r, size),
BitOr => (Scalar::from_uint(l | r, size), left_layout.ty),
BitAnd => (Scalar::from_uint(l & r, size), left_layout.ty),
BitXor => (Scalar::from_uint(l ^ r, size), left_layout.ty),

Add | Sub | Mul | Rem | Div => {
debug_assert!(!left_layout.abi.is_signed());
Expand All @@ -236,7 +242,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
};
let (result, oflo) = op(l, r);
let truncated = self.truncate(result, left_layout);
return Ok((Scalar::from_uint(truncated, size), oflo || truncated != result));
return Ok((
Scalar::from_uint(truncated, size),
oflo || truncated != result,
left_layout.ty,
));
}

_ => {
Expand All @@ -250,17 +260,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};

Ok((val, false))
Ok((val, false, ty))
}

/// Returns the result of the specified operation and whether it overflowed.
#[inline]
pub fn binary_op(
/// Returns the result of the specified operation, whether it overflowed, and
/// the result type.
pub fn overflowing_binary_op(
&self,
bin_op: mir::BinOp,
left: ImmTy<'tcx, M::PointerTag>,
right: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool)> {
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool, Ty<'tcx>)> {
trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})",
bin_op, *left, left.layout.ty, *right, right.layout.ty);

Expand All @@ -279,11 +289,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
ty::Float(fty) => {
assert_eq!(left.layout.ty, right.layout.ty);
let ty = left.layout.ty;
let left = left.to_scalar()?;
let right = right.to_scalar()?;
Ok(match fty {
FloatTy::F32 => self.binary_float_op(bin_op, left.to_f32()?, right.to_f32()?),
FloatTy::F64 => self.binary_float_op(bin_op, left.to_f64()?, right.to_f64()?),
FloatTy::F32 =>
self.binary_float_op(bin_op, ty, left.to_f32()?, right.to_f32()?),
FloatTy::F64 =>
self.binary_float_op(bin_op, ty, left.to_f64()?, right.to_f64()?),
})
}
_ if left.layout.ty.is_integral() => {
Expand Down Expand Up @@ -312,11 +325,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

/// Typed version of `checked_binary_op`, returning an `ImmTy`. Also ignores overflows.
#[inline]
pub fn binary_op(
&self,
bin_op: mir::BinOp,
left: ImmTy<'tcx, M::PointerTag>,
right: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, ImmTy<'tcx, M::PointerTag>> {
let (val, _overflow, ty) = self.overflowing_binary_op(bin_op, left, right)?;
Ok(ImmTy::from_scalar(val, self.layout_of(ty)?))
}

pub fn unary_op(
&self,
un_op: mir::UnOp,
val: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, Scalar<M::PointerTag>> {
) -> InterpResult<'tcx, ImmTy<'tcx, M::PointerTag>> {
use rustc::mir::UnOp::*;

let layout = val.layout;
Expand All @@ -330,15 +355,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Not => !val,
_ => bug!("Invalid bool op {:?}", un_op)
};
Ok(Scalar::from_bool(res))
Ok(ImmTy::from_scalar(Scalar::from_bool(res), self.layout_of(self.tcx.types.bool)?))
}
ty::Float(fty) => {
let res = match (un_op, fty) {
(Neg, FloatTy::F32) => Scalar::from_f32(-val.to_f32()?),
(Neg, FloatTy::F64) => Scalar::from_f64(-val.to_f64()?),
_ => bug!("Invalid float op {:?}", un_op)
};
Ok(res)
Ok(ImmTy::from_scalar(res, layout))
}
_ => {
assert!(layout.ty.is_integral());
Expand All @@ -351,7 +376,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};
// res needs tuncating
Ok(Scalar::from_uint(self.truncate(res, layout), layout.size))
Ok(ImmTy::from_uint(self.truncate(res, layout), layout))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub enum Place<Tag=(), Id=AllocId> {

#[derive(Copy, Clone, Debug)]
pub struct PlaceTy<'tcx, Tag=()> {
place: Place<Tag>,
place: Place<Tag>, // Keep this private, it helps enforce invariants
pub layout: TyLayout<'tcx>,
}

Expand Down
Loading