Skip to content

Commit

Permalink
Promote unchecked_add/sub/mul/shl/shr to mir::BinOp
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottmcm
scottmcm committed Jun 4, 2023
1 parent 9eee230 commit bd71c26
Show file tree
Hide file tree
Showing 42 changed files with 433 additions and 228 deletions.
9 changes: 5 additions & 4 deletions compiler/rustc_codegen_cranelift/src/codegen_i128.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ pub(crate) fn maybe_codegen<'tcx>(

match bin_op {
BinOp::BitAnd | BinOp::BitOr | BinOp::BitXor => None,
BinOp::Add | BinOp::Sub => None,
BinOp::Mul => {
BinOp::Add | BinOp::AddUnchecked | BinOp::Sub | BinOp::SubUnchecked => None,
BinOp::Mul | BinOp::MulUnchecked => {
let args = [lhs.load_scalar(fx), rhs.load_scalar(fx)];
let ret_val = fx.lib_call(
"__multi3",
Expand Down Expand Up @@ -69,7 +69,7 @@ pub(crate) fn maybe_codegen<'tcx>(
}
}
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne => None,
BinOp::Shl | BinOp::Shr => None,
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => None,
}
}

Expand Down Expand Up @@ -131,9 +131,10 @@ pub(crate) fn maybe_codegen_checked<'tcx>(
fx.lib_call(name, param_types, vec![], &args);
Some(out_place.to_cvalue(fx))
}
BinOp::AddUnchecked | BinOp::SubUnchecked | BinOp::MulUnchecked => unreachable!(),
BinOp::Offset => unreachable!("offset should only be used on pointers, not 128bit ints"),
BinOp::Div | BinOp::Rem => unreachable!(),
BinOp::Lt | BinOp::Le | BinOp::Eq | BinOp::Ge | BinOp::Gt | BinOp::Ne => unreachable!(),
BinOp::Shl | BinOp::Shr => unreachable!(),
BinOp::Shl | BinOp::ShlUnchecked | BinOp::Shr | BinOp::ShrUnchecked => unreachable!(),
}
}
16 changes: 10 additions & 6 deletions compiler/rustc_codegen_cranelift/src/num.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,11 @@ pub(crate) fn codegen_int_binop<'tcx>(
let rhs = in_rhs.load_scalar(fx);

let b = fx.bcx.ins();
// FIXME trap on overflow for the Unchecked versions
let val = match bin_op {
BinOp::Add => b.iadd(lhs, rhs),
BinOp::Sub => b.isub(lhs, rhs),
BinOp::Mul => b.imul(lhs, rhs),
BinOp::Add | BinOp::AddUnchecked => b.iadd(lhs, rhs),
BinOp::Sub | BinOp::SubUnchecked => b.isub(lhs, rhs),
BinOp::Mul | BinOp::MulUnchecked => b.imul(lhs, rhs),
BinOp::Div => {
if signed {
b.sdiv(lhs, rhs)
Expand All @@ -149,16 +150,19 @@ pub(crate) fn codegen_int_binop<'tcx>(
BinOp::BitXor => b.bxor(lhs, rhs),
BinOp::BitAnd => b.band(lhs, rhs),
BinOp::BitOr => b.bor(lhs, rhs),
BinOp::Shl => b.ishl(lhs, rhs),
BinOp::Shr => {
BinOp::Shl | BinOp::ShlUnchecked => b.ishl(lhs, rhs),
BinOp::Shr | BinOp::ShrUnchecked => {
if signed {
b.sshr(lhs, rhs)
} else {
b.ushr(lhs, rhs)
}
}
BinOp::Offset => unreachable!("Offset is not an integer operation"),
// Compare binops handles by `codegen_binop`.
_ => unreachable!("{:?}({:?}, {:?})", bin_op, in_lhs.layout().ty, in_rhs.layout().ty),
BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge => {
unreachable!("{:?}({:?}, {:?})", bin_op, in_lhs.layout().ty, in_rhs.layout().ty);
}
};

CValue::by_val(val, in_lhs.layout())
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub fn build_langcall<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// all shifts). For 32- and 64-bit types, this matches the semantics
// of Java. (See related discussion on #1877 and #10183.)

pub fn build_unchecked_lshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
pub fn build_masked_lshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
lhs: Bx::Value,
rhs: Bx::Value,
Expand All @@ -143,7 +143,7 @@ pub fn build_unchecked_lshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx.shl(lhs, rhs)
}

pub fn build_unchecked_rshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
pub fn build_masked_rshift<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &mut Bx,
lhs_t: Ty<'tcx>,
lhs: Bx::Value,
Expand Down
33 changes: 31 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,20 +798,41 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.add(lhs, rhs)
}
}
mir::BinOp::AddUnchecked => {
if is_signed {
bx.unchecked_sadd(lhs, rhs)
} else {
bx.unchecked_uadd(lhs, rhs)
}
}
mir::BinOp::Sub => {
if is_float {
bx.fsub(lhs, rhs)
} else {
bx.sub(lhs, rhs)
}
}
mir::BinOp::SubUnchecked => {
if is_signed {
bx.unchecked_ssub(lhs, rhs)
} else {
bx.unchecked_usub(lhs, rhs)
}
}
mir::BinOp::Mul => {
if is_float {
bx.fmul(lhs, rhs)
} else {
bx.mul(lhs, rhs)
}
}
mir::BinOp::MulUnchecked => {
if is_signed {
bx.unchecked_smul(lhs, rhs)
} else {
bx.unchecked_umul(lhs, rhs)
}
}
mir::BinOp::Div => {
if is_float {
bx.fdiv(lhs, rhs)
Expand Down Expand Up @@ -848,8 +869,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.inbounds_gep(llty, lhs, &[rhs])
}
}
mir::BinOp::Shl => common::build_unchecked_lshift(bx, lhs, rhs),
mir::BinOp::Shr => common::build_unchecked_rshift(bx, input_ty, lhs, rhs),
mir::BinOp::Shl => common::build_masked_lshift(bx, lhs, rhs),
mir::BinOp::ShlUnchecked => {
let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs);
bx.shl(lhs, rhs)
}
mir::BinOp::Shr => common::build_masked_rshift(bx, input_ty, lhs, rhs),
mir::BinOp::ShrUnchecked => {
let rhs = base::cast_shift_expr_rhs(bx, lhs, rhs);
if is_signed { bx.ashr(lhs, rhs) } else { bx.lshr(lhs, rhs) }
}
mir::BinOp::Ne
| mir::BinOp::Lt
| mir::BinOp::Gt
Expand Down
66 changes: 44 additions & 22 deletions compiler/rustc_const_eval/src/interpret/operator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ use rustc_middle::mir;
use rustc_middle::mir::interpret::{InterpResult, Scalar};
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::ty::{self, FloatTy, Ty};
use rustc_span::symbol::sym;
use rustc_target::abi::Abi;

use super::{ImmTy, Immediate, InterpCx, Machine, PlaceTy};

use crate::fluent_generated as fluent;

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Applies the binary operation `op` to the two operands and writes a tuple of the result
/// and a boolean signifying the potential overflow to the destination.
Expand Down Expand Up @@ -139,8 +142,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, (Scalar<M::Provenance>, bool, Ty<'tcx>)> {
use rustc_middle::mir::BinOp::*;

let throw_ub_on_overflow = match bin_op {
AddUnchecked => Some(sym::unchecked_add),
SubUnchecked => Some(sym::unchecked_sub),
MulUnchecked => Some(sym::unchecked_mul),
ShlUnchecked => Some(sym::unchecked_shl),
ShrUnchecked => Some(sym::unchecked_shr),
_ => None,
};

// Shift ops can have an RHS with a different numeric type.
if bin_op == Shl || bin_op == Shr {
if matches!(bin_op, Shl | ShlUnchecked | Shr | ShrUnchecked) {
let size = u128::from(left_layout.size.bits());
// Even if `r` is signed, we treat it as if it was unsigned (i.e., we use its
// zero-extended form). This matches the codegen backend:
Expand All @@ -155,6 +167,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// integers are maximally 128bits wide, so negative shifts *always* overflow and we have
// consistent results for the same value represented at different bit widths.
assert!(size <= 128);
let original_r = r;
let overflow = r >= size;
// The shift offset is implicitly masked to the type size, to make sure this operation
// is always defined. This is the one MIR operator that does *not* directly map to a
Expand All @@ -166,19 +179,28 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let result = if left_layout.abi.is_signed() {
let l = self.sign_extend(l, left_layout) as i128;
let result = match bin_op {
Shl => l.checked_shl(r).unwrap(),
Shr => l.checked_shr(r).unwrap(),
Shl | ShlUnchecked => l.checked_shl(r).unwrap(),
Shr | ShrUnchecked => l.checked_shr(r).unwrap(),
_ => bug!(),
};
result as u128
} else {
match bin_op {
Shl => l.checked_shl(r).unwrap(),
Shr => l.checked_shr(r).unwrap(),
Shl | ShlUnchecked => l.checked_shl(r).unwrap(),
Shr | ShrUnchecked => l.checked_shr(r).unwrap(),
_ => bug!(),
}
};
let truncated = self.truncate(result, left_layout);

if overflow && let Some(intrinsic_name) = throw_ub_on_overflow {
throw_ub_custom!(
fluent::const_eval_overflow_shift,
val = original_r,
name = intrinsic_name
);
}

return Ok((Scalar::from_uint(truncated, left_layout.size), overflow, left_layout.ty));
}

Expand Down Expand Up @@ -216,9 +238,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Rem if r == 0 => throw_ub!(RemainderByZero),
Div => Some(i128::overflowing_div),
Rem => Some(i128::overflowing_rem),
Add => Some(i128::overflowing_add),
Sub => Some(i128::overflowing_sub),
Mul => Some(i128::overflowing_mul),
Add | AddUnchecked => Some(i128::overflowing_add),
Sub | SubUnchecked => Some(i128::overflowing_sub),
Mul | MulUnchecked => Some(i128::overflowing_mul),
_ => None,
};
if let Some(op) = op {
Expand All @@ -242,11 +264,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// If that truncation loses any information, we have an overflow.
let result = result as u128;
let truncated = self.truncate(result, left_layout);
return Ok((
Scalar::from_uint(truncated, size),
oflo || self.sign_extend(truncated, left_layout) != result,
left_layout.ty,
));
let overflow = oflo || self.sign_extend(truncated, left_layout) != result;
if overflow && let Some(intrinsic_name) = throw_ub_on_overflow {
throw_ub_custom!(fluent::const_eval_overflow, name = intrinsic_name);
}
return Ok((Scalar::from_uint(truncated, size), overflow, left_layout.ty));
}
}

Expand All @@ -263,12 +285,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
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 => {
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Rem | Div => {
assert!(!left_layout.abi.is_signed());
let op: fn(u128, u128) -> (u128, bool) = match bin_op {
Add => u128::overflowing_add,
Sub => u128::overflowing_sub,
Mul => u128::overflowing_mul,
Add | AddUnchecked => u128::overflowing_add,
Sub | SubUnchecked => u128::overflowing_sub,
Mul | MulUnchecked => u128::overflowing_mul,
Div if r == 0 => throw_ub!(DivisionByZero),
Rem if r == 0 => throw_ub!(RemainderByZero),
Div => u128::overflowing_div,
Expand All @@ -279,11 +301,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Truncate to target type.
// If that truncation loses any information, we have an overflow.
let truncated = self.truncate(result, left_layout);
return Ok((
Scalar::from_uint(truncated, size),
oflo || truncated != result,
left_layout.ty,
));
let overflow = oflo || truncated != result;
if overflow && let Some(intrinsic_name) = throw_ub_on_overflow {
throw_ub_custom!(fluent::const_eval_overflow, name = intrinsic_name);
}
return Ok((Scalar::from_uint(truncated, size), overflow, left_layout.ty));
}

_ => span_bug!(
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_const_eval/src/interpret/step.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use super::{ImmTy, InterpCx, Machine};
fn binop_left_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Offset | Shl | Shr => true,
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true,
Eq | Ne | Lt | Le | Gt | Ge => false,
}
}
Expand All @@ -26,8 +27,9 @@ fn binop_left_homogeneous(op: mir::BinOp) -> bool {
fn binop_right_homogeneous(op: mir::BinOp) -> bool {
use rustc_middle::mir::BinOp::*;
match op {
Add | Sub | Mul | Div | Rem | BitXor | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true,
Offset | Shl | Shr => false,
Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor
| BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true,
Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => false,
}
}

Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_const_eval/src/transform/promote_consts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -572,13 +572,18 @@ impl<'tcx> Validator<'_, 'tcx> {
| BinOp::Gt
| BinOp::Offset
| BinOp::Add
| BinOp::AddUnchecked
| BinOp::Sub
| BinOp::SubUnchecked
| BinOp::Mul
| BinOp::MulUnchecked
| BinOp::BitXor
| BinOp::BitAnd
| BinOp::BitOr
| BinOp::Shl
| BinOp::Shr => {}
| BinOp::ShlUnchecked
| BinOp::Shr
| BinOp::ShrUnchecked => {}
}

self.validate_operand(lhs)?;
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
}
Shl | Shr => {
Shl | ShlUnchecked | Shr | ShrUnchecked => {
for x in [a, b] {
check_kinds!(
x,
Expand Down Expand Up @@ -601,6 +601,24 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
);
}
}
AddUnchecked | SubUnchecked | MulUnchecked => {
for x in [a, b] {
check_kinds!(
x,
"Cannot perform unchecked arithmetic on type {:?}",
ty::Uint(..) | ty::Int(..)
)
}
if a != b {
self.fail(
location,
format!(
"Cannot perform unchecked arithmetic on unequal types {:?} and {:?}",
a, b
),
);
}
}
}
}
Rvalue::CheckedBinaryOp(op, vals) => {
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_middle/src/mir/syntax.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1267,10 +1267,16 @@ pub enum UnOp {
pub enum BinOp {
/// The `+` operator (addition)
Add,
/// Like `Add`, but with UB on overflow. (Integers only.)
AddUnchecked,
/// The `-` operator (subtraction)
Sub,
/// Like `Sub`, but with UB on overflow. (Integers only.)
SubUnchecked,
/// The `*` operator (multiplication)
Mul,
/// Like `Mul`, but with UB on overflow. (Integers only.)
MulUnchecked,
/// The `/` operator (division)
///
/// For integer types, division by zero is UB, as is `MIN / -1` for signed.
Expand All @@ -1296,10 +1302,17 @@ pub enum BinOp {
///
/// The offset is truncated to the size of the first operand before shifting.
Shl,
/// Like `Shl`, but is UB if the RHS >= LHS::BITS
ShlUnchecked,
/// The `>>` operator (shift right)
///
/// The offset is truncated to the size of the first operand before shifting.
///
/// This is an arithmetic shift if the LHS is signed
/// and a logical shift if the LHS is unsigned.
Shr,
/// Like `Shl`, but is UB if the RHS >= LHS::BITS
ShrUnchecked,
/// The `==` operator (equality)
Eq,
/// The `<` operator (less than)
Expand Down
Loading

0 comments on commit bd71c26

Please sign in to comment.