Auto merge of #117192 - saethlin:leaves-can-assert, r=scottmcm

Don't treat asserts as a call in cross-crate inlining

Making functions with calls in their bodies automatically cross-crate-inlinable tends to tank incremental build times. Though assert terminators are _like_ calls, they don't exhibit the same behavior.

Author: bors

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;
@@ -110,6 +111,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>,
@@ -129,9 +139,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;
@@ -140,7 +151,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
@@ -155,21 +166,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");
+            }
         }
     }
 }

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
+}

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)
+}

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)]