From daa77e621f427b5b21d584ef6d2e87101e3de3d2 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Wed, 25 Oct 2023 17:33:43 -0400 Subject: [PATCH] Don't treat asserts as a call in cross-crate inlining --- .../src/cross_crate_inline.rs | 35 +++++++++++++++---- .../cross-crate-inlining/auxiliary/leaf.rs | 5 +++ .../cross-crate-inlining/leaf-inlining.rs | 9 +++++ .../item-collection/implicit-panic-call.rs | 1 + 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_transform/src/cross_crate_inline.rs b/compiler/rustc_mir_transform/src/cross_crate_inline.rs index 7fc9fb9cca2d7..84c1d13e63714 100644 --- a/compiler/rustc_mir_transform/src/cross_crate_inline.rs +++ b/compiler/rustc_mir_transform/src/cross_crate_inline.rs @@ -1,6 +1,7 @@ use rustc_hir::attrs::InlineAttr; use rustc_hir::def::DefKind; use rustc_hir::def_id::LocalDefId; +use rustc_middle::bug; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; use rustc_middle::query::Providers; @@ -102,6 +103,15 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { && checker.statements <= threshold } +// The threshold that CostChecker computes is balancing the desire to make more things +// inlinable cross crates against the growth in incremental CGU size that happens when too many +// things in the sysroot are made inlinable. +// Permitting calls causes the size of some incremental CGUs to grow, because more functions are +// made inlinable out of the sysroot or dependencies. +// Assert terminators are similar to calls, but do not have the same impact on compile time, so +// those are just treated as statements. +// A threshold exists at all because we don't want to blindly mark a huge function as inlinable. + struct CostChecker<'b, 'tcx> { tcx: TyCtxt<'tcx>, callee_body: &'b Body<'tcx>, @@ -121,9 +131,10 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) { + self.statements += 1; let tcx = self.tcx; - match terminator.kind { - TerminatorKind::Drop { ref place, unwind, .. } => { + match &terminator.kind { + TerminatorKind::Drop { place, unwind, .. } => { let ty = place.ty(self.callee_body, tcx).ty; if !ty.is_trivially_pure_clone_copy() { self.calls += 1; @@ -132,7 +143,7 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { } } } - TerminatorKind::Call { ref func, unwind, .. } => { + TerminatorKind::Call { func, unwind, .. } => { // We track calls because they make our function not a leaf (and in theory, the // number of calls indicates how likely this function is to perturb other CGUs). // But intrinsics don't have a body that gets assigned to a CGU, so they are @@ -147,21 +158,31 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> { self.landing_pads += 1; } } - TerminatorKind::Assert { unwind, .. } => { + TerminatorKind::TailCall { .. } => { self.calls += 1; + } + TerminatorKind::Assert { unwind, .. } => { if let UnwindAction::Cleanup(_) = unwind { self.landing_pads += 1; } } TerminatorKind::UnwindResume => self.resumes += 1, TerminatorKind::InlineAsm { unwind, .. } => { - self.statements += 1; if let UnwindAction::Cleanup(_) = unwind { self.landing_pads += 1; } } - TerminatorKind::Return => {} - _ => self.statements += 1, + TerminatorKind::Return + | TerminatorKind::Goto { .. } + | TerminatorKind::SwitchInt { .. } + | TerminatorKind::Unreachable + | TerminatorKind::UnwindTerminate(_) => {} + kind @ (TerminatorKind::FalseUnwind { .. } + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::CoroutineDrop) => { + bug!("{kind:?} should not be in runtime MIR"); + } } } } diff --git a/tests/codegen-llvm/cross-crate-inlining/auxiliary/leaf.rs b/tests/codegen-llvm/cross-crate-inlining/auxiliary/leaf.rs index 7b5679c3f4def..5b3cd34a7fa57 100644 --- a/tests/codegen-llvm/cross-crate-inlining/auxiliary/leaf.rs +++ b/tests/codegen-llvm/cross-crate-inlining/auxiliary/leaf.rs @@ -23,3 +23,8 @@ fn inner() -> String { pub fn leaf_with_intrinsic(a: &[u64; 2], b: &[u64; 2]) -> bool { a == b } + +// This function's optimized MIR contains assert terminators, not calls. +pub fn leaf_with_assert(a: i32, b: i32) -> i32 { + a / b +} diff --git a/tests/codegen-llvm/cross-crate-inlining/leaf-inlining.rs b/tests/codegen-llvm/cross-crate-inlining/leaf-inlining.rs index 5e7912791ad60..3173adbd03ee6 100644 --- a/tests/codegen-llvm/cross-crate-inlining/leaf-inlining.rs +++ b/tests/codegen-llvm/cross-crate-inlining/leaf-inlining.rs @@ -25,3 +25,12 @@ pub fn leaf_with_intrinsic_outer(a: &[u64; 2], b: &[u64; 2]) -> bool { // CHECK-NOT: call {{.*}}leaf_with_intrinsic leaf::leaf_with_intrinsic(a, b) } + +// Check that we inline functions with assert terminators +#[no_mangle] +pub fn leaf_with_assert(a: i32, b: i32) -> i32 { + // CHECK-NOT: call {{.*}}leaf_with_assert + // CHECK: sdiv i32 %a, %b + // CHECK-NOT: call {{.*}}leaf_with_assert + leaf::leaf_with_assert(a, b) +} diff --git a/tests/codegen-units/item-collection/implicit-panic-call.rs b/tests/codegen-units/item-collection/implicit-panic-call.rs index 612132f056be4..3b442942cc5c4 100644 --- a/tests/codegen-units/item-collection/implicit-panic-call.rs +++ b/tests/codegen-units/item-collection/implicit-panic-call.rs @@ -1,5 +1,6 @@ // rust-lang/rust#90405 // Ensure implicit panic calls are collected +//@ compile-flags: -Zcross-crate-inline-threshold=never #![feature(lang_items)] #![feature(no_core)]