Skip to content

Commit

Permalink
Remove Rvalue::CheckedBinaryOp
Browse files Browse the repository at this point in the history
  • Loading branch information
scottmcm committed May 16, 2024
1 parent ee97564 commit 7070071
Show file tree
Hide file tree
Showing 58 changed files with 207 additions and 194 deletions.
3 changes: 1 addition & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1312,8 +1312,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
}

Rvalue::BinaryOp(_bin_op, box (operand1, operand2))
| Rvalue::CheckedBinaryOp(_bin_op, box (operand1, operand2)) => {
Rvalue::BinaryOp(_bin_op, box (operand1, operand2)) => {
self.consume_operand(location, (operand1, span), flow_state);
self.consume_operand(location, (operand2, span), flow_state);
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_borrowck/src/polonius/loan_invalidations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,7 @@ impl<'cx, 'tcx> LoanInvalidationsGenerator<'cx, 'tcx> {
);
}

Rvalue::BinaryOp(_bin_op, box (operand1, operand2))
| Rvalue::CheckedBinaryOp(_bin_op, box (operand1, operand2)) => {
Rvalue::BinaryOp(_bin_op, box (operand1, operand2)) => {
self.consume_operand(location, operand1);
self.consume_operand(location, operand2);
}
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2417,8 +2417,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.check_operand(op, location);
}

Rvalue::BinaryOp(_, box (left, right))
| Rvalue::CheckedBinaryOp(_, box (left, right)) => {
Rvalue::BinaryOp(_, box (left, right)) => {
self.check_operand(left, location);
self.check_operand(right, location);
}
Expand All @@ -2445,7 +2444,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| Rvalue::Cast(..)
| Rvalue::ShallowInitBox(..)
| Rvalue::BinaryOp(..)
| Rvalue::CheckedBinaryOp(..)
| Rvalue::NullaryOp(..)
| Rvalue::CopyForDeref(..)
| Rvalue::UnaryOp(..)
Expand Down
13 changes: 5 additions & 8 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,14 +576,11 @@ fn codegen_stmt<'tcx>(
let lhs = codegen_operand(fx, &lhs_rhs.0);
let rhs = codegen_operand(fx, &lhs_rhs.1);

let res = crate::num::codegen_binop(fx, bin_op, lhs, rhs);
lval.write_cvalue(fx, res);
}
Rvalue::CheckedBinaryOp(bin_op, ref lhs_rhs) => {
let lhs = codegen_operand(fx, &lhs_rhs.0);
let rhs = codegen_operand(fx, &lhs_rhs.1);

let res = crate::num::codegen_checked_int_binop(fx, bin_op, lhs, rhs);
let res = if let Some(bin_op) = bin_op.overflowing_to_wrapping() {
crate::num::codegen_checked_int_binop(fx, bin_op, lhs, rhs)
} else {
crate::num::codegen_binop(fx, bin_op, lhs, rhs)
};
lval.write_cvalue(fx, res);
}
Rvalue::UnaryOp(un_op, ref operand) => {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_cranelift/src/codegen_i128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub(crate) fn maybe_codegen<'tcx>(
}
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne | BinOp::Cmp => None,
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => None,
BinOp::AddWithOverflow | BinOp::SubWithOverflow | BinOp::MulWithOverflow => unreachable!(),
}
}

Expand Down Expand Up @@ -132,6 +133,7 @@ pub(crate) fn maybe_codegen_checked<'tcx>(
Some(out_place.to_cvalue(fx))
}
BinOp::AddUnchecked | BinOp::SubUnchecked | BinOp::MulUnchecked => unreachable!(),
BinOp::AddWithOverflow | BinOp::SubWithOverflow | BinOp::MulWithOverflow => unreachable!(),
BinOp::Offset => unreachable!("offset should only be used on pointers, not 128bit ints"),
BinOp::Div | BinOp::Rem => unreachable!(),
BinOp::Cmp => unreachable!(),
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_codegen_cranelift/src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@ pub(crate) fn codegen_int_binop<'tcx>(
}
BinOp::Offset => unreachable!("Offset is not an integer operation"),
// Compare binops handles by `codegen_binop`.
BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge | BinOp::Cmp => {
// Overflow binops handled by `codegen_checked_int_binop`
BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge | BinOp::Cmp
| BinOp::AddWithOverflow | BinOp::SubWithOverflow | BinOp::MulWithOverflow => {
unreachable!("{:?}({:?}, {:?})", bin_op, in_lhs.layout().ty, in_rhs.layout().ty);
}
};
Expand Down
38 changes: 22 additions & 16 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
}

mir::Rvalue::BinaryOp(op_with_overflow, box (ref lhs, ref rhs))
if let Some(op) = op_with_overflow.overflowing_to_wrapping() =>
{
let lhs = self.codegen_operand(bx, lhs);
let rhs = self.codegen_operand(bx, rhs);
let result = self.codegen_scalar_checked_binop(
bx,
op,
lhs.immediate(),
rhs.immediate(),
lhs.layout.ty,
);
let val_ty = op.ty(bx.tcx(), lhs.layout.ty, rhs.layout.ty);
let operand_ty = Ty::new_tup(bx.tcx(), &[val_ty, bx.tcx().types.bool]);
OperandRef { val: result, layout: bx.cx().layout_of(operand_ty) }
}
mir::Rvalue::BinaryOp(op, box (ref lhs, ref rhs)) => {
let lhs = self.codegen_operand(bx, lhs);
let rhs = self.codegen_operand(bx, rhs);
Expand Down Expand Up @@ -600,20 +616,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
layout: bx.cx().layout_of(op.ty(bx.tcx(), lhs.layout.ty, rhs.layout.ty)),
}
}
mir::Rvalue::CheckedBinaryOp(op, box (ref lhs, ref rhs)) => {
let lhs = self.codegen_operand(bx, lhs);
let rhs = self.codegen_operand(bx, rhs);
let result = self.codegen_scalar_checked_binop(
bx,
op,
lhs.immediate(),
rhs.immediate(),
lhs.layout.ty,
);
let val_ty = op.ty(bx.tcx(), lhs.layout.ty, rhs.layout.ty);
let operand_ty = Ty::new_tup(bx.tcx(), &[val_ty, bx.tcx().types.bool]);
OperandRef { val: result, layout: bx.cx().layout_of(operand_ty) }
}

mir::Rvalue::UnaryOp(op, ref operand) => {
let operand = self.codegen_operand(bx, operand);
Expand Down Expand Up @@ -792,7 +794,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
debug_assert!(
if bx.cx().type_has_metadata(ty) {
matches!(val, OperandValue::Pair(..))
} else {
} else {
matches!(val, OperandValue::Immediate(..))
},
"Address of place was unexpectedly {val:?} for pointee type {ty:?}",
Expand Down Expand Up @@ -938,6 +940,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.select(is_lt, bx.cx().const_i8(Ordering::Less as i8), ge)
}
}
mir::BinOp::AddWithOverflow
| mir::BinOp::SubWithOverflow
| mir::BinOp::MulWithOverflow => {
bug!("{op:?} needs to return a pair, so call codegen_scalar_checked_binop instead")
}
}
}

Expand Down Expand Up @@ -1050,7 +1057,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::Rvalue::Cast(..) | // (*)
mir::Rvalue::ShallowInitBox(..) | // (*)
mir::Rvalue::BinaryOp(..) |
mir::Rvalue::CheckedBinaryOp(..) |
mir::Rvalue::UnaryOp(..) |
mir::Rvalue::Discriminant(..) |
mir::Rvalue::NullaryOp(..) |
Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let left = self.read_immediate(&self.eval_operand(left, layout)?)?;
let layout = util::binop_right_homogeneous(bin_op).then_some(left.layout);
let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
self.binop_ignore_overflow(bin_op, &left, &right, &dest)?;
}

CheckedBinaryOp(bin_op, box (ref left, ref right)) => {
// 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 layout = util::binop_right_homogeneous(bin_op).then_some(left.layout);
let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
self.binop_with_overflow(bin_op, &left, &right, &dest)?;
if let Some(bin_op) = bin_op.overflowing_to_wrapping() {
self.binop_with_overflow(bin_op, &left, &right, &dest)?;
} else {
self.binop_ignore_overflow(bin_op, &left, &right, &dest)?;
}
}

UnaryOp(un_op, ref operand) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
}
}

Rvalue::BinaryOp(op, box (lhs, rhs)) | Rvalue::CheckedBinaryOp(op, box (lhs, rhs)) => {
Rvalue::BinaryOp(op, box (lhs, rhs)) => {
let lhs_ty = lhs.ty(self.body, self.tcx);
let rhs_ty = rhs.ty(self.body, self.tcx);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ where
| Rvalue::Cast(_, operand, _)
| Rvalue::ShallowInitBox(operand, _) => in_operand::<Q, _>(cx, in_local, operand),

Rvalue::BinaryOp(_, box (lhs, rhs)) | Rvalue::CheckedBinaryOp(_, box (lhs, rhs)) => {
Rvalue::BinaryOp(_, box (lhs, rhs)) => {
in_operand::<Q, _>(cx, in_local, lhs) || in_operand::<Q, _>(cx, in_local, rhs)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ where
| mir::Rvalue::Repeat(..)
| mir::Rvalue::Len(..)
| mir::Rvalue::BinaryOp(..)
| mir::Rvalue::CheckedBinaryOp(..)
| mir::Rvalue::NullaryOp(..)
| mir::Rvalue::UnaryOp(..)
| mir::Rvalue::Discriminant(..)
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,14 +1064,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
)
}
}
}
}
Rvalue::CheckedBinaryOp(op, vals) => {
use BinOp::*;
let a = vals.0.ty(&self.body.local_decls, self.tcx);
let b = vals.1.ty(&self.body.local_decls, self.tcx);
match op {
Add | Sub | Mul => {
AddWithOverflow | SubWithOverflow | MulWithOverflow => {
for x in [a, b] {
check_kinds!(
x,
Expand All @@ -1088,7 +1081,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
}
_ => self.fail(location, format!("There is no checked version of {op:?}")),
}
}
Rvalue::UnaryOp(op, operand) => {
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_const_eval/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true,
Eq | Ne | Lt | Le | Gt | Ge | Cmp => false,
AddWithOverflow | SubWithOverflow | MulWithOverflow | Eq | Ne | Lt | Le | Gt | Ge | Cmp => {
false
}
}
}

Expand All @@ -29,8 +31,9 @@ pub fn binop_left_homogeneous(op: mir::BinOp) -> bool {
pub fn binop_right_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge | Cmp => true,
Add | AddUnchecked | AddWithOverflow | Sub | SubUnchecked | SubWithOverflow | Mul
| MulUnchecked | MulWithOverflow | Div | Rem | BitXor | BitAnd | BitOr | Eq | Ne | Lt
| Le | Gt | Ge | Cmp => true,
Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => false,
}
}
3 changes: 0 additions & 3 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,9 +971,6 @@ impl<'tcx> Debug for Rvalue<'tcx> {
with_no_trimmed_paths!(write!(fmt, "{place:?} as {ty} ({kind:?})"))
}
BinaryOp(ref op, box (ref a, ref b)) => write!(fmt, "{op:?}({a:?}, {b:?})"),
CheckedBinaryOp(ref op, box (ref a, ref b)) => {
write!(fmt, "Checked{op:?}({a:?}, {b:?})")
}
UnaryOp(ref op, ref a) => write!(fmt, "{op:?}({a:?})"),
Discriminant(ref place) => write!(fmt, "discriminant({place:?})"),
NullaryOp(ref op, ref t) => {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,6 @@ impl<'tcx> Rvalue<'tcx> {
_,
)
| Rvalue::BinaryOp(_, _)
| Rvalue::CheckedBinaryOp(_, _)
| Rvalue::NullaryOp(_, _)
| Rvalue::UnaryOp(_, _)
| Rvalue::Discriminant(_)
Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1295,18 +1295,12 @@ pub enum Rvalue<'tcx> {
/// truncated as needed.
/// * The `Bit*` operations accept signed integers, unsigned integers, or bools with matching
/// types and return a value of that type.
/// * The `FooWithOverflow` are like the `Foo`, but returning `(T, bool)` instead of just `T`,
/// where the `bool` is true if the result is not equal to the infinite-precision result.
/// * The remaining operations accept signed integers, unsigned integers, or floats with
/// matching types and return a value of that type.
BinaryOp(BinOp, Box<(Operand<'tcx>, Operand<'tcx>)>),

/// Same as `BinaryOp`, but yields `(T, bool)` with a `bool` indicating an error condition.
///
/// For addition, subtraction, and multiplication on integers the error condition is set when
/// the infinite precision result would not be equal to the actual result.
///
/// Other combinations of types and operators are unsupported.
CheckedBinaryOp(BinOp, Box<(Operand<'tcx>, Operand<'tcx>)>),

/// Computes a value as described by the operation.
NullaryOp(NullOp<'tcx>, Ty<'tcx>),

Expand Down Expand Up @@ -1449,14 +1443,23 @@ pub enum BinOp {
Add,
/// Like `Add`, but with UB on overflow. (Integers only.)
AddUnchecked,
/// Like `Add`, but returns `(T, bool)` of both the wrapped result
/// and a bool indicating that it overflowed.
AddWithOverflow,
/// The `-` operator (subtraction)
Sub,
/// Like `Sub`, but with UB on overflow. (Integers only.)
SubUnchecked,
/// Like `Sub`, but returns `(T, bool)` of both the wrapped result
/// and a bool indicating that it overflowed.
SubWithOverflow,
/// The `*` operator (multiplication)
Mul,
/// Like `Mul`, but with UB on overflow. (Integers only.)
MulUnchecked,
/// Like `Mul`, but returns `(T, bool)` of both the wrapped result
/// and a bool indicating that it overflowed.
MulWithOverflow,
/// The `/` operator (division)
///
/// For integer types, division by zero is UB, as is `MIN / -1` for signed.
Expand Down
34 changes: 28 additions & 6 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,6 @@ impl<'tcx> Rvalue<'tcx> {
let rhs_ty = rhs.ty(local_decls, tcx);
op.ty(tcx, lhs_ty, rhs_ty)
}
Rvalue::CheckedBinaryOp(op, box (ref lhs, ref rhs)) => {
let lhs_ty = lhs.ty(local_decls, tcx);
let rhs_ty = rhs.ty(local_decls, tcx);
let ty = op.ty(tcx, lhs_ty, rhs_ty);
Ty::new_tup(tcx, &[ty, tcx.types.bool])
}
Rvalue::UnaryOp(UnOp::Not | UnOp::Neg, ref operand) => operand.ty(local_decls, tcx),
Rvalue::Discriminant(ref place) => place.ty(local_decls, tcx).ty.discriminant_ty(tcx),
Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf | NullOp::OffsetOf(..), _) => {
Expand Down Expand Up @@ -263,6 +257,11 @@ impl<'tcx> BinOp {
assert_eq!(lhs_ty, rhs_ty);
lhs_ty
}
&BinOp::AddWithOverflow | &BinOp::SubWithOverflow | &BinOp::MulWithOverflow => {
// these should be integers of the same size.
assert_eq!(lhs_ty, rhs_ty);
Ty::new_tup(tcx, &[lhs_ty, tcx.types.bool])
}
&BinOp::Shl
| &BinOp::ShlUnchecked
| &BinOp::Shr
Expand Down Expand Up @@ -315,6 +314,9 @@ impl BinOp {
BinOp::Le => hir::BinOpKind::Le,
BinOp::Ge => hir::BinOpKind::Ge,
BinOp::Cmp
| BinOp::AddWithOverflow
| BinOp::SubWithOverflow
| BinOp::MulWithOverflow
| BinOp::AddUnchecked
| BinOp::SubUnchecked
| BinOp::MulUnchecked
Expand All @@ -325,4 +327,24 @@ impl BinOp {
}
}
}

/// If this is a `FooWithOverflow`, return `Some(Foo)`.
pub fn overflowing_to_wrapping(self) -> Option<BinOp> {
Some(match self {
BinOp::AddWithOverflow => BinOp::Add,
BinOp::SubWithOverflow => BinOp::Sub,
BinOp::MulWithOverflow => BinOp::Mul,
_ => return None,
})
}

/// If this is a `Foo`, return `Some(FooWithOverflow)`.
pub fn wrapping_to_overflowing(self) -> Option<BinOp> {
Some(match self {
BinOp::Add => BinOp::AddWithOverflow,
BinOp::Sub => BinOp::SubWithOverflow,
BinOp::Mul => BinOp::MulWithOverflow,
_ => return None,
})
}
}
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,7 @@ macro_rules! make_mir_visitor {
self.visit_ty($(& $mutability)? *ty, TyContext::Location(location));
}

Rvalue::BinaryOp(_bin_op, box(lhs, rhs))
| Rvalue::CheckedBinaryOp(_bin_op, box(lhs, rhs)) => {
Rvalue::BinaryOp(_bin_op, box(lhs, rhs)) => {
self.visit_operand(lhs, location);
self.visit_operand(rhs, location);
}
Expand Down
Loading

0 comments on commit 7070071

Please sign in to comment.