Skip to content
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

Simpler checked shifts in MIR building #109475

Merged
merged 2 commits into from
Mar 23, 2023
Merged

Simpler checked shifts in MIR building #109475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec25f08
Demonstration test for MIR building of checked shift operators
scottmcm Mar 22, 2023
b537e6b
Generate simpler MIR for shifts
scottmcm Mar 22, 2023
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
56 changes: 33 additions & 23 deletions compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,41 +566,51 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Rvalue::Use(Operand::Move(val))
}
BinOp::Shl | BinOp::Shr if self.check_overflow && ty.is_integral() => {
// Consider that the shift overflows if `rhs < 0` or `rhs >= bits`.
// This can be encoded as a single operation as `(rhs & -bits) != 0`.
let (size, _) = ty.int_size_and_signed(self.tcx);
let bits = size.bits();
debug_assert!(bits.is_power_of_two());
let mask = !((bits - 1) as u128);

// For an unsigned RHS, the shift is in-range for `rhs < bits`.
// For a signed RHS, `IntToInt` cast to the equivalent unsigned
// type and do that same comparison. Because the type is the
// same size, there's no negative shift amount that ends up
// overlapping with valid ones, thus it catches negatives too.
let (lhs_size, _) = ty.int_size_and_signed(self.tcx);
let rhs_ty = rhs.ty(&self.local_decls, self.tcx);
let (rhs_size, _) = rhs_ty.int_size_and_signed(self.tcx);
let mask = Operand::const_from_scalar(

let (unsigned_rhs, unsigned_ty) = match rhs_ty.kind() {
ty::Uint(_) => (rhs.to_copy(), rhs_ty),
ty::Int(int_width) => {
let uint_ty = self.tcx.mk_mach_uint(int_width.to_unsigned());
let rhs_temp = self.temp(uint_ty, span);
self.cfg.push_assign(
block,
source_info,
rhs_temp,
Rvalue::Cast(CastKind::IntToInt, rhs.to_copy(), uint_ty),
);
(Operand::Move(rhs_temp), uint_ty)
}
_ => unreachable!("only integers are shiftable"),
};

// This can't overflow because the largest shiftable types are 128-bit,
// which fits in `u8`, the smallest possible `unsigned_ty`.
// (And `from_uint` will `bug!` if that's ever no longer true.)
let lhs_bits = Operand::const_from_scalar(
self.tcx,
rhs_ty,
Scalar::from_uint(rhs_size.truncate(mask), rhs_size),
unsigned_ty,
Scalar::from_uint(lhs_size.bits(), rhs_size),
span,
);

let outer_bits = self.temp(rhs_ty, span);
self.cfg.push_assign(
block,
source_info,
outer_bits,
Rvalue::BinaryOp(BinOp::BitAnd, Box::new((rhs.to_copy(), mask))),
);

let overflows = self.temp(bool_ty, span);
let zero = self.zero_literal(span, rhs_ty);
let inbounds = self.temp(bool_ty, span);
self.cfg.push_assign(
block,
source_info,
overflows,
Rvalue::BinaryOp(BinOp::Ne, Box::new((Operand::Move(outer_bits), zero))),
inbounds,
Rvalue::BinaryOp(BinOp::Lt, Box::new((unsigned_rhs, lhs_bits))),
);

let overflow_err = AssertKind::Overflow(op, lhs.to_copy(), rhs.to_copy());
block = self.assert(block, Operand::Move(overflows), false, overflow_err, span);
block = self.assert(block, Operand::Move(inbounds), true, overflow_err, span);
Rvalue::BinaryOp(op, Box::new((lhs, rhs)))
}
BinOp::Div | BinOp::Rem if ty.is_integral() => {
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_type_ir/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,17 @@ impl IntTy {
_ => *self,
}
}

pub fn to_unsigned(self) -> UintTy {
match self {
IntTy::Isize => UintTy::Usize,
IntTy::I8 => UintTy::U8,
IntTy::I16 => UintTy::U16,
IntTy::I32 => UintTy::U32,
IntTy::I64 => UintTy::U64,
IntTy::I128 => UintTy::U128,
}
}
}

#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Debug)]
Expand Down Expand Up @@ -479,6 +490,17 @@ impl UintTy {
_ => *self,
}
}

pub fn to_signed(self) -> IntTy {
match self {
UintTy::Usize => IntTy::Isize,
UintTy::U8 => IntTy::I8,
UintTy::U16 => IntTy::I16,
UintTy::U32 => IntTy::I32,
UintTy::U64 => IntTy::I64,
UintTy::U128 => IntTy::I128,
}
}
}

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
Expand Down
20 changes: 20 additions & 0 deletions tests/mir-opt/building/shifts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// compile-flags: -C debug-assertions=yes

// EMIT_MIR shifts.shift_signed.built.after.mir
fn shift_signed(small: i8, big: u128, a: i8, b: i32, c: i128) -> ([i8; 3], [u128; 3]) {
(
[small >> a, small >> b, small >> c],
[big << a, big << b, big << c],
)
}

// EMIT_MIR shifts.shift_unsigned.built.after.mir
fn shift_unsigned(small: u8, big: i128, a: u8, b: u32, c: u128) -> ([u8; 3], [i128; 3]) {
(
[small >> a, small >> b, small >> c],
[big << a, big << b, big << c],
)
}

fn main() {
}
147 changes: 147 additions & 0 deletions tests/mir-opt/building/shifts.shift_signed.built.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// MIR for `shift_signed` after built

fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128; 3]) {
debug small => _1; // in scope 0 at $DIR/shifts.rs:+0:17: +0:22
debug big => _2; // in scope 0 at $DIR/shifts.rs:+0:28: +0:31
debug a => _3; // in scope 0 at $DIR/shifts.rs:+0:39: +0:40
debug b => _4; // in scope 0 at $DIR/shifts.rs:+0:46: +0:47
debug c => _5; // in scope 0 at $DIR/shifts.rs:+0:54: +0:55
let mut _0: ([i8; 3], [u128; 3]); // return place in scope 0 at $DIR/shifts.rs:+0:66: +0:86
let mut _6: [i8; 3]; // in scope 0 at $DIR/shifts.rs:+2:9: +2:45
let mut _7: i8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
let mut _8: i8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:15
let mut _9: i8; // in scope 0 at $DIR/shifts.rs:+2:19: +2:20
let mut _10: u8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
let mut _11: bool; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
let mut _12: i8; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
let mut _13: i8; // in scope 0 at $DIR/shifts.rs:+2:22: +2:27
let mut _14: i32; // in scope 0 at $DIR/shifts.rs:+2:31: +2:32
let mut _15: u32; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
let mut _16: bool; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
let mut _17: i8; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
let mut _18: i8; // in scope 0 at $DIR/shifts.rs:+2:34: +2:39
let mut _19: i128; // in scope 0 at $DIR/shifts.rs:+2:43: +2:44
let mut _20: u128; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
let mut _21: bool; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
let mut _22: [u128; 3]; // in scope 0 at $DIR/shifts.rs:+3:9: +3:39
let mut _23: u128; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
let mut _24: u128; // in scope 0 at $DIR/shifts.rs:+3:10: +3:13
let mut _25: i8; // in scope 0 at $DIR/shifts.rs:+3:17: +3:18
let mut _26: u8; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
let mut _27: bool; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
let mut _28: u128; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
let mut _29: u128; // in scope 0 at $DIR/shifts.rs:+3:20: +3:23
let mut _30: i32; // in scope 0 at $DIR/shifts.rs:+3:27: +3:28
let mut _31: u32; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
let mut _32: bool; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
let mut _33: u128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38
let mut _34: u128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:33
let mut _35: i128; // in scope 0 at $DIR/shifts.rs:+3:37: +3:38
let mut _36: u128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38
let mut _37: bool; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38

bb0: {
StorageLive(_6); // scope 0 at $DIR/shifts.rs:+2:9: +2:45
StorageLive(_7); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
StorageLive(_8); // scope 0 at $DIR/shifts.rs:+2:10: +2:15
_8 = _1; // scope 0 at $DIR/shifts.rs:+2:10: +2:15
StorageLive(_9); // scope 0 at $DIR/shifts.rs:+2:19: +2:20
_9 = _3; // scope 0 at $DIR/shifts.rs:+2:19: +2:20
_10 = _9 as u8 (IntToInt); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
_11 = Lt(move _10, const 8_u8); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
assert(move _11, "attempt to shift right by `{}`, which would overflow", _9) -> [success: bb1, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+2:10: +2:20
}

bb1: {
_7 = Shr(move _8, move _9); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
StorageDead(_9); // scope 0 at $DIR/shifts.rs:+2:19: +2:20
StorageDead(_8); // scope 0 at $DIR/shifts.rs:+2:19: +2:20
StorageLive(_12); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
StorageLive(_13); // scope 0 at $DIR/shifts.rs:+2:22: +2:27
_13 = _1; // scope 0 at $DIR/shifts.rs:+2:22: +2:27
StorageLive(_14); // scope 0 at $DIR/shifts.rs:+2:31: +2:32
_14 = _4; // scope 0 at $DIR/shifts.rs:+2:31: +2:32
_15 = _14 as u32 (IntToInt); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
_16 = Lt(move _15, const 8_u32); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
assert(move _16, "attempt to shift right by `{}`, which would overflow", _14) -> [success: bb2, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+2:22: +2:32
}

bb2: {
_12 = Shr(move _13, move _14); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
StorageDead(_14); // scope 0 at $DIR/shifts.rs:+2:31: +2:32
StorageDead(_13); // scope 0 at $DIR/shifts.rs:+2:31: +2:32
StorageLive(_17); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
StorageLive(_18); // scope 0 at $DIR/shifts.rs:+2:34: +2:39
_18 = _1; // scope 0 at $DIR/shifts.rs:+2:34: +2:39
StorageLive(_19); // scope 0 at $DIR/shifts.rs:+2:43: +2:44
_19 = _5; // scope 0 at $DIR/shifts.rs:+2:43: +2:44
_20 = _19 as u128 (IntToInt); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
_21 = Lt(move _20, const 8_u128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
assert(move _21, "attempt to shift right by `{}`, which would overflow", _19) -> [success: bb3, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+2:34: +2:44
}

bb3: {
_17 = Shr(move _18, move _19); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
StorageDead(_19); // scope 0 at $DIR/shifts.rs:+2:43: +2:44
StorageDead(_18); // scope 0 at $DIR/shifts.rs:+2:43: +2:44
_6 = [move _7, move _12, move _17]; // scope 0 at $DIR/shifts.rs:+2:9: +2:45
StorageDead(_17); // scope 0 at $DIR/shifts.rs:+2:44: +2:45
StorageDead(_12); // scope 0 at $DIR/shifts.rs:+2:44: +2:45
StorageDead(_7); // scope 0 at $DIR/shifts.rs:+2:44: +2:45
StorageLive(_22); // scope 0 at $DIR/shifts.rs:+3:9: +3:39
StorageLive(_23); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
StorageLive(_24); // scope 0 at $DIR/shifts.rs:+3:10: +3:13
_24 = _2; // scope 0 at $DIR/shifts.rs:+3:10: +3:13
StorageLive(_25); // scope 0 at $DIR/shifts.rs:+3:17: +3:18
_25 = _3; // scope 0 at $DIR/shifts.rs:+3:17: +3:18
_26 = _25 as u8 (IntToInt); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
_27 = Lt(move _26, const 128_u8); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
assert(move _27, "attempt to shift left by `{}`, which would overflow", _25) -> [success: bb4, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+3:10: +3:18
}

bb4: {
_23 = Shl(move _24, move _25); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
StorageDead(_25); // scope 0 at $DIR/shifts.rs:+3:17: +3:18
StorageDead(_24); // scope 0 at $DIR/shifts.rs:+3:17: +3:18
StorageLive(_28); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
StorageLive(_29); // scope 0 at $DIR/shifts.rs:+3:20: +3:23
_29 = _2; // scope 0 at $DIR/shifts.rs:+3:20: +3:23
StorageLive(_30); // scope 0 at $DIR/shifts.rs:+3:27: +3:28
_30 = _4; // scope 0 at $DIR/shifts.rs:+3:27: +3:28
_31 = _30 as u32 (IntToInt); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
_32 = Lt(move _31, const 128_u32); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
assert(move _32, "attempt to shift left by `{}`, which would overflow", _30) -> [success: bb5, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+3:20: +3:28
}

bb5: {
_28 = Shl(move _29, move _30); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
StorageDead(_30); // scope 0 at $DIR/shifts.rs:+3:27: +3:28
StorageDead(_29); // scope 0 at $DIR/shifts.rs:+3:27: +3:28
StorageLive(_33); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
StorageLive(_34); // scope 0 at $DIR/shifts.rs:+3:30: +3:33
_34 = _2; // scope 0 at $DIR/shifts.rs:+3:30: +3:33
StorageLive(_35); // scope 0 at $DIR/shifts.rs:+3:37: +3:38
_35 = _5; // scope 0 at $DIR/shifts.rs:+3:37: +3:38
_36 = _35 as u128 (IntToInt); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
_37 = Lt(move _36, const 128_u128); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
assert(move _37, "attempt to shift left by `{}`, which would overflow", _35) -> [success: bb6, unwind: bb7]; // scope 0 at $DIR/shifts.rs:+3:30: +3:38
}

bb6: {
_33 = Shl(move _34, move _35); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
StorageDead(_35); // scope 0 at $DIR/shifts.rs:+3:37: +3:38
StorageDead(_34); // scope 0 at $DIR/shifts.rs:+3:37: +3:38
_22 = [move _23, move _28, move _33]; // scope 0 at $DIR/shifts.rs:+3:9: +3:39
StorageDead(_33); // scope 0 at $DIR/shifts.rs:+3:38: +3:39
StorageDead(_28); // scope 0 at $DIR/shifts.rs:+3:38: +3:39
StorageDead(_23); // scope 0 at $DIR/shifts.rs:+3:38: +3:39
_0 = (move _6, move _22); // scope 0 at $DIR/shifts.rs:+1:5: +4:6
StorageDead(_22); // scope 0 at $DIR/shifts.rs:+4:5: +4:6
StorageDead(_6); // scope 0 at $DIR/shifts.rs:+4:5: +4:6
return; // scope 0 at $DIR/shifts.rs:+5:2: +5:2
}

bb7 (cleanup): {
resume; // scope 0 at $DIR/shifts.rs:+0:1: +5:2
}
}
Loading