Skip to content

Commit 7aa413d

Browse files
borsbors
committed
Auto merge of rust-lang#107921 - cjgillot:codegen-overflow-check, r=tmiasko
Make codegen choose whether to emit overflow checks ConstProp and DataflowConstProp currently have a specific code path not to propagate constants when they overflow. This is meant to have the correct behaviour when inlining from a crate with overflow checks (like `core`) into a crate compiled without. This PR shifts the behaviour change to the `Assert(Overflow*)` MIR terminators: if the crate is compiled without overflow checks, just skip emitting the assertions. This is already what happens with `OverflowNeg`. This allows ConstProp and DataflowConstProp to transform `CheckedBinaryOp(Add, u8::MAX, 1)` into `const (0, true)`, and let codegen ignore the `true`. The interpreter is modified to conform to this behaviour. Fixes rust-lang#35310
2 parents dc89a80 + 9f6c1df commit 7aa413d

File tree

24 files changed

+250
-217
lines changed

24 files changed

+250
-217
lines changed

‎compiler/rustc_codegen_cranelift/src/base.rs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,12 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
347347
}
348348
TerminatorKind::Assert { cond, expected, msg, target, cleanup: _ } => {
349349
if !fx.tcx.sess.overflow_checks() {
350-
if let mir::AssertKind::OverflowNeg(_) = *msg {
350+
let overflow_not_to_check = match msg {
351+
AssertKind::OverflowNeg(..) => true,
352+
AssertKind::Overflow(op, ..) => op.is_checkable(),
353+
_ => false,
354+
};
355+
if overflow_not_to_check {
351356
let target = fx.get_block(*target);
352357
fx.bcx.ins().jump(target, &[]);
353358
continue;
@@ -567,15 +572,7 @@ fn codegen_stmt<'tcx>(
567572
let lhs = codegen_operand(fx, &lhs_rhs.0);
568573
let rhs = codegen_operand(fx, &lhs_rhs.1);
569574

570-
let res = if !fx.tcx.sess.overflow_checks() {
571-
let val =
572-
crate::num::codegen_int_binop(fx, bin_op, lhs, rhs).load_scalar(fx);
573-
let is_overflow = fx.bcx.ins().iconst(types::I8, 0);
574-
CValue::by_val_pair(val, is_overflow, lval.layout())
575-
} else {
576-
crate::num::codegen_checked_int_binop(fx, bin_op, lhs, rhs)
577-
};
578-
575+
let res = crate::num::codegen_checked_int_binop(fx, bin_op, lhs, rhs);
579576
lval.write_cvalue(fx, res);
580577
}
581578
Rvalue::UnaryOp(un_op, ref operand) => {

‎compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -493,20 +493,6 @@ fn codegen_regular_intrinsic_call<'tcx>(
493493
let res = crate::num::codegen_int_binop(fx, bin_op, x, y);
494494
ret.write_cvalue(fx, res);
495495
}
496-
sym::add_with_overflow | sym::sub_with_overflow | sym::mul_with_overflow => {
497-
intrinsic_args!(fx, args => (x, y); intrinsic);
498-
499-
assert_eq!(x.layout().ty, y.layout().ty);
500-
let bin_op = match intrinsic {
501-
sym::add_with_overflow => BinOp::Add,
502-
sym::sub_with_overflow => BinOp::Sub,
503-
sym::mul_with_overflow => BinOp::Mul,
504-
_ => unreachable!(),
505-
};
506-
507-
let res = crate::num::codegen_checked_int_binop(fx, bin_op, x, y);
508-
ret.write_cvalue(fx, res);
509-
}
510496
sym::saturating_add | sym::saturating_sub => {
511497
intrinsic_args!(fx, args => (lhs, rhs); intrinsic);
512498

‎compiler/rustc_codegen_ssa/src/mir/block.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -563,11 +563,13 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
563563
// with #[rustc_inherit_overflow_checks] and inlined from
564564
// another crate (mostly core::num generic/#[inline] fns),
565565
// while the current crate doesn't use overflow checks.
566-
// NOTE: Unlike binops, negation doesn't have its own
567-
// checked operation, just a comparison with the minimum
568-
// value, so we have to check for the assert message.
569-
if !bx.check_overflow() {
570-
if let AssertKind::OverflowNeg(_) = *msg {
566+
if !bx.cx().check_overflow() {
567+
let overflow_not_to_check = match msg {
568+
AssertKind::OverflowNeg(..) => true,
569+
AssertKind::Overflow(op, ..) => op.is_checkable(),
570+
_ => false,
571+
};
572+
if overflow_not_to_check {
571573
const_cond = Some(expected);
572574
}
573575
}

‎compiler/rustc_codegen_ssa/src/mir/intrinsic.rs‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
218218
args[1].val.unaligned_volatile_store(bx, dst);
219219
return;
220220
}
221-
sym::add_with_overflow
222-
| sym::sub_with_overflow
223-
| sym::mul_with_overflow
224221
| sym::unchecked_div
225222
| sym::unchecked_rem
226223
| sym::unchecked_shl
@@ -232,28 +229,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
232229
let ty = arg_tys[0];
233230
match int_type_width_signed(ty, bx.tcx()) {
234231
Some((_width, signed)) => match name {
235-
sym::add_with_overflow
236-
| sym::sub_with_overflow
237-
| sym::mul_with_overflow => {
238-
let op = match name {
239-
sym::add_with_overflow => OverflowOp::Add,
240-
sym::sub_with_overflow => OverflowOp::Sub,
241-
sym::mul_with_overflow => OverflowOp::Mul,
242-
_ => bug!(),
243-
};
244-
let (val, overflow) =
245-
bx.checked_binop(op, ty, args[0].immediate(), args[1].immediate());
246-
// Convert `i1` to a `bool`, and write it to the out parameter
247-
let val = bx.from_immediate(val);
248-
let overflow = bx.from_immediate(overflow);
249-
250-
let dest = result.project_field(bx, 0);
251-
bx.store(val, dest.llval, dest.align);
252-
let dest = result.project_field(bx, 1);
253-
bx.store(overflow, dest.llval, dest.align);
254-
255-
return;
256-
}
257232
sym::exact_div => {
258233
if signed {
259234
bx.exactsdiv(args[0].immediate(), args[1].immediate())

‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -652,15 +652,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
652652
rhs: Bx::Value,
653653
input_ty: Ty<'tcx>,
654654
) -> OperandValue<Bx::Value> {
655-
// This case can currently arise only from functions marked
656-
// with #[rustc_inherit_overflow_checks] and inlined from
657-
// another crate (mostly core::num generic/#[inline] fns),
658-
// while the current crate doesn't use overflow checks.
659-
if !bx.cx().check_overflow() {
660-
let val = self.codegen_scalar_binop(bx, op, lhs, rhs, input_ty);
661-
return OperandValue::Pair(val, bx.cx().const_bool(false));
662-
}
663-
664655
let (val, of) = match op {
665656
// These are checked using intrinsics
666657
mir::BinOp::Add | mir::BinOp::Sub | mir::BinOp::Mul => {

‎compiler/rustc_const_eval/src/interpret/intrinsics.rs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -210,19 +210,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
210210
let out_val = numeric_intrinsic(intrinsic_name, bits, kind);
211211
self.write_scalar(out_val, dest)?;
212212
}
213-
sym::add_with_overflow | sym::sub_with_overflow | sym::mul_with_overflow => {
214-
let lhs = self.read_immediate(&args[0])?;
215-
let rhs = self.read_immediate(&args[1])?;
216-
let bin_op = match intrinsic_name {
217-
sym::add_with_overflow => BinOp::Add,
218-
sym::sub_with_overflow => BinOp::Sub,
219-
sym::mul_with_overflow => BinOp::Mul,
220-
_ => bug!(),
221-
};
222-
self.binop_with_overflow(
223-
bin_op, /*force_overflow_checks*/ true, &lhs, &rhs, dest,
224-
)?;
225-
}
226213
sym::saturating_add | sym::saturating_sub => {
227214
let l = self.read_immediate(&args[0])?;
228215
let r = self.read_immediate(&args[1])?;

‎compiler/rustc_const_eval/src/interpret/machine.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,9 @@ pub trait Machine<'mir, 'tcx>: Sized {
147147
true
148148
}
149149

150-
/// Whether CheckedBinOp MIR statements should actually check for overflow.
151-
fn checked_binop_checks_overflow(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
150+
/// Whether Assert(OverflowNeg) and Assert(Overflow) MIR terminators should actually
151+
/// check for overflow.
152+
fn ignore_checkable_overflow_assertions(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
152153

153154
/// Entry point for obtaining the MIR of anything that should get evaluated.
154155
/// So not just functions and shims, but also const/static initializers, anonymous
@@ -466,8 +467,8 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
466467
}
467468

468469
#[inline(always)]
469-
fn checked_binop_checks_overflow(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
470-
true
470+
fn ignore_checkable_overflow_assertions(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool {
471+
false
471472
}
472473

473474
#[inline(always)]

‎compiler/rustc_const_eval/src/interpret/operator.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,9 @@ use super::{ImmTy, Immediate, InterpCx, Machine, PlaceTy};
1010
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
1111
/// Applies the binary operation `op` to the two operands and writes a tuple of the result
1212
/// and a boolean signifying the potential overflow to the destination.
13-
///
14-
/// `force_overflow_checks` indicates whether overflow checks should be done even when
15-
/// `tcx.sess.overflow_checks()` is `false`.
1613
pub fn binop_with_overflow(
1714
&mut self,
1815
op: mir::BinOp,
19-
force_overflow_checks: bool,
2016
left: &ImmTy<'tcx, M::Provenance>,
2117
right: &ImmTy<'tcx, M::Provenance>,
2218
dest: &PlaceTy<'tcx, M::Provenance>,
@@ -28,10 +24,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
2824
"type mismatch for result of {:?}",
2925
op,
3026
);
31-
// As per https://github.com/rust-lang/rust/pull/98738, we always return `false` in the 2nd
32-
// component when overflow checking is disabled.
33-
let overflowed =
34-
overflowed && (force_overflow_checks || M::checked_binop_checks_overflow(self));
3527
// Write the result to `dest`.
3628
if let Abi::ScalarPair(..) = dest.layout.abi {
3729
// We can use the optimized path and avoid `place_field` (which might do

‎compiler/rustc_const_eval/src/interpret/step.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
185185
let left = self.read_immediate(&self.eval_operand(left, None)?)?;
186186
let layout = binop_right_homogeneous(bin_op).then_some(left.layout);
187187
let right = self.read_immediate(&self.eval_operand(right, layout)?)?;
188-
self.binop_with_overflow(
189-
bin_op, /*force_overflow_checks*/ false, &left, &right, &dest,
190-
)?;
188+
self.binop_with_overflow(bin_op, &left, &right, &dest)?;
191189
}
192190

193191
UnaryOp(un_op, ref operand) => {

‎compiler/rustc_const_eval/src/interpret/terminator.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
137137
}
138138

139139
Assert { ref cond, expected, ref msg, target, cleanup } => {
140+
let ignored = M::ignore_checkable_overflow_assertions(self)
141+
&& match msg {
142+
mir::AssertKind::OverflowNeg(..) => true,
143+
mir::AssertKind::Overflow(op, ..) => op.is_checkable(),
144+
_ => false,
145+
};
140146
let cond_val = self.read_scalar(&self.eval_operand(cond, None)?)?.to_bool()?;
141-
if expected == cond_val {
147+
if ignored || expected == cond_val {
142148
self.go_to_block(target);
143149
} else {
144150
M::assert_panic(self, msg, cleanup)?;

0 commit comments

Comments
 (0)