Skip to content

Commit b537e6b

Browse files
scottmcmscottmcm
committed
Generate simpler MIR for shifts
1 parent ec25f08 commit b537e6b

File tree

5 files changed

+175
-155
lines changed

5 files changed

+175
-155
lines changed

‎compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -566,41 +566,51 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
566566
Rvalue::Use(Operand::Move(val))
567567
}
568568
BinOp::Shl | BinOp::Shr if self.check_overflow && ty.is_integral() => {
569-
// Consider that the shift overflows if `rhs < 0` or `rhs >= bits`.
570-
// This can be encoded as a single operation as `(rhs & -bits) != 0`.
571-
let (size, _) = ty.int_size_and_signed(self.tcx);
572-
let bits = size.bits();
573-
debug_assert!(bits.is_power_of_two());
574-
let mask = !((bits - 1) as u128);
575-
569+
// For an unsigned RHS, the shift is in-range for `rhs < bits`.
570+
// For a signed RHS, `IntToInt` cast to the equivalent unsigned
571+
// type and do that same comparison. Because the type is the
572+
// same size, there's no negative shift amount that ends up
573+
// overlapping with valid ones, thus it catches negatives too.
574+
let (lhs_size, _) = ty.int_size_and_signed(self.tcx);
576575
let rhs_ty = rhs.ty(&self.local_decls, self.tcx);
577576
let (rhs_size, _) = rhs_ty.int_size_and_signed(self.tcx);
578-
let mask = Operand::const_from_scalar(
577+
578+
let (unsigned_rhs, unsigned_ty) = match rhs_ty.kind() {
579+
ty::Uint(_) => (rhs.to_copy(), rhs_ty),
580+
ty::Int(int_width) => {
581+
let uint_ty = self.tcx.mk_mach_uint(int_width.to_unsigned());
582+
let rhs_temp = self.temp(uint_ty, span);
583+
self.cfg.push_assign(
584+
block,
585+
source_info,
586+
rhs_temp,
587+
Rvalue::Cast(CastKind::IntToInt, rhs.to_copy(), uint_ty),
588+
);
589+
(Operand::Move(rhs_temp), uint_ty)
590+
}
591+
_ => unreachable!("only integers are shiftable"),
592+
};
593+
594+
// This can't overflow because the largest shiftable types are 128-bit,
595+
// which fits in `u8`, the smallest possible `unsigned_ty`.
596+
// (And `from_uint` will `bug!` if that's ever no longer true.)
597+
let lhs_bits = Operand::const_from_scalar(
579598
self.tcx,
580-
rhs_ty,
581-
Scalar::from_uint(rhs_size.truncate(mask), rhs_size),
599+
unsigned_ty,
600+
Scalar::from_uint(lhs_size.bits(), rhs_size),
582601
span,
583602
);
584603

585-
let outer_bits = self.temp(rhs_ty, span);
586-
self.cfg.push_assign(
587-
block,
588-
source_info,
589-
outer_bits,
590-
Rvalue::BinaryOp(BinOp::BitAnd, Box::new((rhs.to_copy(), mask))),
591-
);
592-
593-
let overflows = self.temp(bool_ty, span);
594-
let zero = self.zero_literal(span, rhs_ty);
604+
let inbounds = self.temp(bool_ty, span);
595605
self.cfg.push_assign(
596606
block,
597607
source_info,
598-
overflows,
599-
Rvalue::BinaryOp(BinOp::Ne, Box::new((Operand::Move(outer_bits), zero))),
608+
inbounds,
609+
Rvalue::BinaryOp(BinOp::Lt, Box::new((unsigned_rhs, lhs_bits))),
600610
);
601611

602612
let overflow_err = AssertKind::Overflow(op, lhs.to_copy(), rhs.to_copy());
603-
block = self.assert(block, Operand::Move(overflows), false, overflow_err, span);
613+
block = self.assert(block, Operand::Move(inbounds), true, overflow_err, span);
604614
Rvalue::BinaryOp(op, Box::new((lhs, rhs)))
605615
}
606616
BinOp::Div | BinOp::Rem if ty.is_integral() => {

‎compiler/rustc_type_ir/src/lib.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,17 @@ impl IntTy {
432432
_ => *self,
433433
}
434434
}
435+
436+
pub fn to_unsigned(self) -> UintTy {
437+
match self {
438+
IntTy::Isize => UintTy::Usize,
439+
IntTy::I8 => UintTy::U8,
440+
IntTy::I16 => UintTy::U16,
441+
IntTy::I32 => UintTy::U32,
442+
IntTy::I64 => UintTy::U64,
443+
IntTy::I128 => UintTy::U128,
444+
}
445+
}
435446
}
436447

437448
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Debug)]
@@ -479,6 +490,17 @@ impl UintTy {
479490
_ => *self,
480491
}
481492
}
493+
494+
pub fn to_signed(self) -> IntTy {
495+
match self {
496+
UintTy::Usize => IntTy::Isize,
497+
UintTy::U8 => IntTy::I8,
498+
UintTy::U16 => IntTy::I16,
499+
UintTy::U32 => IntTy::I32,
500+
UintTy::U64 => IntTy::I64,
501+
UintTy::U128 => IntTy::I128,
502+
}
503+
}
482504
}
483505

484506
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]

‎tests/mir-opt/building/shifts.shift_signed.built.after.mir

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,33 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
1111
let mut _7: i8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
1212
let mut _8: i8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:15
1313
let mut _9: i8; // in scope 0 at $DIR/shifts.rs:+2:19: +2:20
14-
let mut _10: i8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
14+
let mut _10: u8; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
1515
let mut _11: bool; // in scope 0 at $DIR/shifts.rs:+2:10: +2:20
1616
let mut _12: i8; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
1717
let mut _13: i8; // in scope 0 at $DIR/shifts.rs:+2:22: +2:27
1818
let mut _14: i32; // in scope 0 at $DIR/shifts.rs:+2:31: +2:32
19-
let mut _15: i32; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
19+
let mut _15: u32; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
2020
let mut _16: bool; // in scope 0 at $DIR/shifts.rs:+2:22: +2:32
2121
let mut _17: i8; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
2222
let mut _18: i8; // in scope 0 at $DIR/shifts.rs:+2:34: +2:39
2323
let mut _19: i128; // in scope 0 at $DIR/shifts.rs:+2:43: +2:44
24-
let mut _20: i128; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
24+
let mut _20: u128; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
2525
let mut _21: bool; // in scope 0 at $DIR/shifts.rs:+2:34: +2:44
2626
let mut _22: [u128; 3]; // in scope 0 at $DIR/shifts.rs:+3:9: +3:39
2727
let mut _23: u128; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
2828
let mut _24: u128; // in scope 0 at $DIR/shifts.rs:+3:10: +3:13
2929
let mut _25: i8; // in scope 0 at $DIR/shifts.rs:+3:17: +3:18
30-
let mut _26: i8; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
30+
let mut _26: u8; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
3131
let mut _27: bool; // in scope 0 at $DIR/shifts.rs:+3:10: +3:18
3232
let mut _28: u128; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
3333
let mut _29: u128; // in scope 0 at $DIR/shifts.rs:+3:20: +3:23
3434
let mut _30: i32; // in scope 0 at $DIR/shifts.rs:+3:27: +3:28
35-
let mut _31: i32; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
35+
let mut _31: u32; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
3636
let mut _32: bool; // in scope 0 at $DIR/shifts.rs:+3:20: +3:28
3737
let mut _33: u128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38
3838
let mut _34: u128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:33
3939
let mut _35: i128; // in scope 0 at $DIR/shifts.rs:+3:37: +3:38
40-
let mut _36: i128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38
40+
let mut _36: u128; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38
4141
let mut _37: bool; // in scope 0 at $DIR/shifts.rs:+3:30: +3:38
4242

4343
bb0: {
@@ -47,9 +47,9 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
4747
_8 = _1; // scope 0 at $DIR/shifts.rs:+2:10: +2:15
4848
StorageLive(_9); // scope 0 at $DIR/shifts.rs:+2:19: +2:20
4949
_9 = _3; // scope 0 at $DIR/shifts.rs:+2:19: +2:20
50-
_10 = BitAnd(_9, const -8_i8); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
51-
_11 = Ne(move _10, const 0_i8); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
52-
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
50+
_10 = _9 as u8 (IntToInt); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
51+
_11 = Lt(move _10, const 8_u8); // scope 0 at $DIR/shifts.rs:+2:10: +2:20
52+
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
5353
}
5454

5555
bb1: {
@@ -61,9 +61,9 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
6161
_13 = _1; // scope 0 at $DIR/shifts.rs:+2:22: +2:27
6262
StorageLive(_14); // scope 0 at $DIR/shifts.rs:+2:31: +2:32
6363
_14 = _4; // scope 0 at $DIR/shifts.rs:+2:31: +2:32
64-
_15 = BitAnd(_14, const -8_i32); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
65-
_16 = Ne(move _15, const 0_i32); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
66-
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
64+
_15 = _14 as u32 (IntToInt); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
65+
_16 = Lt(move _15, const 8_u32); // scope 0 at $DIR/shifts.rs:+2:22: +2:32
66+
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
6767
}
6868

6969
bb2: {
@@ -75,9 +75,9 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
7575
_18 = _1; // scope 0 at $DIR/shifts.rs:+2:34: +2:39
7676
StorageLive(_19); // scope 0 at $DIR/shifts.rs:+2:43: +2:44
7777
_19 = _5; // scope 0 at $DIR/shifts.rs:+2:43: +2:44
78-
_20 = BitAnd(_19, const -8_i128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
79-
_21 = Ne(move _20, const 0_i128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
80-
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
78+
_20 = _19 as u128 (IntToInt); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
79+
_21 = Lt(move _20, const 8_u128); // scope 0 at $DIR/shifts.rs:+2:34: +2:44
80+
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
8181
}
8282

8383
bb3: {
@@ -94,9 +94,9 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
9494
_24 = _2; // scope 0 at $DIR/shifts.rs:+3:10: +3:13
9595
StorageLive(_25); // scope 0 at $DIR/shifts.rs:+3:17: +3:18
9696
_25 = _3; // scope 0 at $DIR/shifts.rs:+3:17: +3:18
97-
_26 = BitAnd(_25, const i8::MIN); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
98-
_27 = Ne(move _26, const 0_i8); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
99-
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
97+
_26 = _25 as u8 (IntToInt); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
98+
_27 = Lt(move _26, const 128_u8); // scope 0 at $DIR/shifts.rs:+3:10: +3:18
99+
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
100100
}
101101

102102
bb4: {
@@ -108,9 +108,9 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
108108
_29 = _2; // scope 0 at $DIR/shifts.rs:+3:20: +3:23
109109
StorageLive(_30); // scope 0 at $DIR/shifts.rs:+3:27: +3:28
110110
_30 = _4; // scope 0 at $DIR/shifts.rs:+3:27: +3:28
111-
_31 = BitAnd(_30, const -128_i32); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
112-
_32 = Ne(move _31, const 0_i32); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
113-
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
111+
_31 = _30 as u32 (IntToInt); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
112+
_32 = Lt(move _31, const 128_u32); // scope 0 at $DIR/shifts.rs:+3:20: +3:28
113+
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
114114
}
115115

116116
bb5: {
@@ -122,9 +122,9 @@ fn shift_signed(_1: i8, _2: u128, _3: i8, _4: i32, _5: i128) -> ([i8; 3], [u128;
122122
_34 = _2; // scope 0 at $DIR/shifts.rs:+3:30: +3:33
123123
StorageLive(_35); // scope 0 at $DIR/shifts.rs:+3:37: +3:38
124124
_35 = _5; // scope 0 at $DIR/shifts.rs:+3:37: +3:38
125-
_36 = BitAnd(_35, const -128_i128); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
126-
_37 = Ne(move _36, const 0_i128); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
127-
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
125+
_36 = _35 as u128 (IntToInt); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
126+
_37 = Lt(move _36, const 128_u128); // scope 0 at $DIR/shifts.rs:+3:30: +3:38
127+
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
128128
}
129129

130130
bb6: {

0 commit comments

Comments
 (0)