Auto merge of #117192 - saethlin:leaves-can-assert, r=<try>

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

try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2

Author: rust-bors[bot]

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

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-windows-32bit.rs

@@ -7,7 +7,7 @@
 //@ [strong] compile-flags: -Z stack-protector=strong
 //@ [basic] compile-flags: -Z stack-protector=basic
 //@ [none] compile-flags: -Z stack-protector=none
-//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled
+//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled -Cpanic=abort
 
 #![crate_type = "lib"]
 #![allow(internal_features)]
@@ -143,26 +143,11 @@ pub fn local_string_addr_taken(f: fn(&String)) {
     f(&x);
 
     // Taking the address of the local variable `x` leads to stack smash
-    // protection with the `strong` heuristic, but not with the `basic`
-    // heuristic. It does not matter that the reference is not mut.
-    //
-    // An interesting note is that a similar function in C++ *would* be
-    // protected by the `basic` heuristic, because `std::string` has a char
-    // array internally as a small object optimization:
-    // ```
-    // cat <<EOF | clang++ -O2 -fstack-protector -S -x c++ - -o - | grep stack_chk
-    // #include <string>
-    // void f(void (*g)(const std::string&)) {
-    //     std::string x;
-    //     g(x);
-    // }
-    // EOF
-    // ```
-    //
+    // protection. It does not matter that the reference is not mut.
 
     // all: __security_check_cookie
-    // strong-NOT: __security_check_cookie
-    // basic-NOT: __security_check_cookie
+    // strong: __security_check_cookie
+    // basic: __security_check_cookie
     // none-NOT: __security_check_cookie
     // missing-NOT: __security_check_cookie
 }
@@ -346,13 +331,8 @@ pub fn unsized_fn_param(s: [u8], l: bool, f: fn([u8])) {
     // alloca, and is therefore not protected by the `strong` or `basic`
     // heuristics.
 
-    // We should have a __security_check_cookie call in `all` and `strong` modes but
-    // LLVM does not support generating stack protectors in functions with funclet
-    // based EH personalities.
-    // https://github.com/llvm/llvm-project/blob/37fd3c96b917096d8a550038f6e61cdf0fc4174f/llvm/lib/CodeGen/StackProtector.cpp#L103C1-L109C4
-    // all-NOT: __security_check_cookie
-    // strong-NOT: __security_check_cookie
-
+    // all: __security_check_cookie
+    // strong: __security_check_cookie
     // basic-NOT: __security_check_cookie
     // none-NOT: __security_check_cookie
     // missing-NOT: __security_check_cookie

tests/assembly-llvm/stack-protector/stack-protector-heuristics-effect-windows-64bit.rs

@@ -7,7 +7,7 @@
 //@ [strong] compile-flags: -Z stack-protector=strong
 //@ [basic] compile-flags: -Z stack-protector=basic
 //@ [none] compile-flags: -Z stack-protector=none
-//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled
+//@ compile-flags: -C opt-level=2 -Z merge-functions=disabled -Cpanic=abort
 
 #![crate_type = "lib"]
 #![feature(unsized_fn_params)]
@@ -138,40 +138,17 @@ pub fn local_var_addr_used_indirectly(f: fn(bool)) {
 // CHECK-LABEL: local_string_addr_taken
 #[no_mangle]
 pub fn local_string_addr_taken(f: fn(&String)) {
-    // CHECK-DAG: .seh_endprologue
     let x = String::new();
     f(&x);
 
     // Taking the address of the local variable `x` leads to stack smash
-    // protection with the `strong` heuristic, but not with the `basic`
-    // heuristic. It does not matter that the reference is not mut.
-    //
-    // An interesting note is that a similar function in C++ *would* be
-    // protected by the `basic` heuristic, because `std::string` has a char
-    // array internally as a small object optimization:
-    // ```
-    // cat <<EOF | clang++ -O2 -fstack-protector -S -x c++ - -o - | grep stack_chk
-    // #include <string>
-    // void f(void (*g)(const std::string&)) {
-    //     std::string x;
-    //     g(x);
-    // }
-    // EOF
-    // ```
-    //
+    // protection. It does not matter that the reference is not mut.
 
-    // We should have a __security_check_cookie call in `all` and `strong` modes but
-    // LLVM does not support generating stack protectors in functions with funclet
-    // based EH personalities.
-    // https://github.com/llvm/llvm-project/blob/37fd3c96b917096d8a550038f6e61cdf0fc4174f/llvm/lib/CodeGen/StackProtector.cpp#L103C1-L109C4
-    // all-NOT: __security_check_cookie
-    // strong-NOT: __security_check_cookie
-
-    // basic-NOT: __security_check_cookie
+    // all: __security_check_cookie
+    // strong: __security_check_cookie
+    // basic: __security_check_cookie
     // none-NOT: __security_check_cookie
     // missing-NOT: __security_check_cookie
-
-    // CHECK-DAG: .seh_endproc
 }
 
 pub trait SelfByRef {
@@ -353,13 +330,8 @@ pub fn unsized_fn_param(s: [u8], l: bool, f: fn([u8])) {
     // alloca, and is therefore not protected by the `strong` or `basic`
     // heuristics.
 
-    // We should have a __security_check_cookie call in `all` and `strong` modes but
-    // LLVM does not support generating stack protectors in functions with funclet
-    // based EH personalities.
-    // https://github.com/llvm/llvm-project/blob/37fd3c96b917096d8a550038f6e61cdf0fc4174f/llvm/lib/CodeGen/StackProtector.cpp#L103C1-L109C4
-    // all-NOT: __security_check_cookie
-    // strong-NOT: __security_check_cookie
-
+    // all: __security_check_cookie
+    // strong: __security_check_cookie
     // basic-NOT: __security_check_cookie
     // none-NOT: __security_check_cookie
     // missing-NOT: __security_check_cookie

tests/assembly-llvm/stack-protector/stack-protector-target-support.rs

@@ -173,7 +173,7 @@
 //@ [r84] needs-llvm-components: x86
 //@ [r85] compile-flags: --target x86_64-unknown-redox
 //@ [r85] needs-llvm-components: x86
-//@ compile-flags: -Z stack-protector=all
+//@ compile-flags: -Z stack-protector=all -Cpanic=abort
 //@ compile-flags: -C opt-level=2
 
 #![crate_type = "lib"]

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