Skip to content

Remove Rvalue::CheckedBinaryOp #125173

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.

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 2 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@ -609,14 +609,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
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_cranelift/src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ pub(crate) fn codegen_int_binop<'tcx>(
}
}
BinOp::Offset => unreachable!("Offset is not an integer operation"),
BinOp::AddWithOverflow | BinOp::SubWithOverflow | BinOp::MulWithOverflow => {
unreachable!("Overflow binops handled by `codegen_checked_int_binop`")
}
// Compare binops handles by `codegen_binop`.
BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge | BinOp::Cmp => {
unreachable!("{:?}({:?}, {:?})", bin_op, in_lhs.layout().ty, in_rhs.layout().ty);
Expand Down
36 changes: 21 additions & 15 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 @@ -924,6 +926,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 @@ -1036,7 +1043,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 @@ -167,15 +167,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 @@ -580,7 +580,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 @@ -261,7 +261,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
29 changes: 2 additions & 27 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1037,8 +1037,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
)
}
}
AddUnchecked | SubUnchecked | MulUnchecked | Shl | ShlUnchecked | Shr
| ShrUnchecked => {
AddUnchecked | AddWithOverflow | SubUnchecked | SubWithOverflow
| MulUnchecked | MulWithOverflow | Shl | ShlUnchecked | Shr | ShrUnchecked => {
for x in [a, b] {
check_kinds!(
x,
Expand Down Expand Up @@ -1067,31 +1067,6 @@ 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 => {
for x in [a, b] {
check_kinds!(
x,
"Cannot perform checked arithmetic on type {:?}",
ty::Uint(..) | ty::Int(..)
)
}
if a != b {
self.fail(
location,
format!(
"Cannot perform checked arithmetic on unequal types {a:?} and {b:?}"
),
);
}
}
_ => self.fail(location, format!("There is no checked version of {op:?}")),
}
}
Rvalue::UnaryOp(op, operand) => {
let a = operand.ty(&self.body.local_decls, self.tcx);
match op {
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 whether 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 whether 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 whether 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
Loading