diff options
7 files changed, 355 insertions, 64 deletions
diff --git a/compiler/rustc_mir_transform/src/cost_checker.rs b/compiler/rustc_mir_transform/src/cost_checker.rs index bca4ef5b3d1..32c0d27f635 100644 --- a/compiler/rustc_mir_transform/src/cost_checker.rs +++ b/compiler/rustc_mir_transform/src/cost_checker.rs @@ -1,3 +1,4 @@ +use rustc_middle::bug; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt}; @@ -6,6 +7,8 @@ const INSTR_COST: usize = 5; const CALL_PENALTY: usize = 25; const LANDINGPAD_PENALTY: usize = 50; const RESUME_PENALTY: usize = 45; +const LARGE_SWITCH_PENALTY: usize = 20; +const CONST_SWITCH_BONUS: usize = 10; /// Verify that the callee body is compatible with the caller. #[derive(Clone)] @@ -42,36 +45,49 @@ impl<'b, 'tcx> CostChecker<'b, 'tcx> { } impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { - fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) { - // Don't count StorageLive/StorageDead in the inlining cost. + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + // Most costs are in rvalues and terminators, not in statements. match statement.kind { - StatementKind::StorageLive(_) - | StatementKind::StorageDead(_) - | StatementKind::Deinit(_) - | StatementKind::Nop => {} + StatementKind::Intrinsic(ref ndi) => { + self.penalty += match **ndi { + NonDivergingIntrinsic::Assume(..) => INSTR_COST, + NonDivergingIntrinsic::CopyNonOverlapping(..) => CALL_PENALTY, + }; + } + _ => self.super_statement(statement, location), + } + } + + fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, _location: Location) { + match rvalue { + Rvalue::NullaryOp(NullOp::UbChecks, ..) if !self.tcx.sess.ub_checks() => { + // If this is in optimized MIR it's because it's used later, + // so if we don't need UB checks this session, give a bonus + // here to offset the cost of the call later. + self.bonus += CALL_PENALTY; + } + // These are essentially constants that didn't end up in an Operand, + // so treat them as also being free. + Rvalue::NullaryOp(..) => {} _ => self.penalty += INSTR_COST, } } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) { - let tcx = self.tcx; - match terminator.kind { - TerminatorKind::Drop { ref place, unwind, .. } => { + match &terminator.kind { + TerminatorKind::Drop { place, unwind, .. } => { // If the place doesn't actually need dropping, treat it like a regular goto. - let ty = self.instantiate_ty(place.ty(self.callee_body, tcx).ty); - if ty.needs_drop(tcx, self.param_env) { + let ty = self.instantiate_ty(place.ty(self.callee_body, self.tcx).ty); + if ty.needs_drop(self.tcx, self.param_env) { self.penalty += CALL_PENALTY; if let UnwindAction::Cleanup(_) = unwind { self.penalty += LANDINGPAD_PENALTY; } - } else { - self.penalty += INSTR_COST; } } - TerminatorKind::Call { func: Operand::Constant(ref f), unwind, .. } => { - let fn_ty = self.instantiate_ty(f.const_.ty()); - self.penalty += if let ty::FnDef(def_id, _) = *fn_ty.kind() - && tcx.intrinsic(def_id).is_some() + TerminatorKind::Call { func, unwind, .. } => { + self.penalty += if let Some((def_id, ..)) = func.const_fn_def() + && self.tcx.intrinsic(def_id).is_some() { // Don't give intrinsics the extra penalty for calls INSTR_COST @@ -82,8 +98,25 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { self.penalty += LANDINGPAD_PENALTY; } } - TerminatorKind::Assert { unwind, .. } => { - self.penalty += CALL_PENALTY; + TerminatorKind::SwitchInt { discr, targets } => { + if discr.constant().is_some() { + // Not only will this become a `Goto`, but likely other + // things will be removable as unreachable. + self.bonus += CONST_SWITCH_BONUS; + } else if targets.all_targets().len() > 3 { + // More than false/true/unreachable gets extra cost. + self.penalty += LARGE_SWITCH_PENALTY; + } else { + self.penalty += INSTR_COST; + } + } + TerminatorKind::Assert { unwind, msg, .. } => { + self.penalty += + if msg.is_optional_overflow_check() && !self.tcx.sess.overflow_checks() { + INSTR_COST + } else { + CALL_PENALTY + }; if let UnwindAction::Cleanup(_) = unwind { self.penalty += LANDINGPAD_PENALTY; } @@ -95,7 +128,17 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { self.penalty += LANDINGPAD_PENALTY; } } - _ => self.penalty += INSTR_COST, + TerminatorKind::Unreachable => { + self.bonus += INSTR_COST; + } + TerminatorKind::Goto { .. } | TerminatorKind::Return => {} + TerminatorKind::UnwindTerminate(..) => {} + kind @ (TerminatorKind::FalseUnwind { .. } + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::CoroutineDrop) => { + bug!("{kind:?} should not be in runtime MIR"); + } } } } diff --git a/tests/codegen/issues/issue-112509-slice-get-andthen-get.rs b/tests/codegen/issues/issue-112509-slice-get-andthen-get.rs index ae02c3fb79e..aee2edd8dfa 100644 --- a/tests/codegen/issues/issue-112509-slice-get-andthen-get.rs +++ b/tests/codegen/issues/issue-112509-slice-get-andthen-get.rs @@ -3,8 +3,12 @@ // CHECK-LABEL: @write_u8_variant_a // CHECK-NEXT: {{.*}}: -// CHECK-NEXT: getelementptr // CHECK-NEXT: icmp ugt +// CHECK-NEXT: getelementptr +// CHECK-NEXT: select i1 {{.+}} null +// CHECK-NEXT: insertvalue +// CHECK-NEXT: insertvalue +// CHECK-NEXT: ret #[no_mangle] pub fn write_u8_variant_a(bytes: &mut [u8], buf: u8, offset: usize) -> Option<&mut [u8]> { let buf = buf.to_le_bytes(); diff --git a/tests/crashes/123893.rs b/tests/crashes/123893.rs index 137ae783511..05237d002b8 100644 --- a/tests/crashes/123893.rs +++ b/tests/crashes/123893.rs @@ -11,6 +11,7 @@ fn generic_impl<T>() { impl<T> MagicTrait for T { const IS_BIG: bool = true; } + more_cost(); if T::IS_BIG { big_impl::<i32>(); } @@ -18,3 +19,6 @@ fn generic_impl<T>() { #[inline(never)] fn big_impl<T>() {} + +#[inline(never)] +fn more_cost() {} diff --git a/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir index cfb9134a1f1..e31a8cb6937 100644 --- a/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir @@ -4,12 +4,87 @@ fn step_forward(_1: u16, _2: usize) -> u16 { debug x => _1; debug n => _2; let mut _0: u16; + scope 1 (inlined <u16 as Step>::forward) { + let mut _8: u16; + scope 2 { + } + scope 3 (inlined <u16 as Step>::forward_checked) { + scope 4 { + scope 6 (inlined core::num::<impl u16>::checked_add) { + let mut _7: bool; + scope 7 { + } + scope 8 (inlined core::num::<impl u16>::overflowing_add) { + let mut _5: (u16, bool); + let _6: bool; + scope 9 { + } + } + } + } + scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) { + let mut _3: bool; + let mut _4: u16; + } + } + scope 10 (inlined Option::<u16>::is_none) { + scope 11 (inlined Option::<u16>::is_some) { + } + } + scope 12 (inlined core::num::<impl u16>::wrapping_add) { + } + } bb0: { - _0 = <u16 as Step>::forward(move _1, move _2) -> [return: bb1, unwind unreachable]; + StorageLive(_4); + StorageLive(_3); + _3 = Gt(_2, const 65535_usize); + switchInt(move _3) -> [0: bb1, otherwise: bb5]; } bb1: { + _4 = _2 as u16 (IntToInt); + StorageDead(_3); + StorageLive(_6); + StorageLive(_5); + _5 = AddWithOverflow(_1, _4); + _6 = (_5.1: bool); + StorageDead(_5); + StorageLive(_7); + _7 = unlikely(move _6) -> [return: bb2, unwind unreachable]; + } + + bb2: { + switchInt(move _7) -> [0: bb3, otherwise: bb4]; + } + + bb3: { + StorageDead(_7); + StorageDead(_6); + goto -> bb7; + } + + bb4: { + StorageDead(_7); + StorageDead(_6); + goto -> bb6; + } + + bb5: { + StorageDead(_3); + goto -> bb6; + } + + bb6: { + assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind unreachable]; + } + + bb7: { + StorageLive(_8); + _8 = _2 as u16 (IntToInt); + _0 = Add(_1, _8); + StorageDead(_8); + StorageDead(_4); return; } } diff --git a/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-unwind.mir index cacc1224aba..8cc9be27e21 100644 --- a/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-unwind.mir +++ b/tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-unwind.mir @@ -4,12 +4,87 @@ fn step_forward(_1: u16, _2: usize) -> u16 { debug x => _1; debug n => _2; let mut _0: u16; + scope 1 (inlined <u16 as Step>::forward) { + let mut _8: u16; + scope 2 { + } + scope 3 (inlined <u16 as Step>::forward_checked) { + scope 4 { + scope 6 (inlined core::num::<impl u16>::checked_add) { + let mut _7: bool; + scope 7 { + } + scope 8 (inlined core::num::<impl u16>::overflowing_add) { + let mut _5: (u16, bool); + let _6: bool; + scope 9 { + } + } + } + } + scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) { + let mut _3: bool; + let mut _4: u16; + } + } + scope 10 (inlined Option::<u16>::is_none) { + scope 11 (inlined Option::<u16>::is_some) { + } + } + scope 12 (inlined core::num::<impl u16>::wrapping_add) { + } + } bb0: { - _0 = <u16 as Step>::forward(move _1, move _2) -> [return: bb1, unwind continue]; + StorageLive(_4); + StorageLive(_3); + _3 = Gt(_2, const 65535_usize); + switchInt(move _3) -> [0: bb1, otherwise: bb5]; } bb1: { + _4 = _2 as u16 (IntToInt); + StorageDead(_3); + StorageLive(_6); + StorageLive(_5); + _5 = AddWithOverflow(_1, _4); + _6 = (_5.1: bool); + StorageDead(_5); + StorageLive(_7); + _7 = unlikely(move _6) -> [return: bb2, unwind unreachable]; + } + + bb2: { + switchInt(move _7) -> [0: bb3, otherwise: bb4]; + } + + bb3: { + StorageDead(_7); + StorageDead(_6); + goto -> bb7; + } + + bb4: { + StorageDead(_7); + StorageDead(_6); + goto -> bb6; + } + + bb5: { + StorageDead(_3); + goto -> bb6; + } + + bb6: { + assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind continue]; + } + + bb7: { + StorageLive(_8); + _8 = _2 as u16 (IntToInt); + _0 = Add(_1, _8); + StorageDead(_8); + StorageDead(_4); return; } } diff --git a/tests/mir-opt/pre-codegen/loops.mapped.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/loops.mapped.PreCodegen.after.mir index b800a1be22b..51d41e9ff05 100644 --- a/tests/mir-opt/pre-codegen/loops.mapped.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/loops.mapped.PreCodegen.after.mir @@ -7,14 +7,30 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () { let mut _3: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>; let mut _4: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>; let mut _5: &mut std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>; - let mut _6: std::option::Option<U>; - let mut _7: isize; - let _9: (); + let mut _13: std::option::Option<U>; + let _15: (); scope 1 { debug iter => _4; - let _8: U; + let _14: U; scope 2 { - debug x => _8; + debug x => _14; + } + scope 4 (inlined <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as Iterator>::next) { + debug self => _5; + let mut _6: &mut impl Iterator<Item = T>; + let mut _7: std::option::Option<T>; + let mut _8: &mut impl Fn(T) -> U; + scope 5 (inlined Option::<T>::map::<U, &mut impl Fn(T) -> U>) { + debug self => _7; + debug f => _8; + let mut _9: isize; + let _10: T; + let mut _11: (T,); + let mut _12: U; + scope 6 { + debug x => _10; + } + } } } scope 3 (inlined <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as IntoIterator>::into_iter) { @@ -32,20 +48,30 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () { } bb2: { - StorageLive(_6); - StorageLive(_5); + StorageLive(_13); _5 = &mut _4; - _6 = <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as Iterator>::next(move _5) -> [return: bb3, unwind: bb9]; + StorageLive(_8); + StorageLive(_7); + StorageLive(_6); + _6 = &mut (_4.0: impl Iterator<Item = T>); + _7 = <impl Iterator<Item = T> as Iterator>::next(move _6) -> [return: bb3, unwind: bb10]; } bb3: { - StorageDead(_5); - _7 = discriminant(_6); - switchInt(move _7) -> [0: bb4, 1: bb6, otherwise: bb8]; + StorageDead(_6); + _8 = &mut (_4.1: impl Fn(T) -> U); + StorageLive(_9); + StorageLive(_10); + _9 = discriminant(_7); + switchInt(move _9) -> [0: bb4, 1: bb6, otherwise: bb9]; } bb4: { - StorageDead(_6); + StorageDead(_10); + StorageDead(_9); + StorageDead(_7); + StorageDead(_8); + StorageDead(_13); drop(_4) -> [return: bb5, unwind continue]; } @@ -55,24 +81,39 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () { } bb6: { - _8 = move ((_6 as Some).0: U); - _9 = opaque::<U>(move _8) -> [return: bb7, unwind: bb9]; + _10 = move ((_7 as Some).0: T); + StorageLive(_12); + StorageLive(_11); + _11 = (_10,); + _12 = <&mut impl Fn(T) -> U as FnOnce<(T,)>>::call_once(move _8, move _11) -> [return: bb7, unwind: bb10]; } bb7: { - StorageDead(_6); - goto -> bb2; + StorageDead(_11); + _13 = Option::<U>::Some(move _12); + StorageDead(_12); + StorageDead(_10); + StorageDead(_9); + StorageDead(_7); + StorageDead(_8); + _14 = move ((_13 as Some).0: U); + _15 = opaque::<U>(move _14) -> [return: bb8, unwind: bb10]; } bb8: { - unreachable; + StorageDead(_13); + goto -> bb2; } - bb9 (cleanup): { - drop(_4) -> [return: bb10, unwind terminate(cleanup)]; + bb9: { + unreachable; } bb10 (cleanup): { + drop(_4) -> [return: bb11, unwind terminate(cleanup)]; + } + + bb11 (cleanup): { resume; } } diff --git a/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir index 8eb102e68f3..35a1c783bf2 100644 --- a/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir @@ -7,19 +7,44 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () { let mut _11: std::slice::Iter<'_, T>; let mut _12: std::iter::Enumerate<std::slice::Iter<'_, T>>; let mut _13: std::iter::Enumerate<std::slice::Iter<'_, T>>; - let mut _14: &mut std::iter::Enumerate<std::slice::Iter<'_, T>>; - let mut _15: std::option::Option<(usize, &T)>; - let mut _16: isize; - let mut _19: &impl Fn(usize, &T); - let mut _20: (usize, &T); - let _21: (); + let mut _21: std::option::Option<(usize, &T)>; + let mut _24: &impl Fn(usize, &T); + let mut _25: (usize, &T); + let _26: (); scope 1 { debug iter => _13; - let _17: usize; - let _18: &T; + let _22: usize; + let _23: &T; scope 2 { - debug i => _17; - debug x => _18; + debug i => _22; + debug x => _23; + } + scope 17 (inlined <Enumerate<std::slice::Iter<'_, T>> as Iterator>::next) { + let mut _14: &mut std::slice::Iter<'_, T>; + let mut _15: std::option::Option<&T>; + let mut _19: (usize, bool); + let mut _20: (usize, &T); + scope 18 { + let _18: usize; + scope 23 { + } + } + scope 19 { + scope 20 { + scope 26 (inlined <Option<(usize, &T)> as FromResidual>::from_residual) { + } + } + } + scope 21 { + scope 22 { + } + } + scope 24 (inlined <Option<&T> as Try>::branch) { + let mut _16: isize; + let _17: &T; + scope 25 { + } + } } } scope 3 (inlined core::slice::<impl [T]>::iter) { @@ -107,20 +132,28 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () { } bb4: { + StorageLive(_21); + StorageLive(_18); + StorageLive(_19); StorageLive(_15); StorageLive(_14); - _14 = &mut _13; - _15 = <Enumerate<std::slice::Iter<'_, T>> as Iterator>::next(move _14) -> [return: bb5, unwind unreachable]; + _14 = &mut (_13.0: std::slice::Iter<'_, T>); + _15 = <std::slice::Iter<'_, T> as Iterator>::next(move _14) -> [return: bb5, unwind unreachable]; } bb5: { StorageDead(_14); + StorageLive(_16); _16 = discriminant(_15); - switchInt(move _16) -> [0: bb6, 1: bb8, otherwise: bb10]; + switchInt(move _16) -> [0: bb6, 1: bb8, otherwise: bb11]; } bb6: { + StorageDead(_16); StorageDead(_15); + StorageDead(_19); + StorageDead(_18); + StorageDead(_21); StorageDead(_13); drop(_2) -> [return: bb7, unwind unreachable]; } @@ -130,23 +163,39 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () { } bb8: { - _17 = (((_15 as Some).0: (usize, &T)).0: usize); - _18 = (((_15 as Some).0: (usize, &T)).1: &T); - StorageLive(_19); - _19 = &_2; - StorageLive(_20); - _20 = (_17, _18); - _21 = <impl Fn(usize, &T) as Fn<(usize, &T)>>::call(move _19, move _20) -> [return: bb9, unwind unreachable]; + _17 = move ((_15 as Some).0: &T); + StorageDead(_16); + StorageDead(_15); + _18 = (_13.1: usize); + _19 = AddWithOverflow((_13.1: usize), const 1_usize); + assert(!move (_19.1: bool), "attempt to compute `{} + {}`, which would overflow", (_13.1: usize), const 1_usize) -> [success: bb9, unwind unreachable]; } bb9: { + (_13.1: usize) = move (_19.0: usize); + StorageLive(_20); + _20 = (_18, _17); + _21 = Option::<(usize, &T)>::Some(move _20); StorageDead(_20); StorageDead(_19); - StorageDead(_15); - goto -> bb4; + StorageDead(_18); + _22 = (((_21 as Some).0: (usize, &T)).0: usize); + _23 = (((_21 as Some).0: (usize, &T)).1: &T); + StorageLive(_24); + _24 = &_2; + StorageLive(_25); + _25 = (_22, _23); + _26 = <impl Fn(usize, &T) as Fn<(usize, &T)>>::call(move _24, move _25) -> [return: bb10, unwind unreachable]; } bb10: { + StorageDead(_25); + StorageDead(_24); + StorageDead(_21); + goto -> bb4; + } + + bb11: { unreachable; } } |