From 916e0d29637bb265975d30d9a68073450ed4c7d9 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 16 Jun 2024 23:44:29 -0700 Subject: [PATCH 1/3] Give `CostChecker` both penalties and bonuses --- .../rustc_mir_transform/src/cost_checker.rs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_transform/src/cost_checker.rs b/compiler/rustc_mir_transform/src/cost_checker.rs index 2c692c9500303..bca4ef5b3d126 100644 --- a/compiler/rustc_mir_transform/src/cost_checker.rs +++ b/compiler/rustc_mir_transform/src/cost_checker.rs @@ -12,7 +12,8 @@ const RESUME_PENALTY: usize = 45; pub(crate) struct CostChecker<'b, 'tcx> { tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, - cost: usize, + penalty: usize, + bonus: usize, callee_body: &'b Body<'tcx>, instance: Option>, } @@ -24,11 +25,11 @@ impl<'b, 'tcx> CostChecker<'b, 'tcx> { instance: Option>, callee_body: &'b Body<'tcx>, ) -> CostChecker<'b, 'tcx> { - CostChecker { tcx, param_env, callee_body, instance, cost: 0 } + CostChecker { tcx, param_env, callee_body, instance, penalty: 0, bonus: 0 } } pub fn cost(&self) -> usize { - self.cost + usize::saturating_sub(self.penalty, self.bonus) } fn instantiate_ty(&self, v: Ty<'tcx>) -> Ty<'tcx> { @@ -48,7 +49,7 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { | StatementKind::StorageDead(_) | StatementKind::Deinit(_) | StatementKind::Nop => {} - _ => self.cost += INSTR_COST, + _ => self.penalty += INSTR_COST, } } @@ -59,17 +60,17 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { // 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) { - self.cost += CALL_PENALTY; + self.penalty += CALL_PENALTY; if let UnwindAction::Cleanup(_) = unwind { - self.cost += LANDINGPAD_PENALTY; + self.penalty += LANDINGPAD_PENALTY; } } else { - self.cost += INSTR_COST; + self.penalty += INSTR_COST; } } TerminatorKind::Call { func: Operand::Constant(ref f), unwind, .. } => { let fn_ty = self.instantiate_ty(f.const_.ty()); - self.cost += if let ty::FnDef(def_id, _) = *fn_ty.kind() + self.penalty += if let ty::FnDef(def_id, _) = *fn_ty.kind() && tcx.intrinsic(def_id).is_some() { // Don't give intrinsics the extra penalty for calls @@ -78,23 +79,23 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { CALL_PENALTY }; if let UnwindAction::Cleanup(_) = unwind { - self.cost += LANDINGPAD_PENALTY; + self.penalty += LANDINGPAD_PENALTY; } } TerminatorKind::Assert { unwind, .. } => { - self.cost += CALL_PENALTY; + self.penalty += CALL_PENALTY; if let UnwindAction::Cleanup(_) = unwind { - self.cost += LANDINGPAD_PENALTY; + self.penalty += LANDINGPAD_PENALTY; } } - TerminatorKind::UnwindResume => self.cost += RESUME_PENALTY, + TerminatorKind::UnwindResume => self.penalty += RESUME_PENALTY, TerminatorKind::InlineAsm { unwind, .. } => { - self.cost += INSTR_COST; + self.penalty += INSTR_COST; if let UnwindAction::Cleanup(_) = unwind { - self.cost += LANDINGPAD_PENALTY; + self.penalty += LANDINGPAD_PENALTY; } } - _ => self.cost += INSTR_COST, + _ => self.penalty += INSTR_COST, } } } From 701f211ffbfe698d460f51298064360d17ec3988 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Mon, 17 Jun 2024 00:36:21 -0700 Subject: [PATCH 2/3] Give inlining bonuses to things that optimize out --- .../rustc_mir_transform/src/cost_checker.rs | 52 ++++++++++++++++--- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir_transform/src/cost_checker.rs b/compiler/rustc_mir_transform/src/cost_checker.rs index bca4ef5b3d126..152321ccc2219 100644 --- a/compiler/rustc_mir_transform/src/cost_checker.rs +++ b/compiler/rustc_mir_transform/src/cost_checker.rs @@ -6,6 +6,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 = 30; /// Verify that the callee body is compatible with the caller. #[derive(Clone)] @@ -42,13 +44,30 @@ 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, } } @@ -82,8 +101,25 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { self.penalty += LANDINGPAD_PENALTY; } } - TerminatorKind::Assert { unwind, .. } => { - self.penalty += CALL_PENALTY; + TerminatorKind::SwitchInt { ref discr, ref 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, ref 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; } From c5864081afd8441e31e9ad67a6aeced93d9d6c2d Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Mon, 17 Jun 2024 01:49:09 -0700 Subject: [PATCH 3/3] Shrink some slice iterator MIR --- library/core/src/slice/iter/macros.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/core/src/slice/iter/macros.rs b/library/core/src/slice/iter/macros.rs index 0b8ff5cc01242..c2a3819464410 100644 --- a/library/core/src/slice/iter/macros.rs +++ b/library/core/src/slice/iter/macros.rs @@ -103,7 +103,8 @@ macro_rules! iterator { // so this new pointer is inside `self` and thus guaranteed to be non-null. unsafe { if_zst!(mut self, - len => *len = len.unchecked_sub(offset), + // Using the intrinsic directly avoids emitting a UbCheck + len => *len = crate::intrinsics::unchecked_sub(*len, offset), _end => self.ptr = self.ptr.add(offset), ); } @@ -119,7 +120,8 @@ macro_rules! iterator { // SAFETY: By our precondition, `offset` can be at most the // current length, so the subtraction can never overflow. len => unsafe { - *len = len.unchecked_sub(offset); + // Using the intrinsic directly avoids emitting a UbCheck + *len = crate::intrinsics::unchecked_sub(*len, offset); self.ptr }, // SAFETY: the caller guarantees that `offset` doesn't exceed `self.len()`,