Auto merge of #121114 - Nilstrieb:no-inline!, r=saethlin

Add `#[rustc_no_mir_inline]` for standard library UB checks

should help with #121110 and also with #120848

Because the MIR inliner cannot know whether the checks are enabled or not, so inlining is an unnecessary compile time pessimization when debug assertions are disabled. LLVM knows whether they are enabled or not, so it can optimize accordingly without wasting time.

r? `@saethlin`

Author: bors

compiler/rustc_feature/src/builtin_attrs.rs

@@ -792,6 +792,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         rustc_intrinsic, Normal, template!(Word), ErrorFollowing,
         "the `#[rustc_intrinsic]` attribute is used to declare intrinsics with function bodies",
     ),
+    rustc_attr!(
+        rustc_no_mir_inline, Normal, template!(Word), WarnFollowing,
+        "#[rustc_no_mir_inline] prevents the MIR inliner from inlining a function while not affecting codegen"
+    ),
 
     // ==========================================================================
     // Internal attributes, Testing:

compiler/rustc_mir_transform/src/inline.rs

@@ -421,6 +421,10 @@ impl<'tcx> Inliner<'tcx> {
         callee_attrs: &CodegenFnAttrs,
         cross_crate_inlinable: bool,
     ) -> Result<(), &'static str> {
+        if self.tcx.has_attr(callsite.callee.def_id(), sym::rustc_no_mir_inline) {
+            return Err("#[rustc_no_mir_inline]");
+        }
+
         if let InlineAttr::Never = callee_attrs.inline {
             return Err("never inline hint");
         }

compiler/rustc_span/src/symbol.rs

@@ -1441,6 +1441,7 @@ symbols! {
         rustc_mir,
         rustc_must_implement_one_of,
         rustc_never_returns_null_ptr,
+        rustc_no_mir_inline,
         rustc_nonnull_optimization_guaranteed,
         rustc_nounwind,
         rustc_object_lifetime_default,

library/core/src/intrinsics.rs

@@ -2706,13 +2706,25 @@ pub const unsafe fn const_deallocate(_ptr: *mut u8, _size: usize, _align: usize)
 macro_rules! assert_unsafe_precondition {
     ($message:expr, ($($name:ident:$ty:ty = $arg:expr),*$(,)?) => $e:expr $(,)?) => {
         {
+            // #[cfg(bootstrap)] (this comment)
             // When the standard library is compiled with debug assertions, we want the check to inline for better performance.
             // This is important when working on the compiler, which is compiled with debug assertions locally.
             // When not compiled with debug assertions (so the precompiled std) we outline the check to minimize the compile
             // time impact when debug assertions are disabled.
-            // It is not clear whether that is the best solution, see #120848.
-            #[cfg_attr(debug_assertions, inline(always))]
-            #[cfg_attr(not(debug_assertions), inline(never))]
+            // The proper solution to this is the `#[rustc_no_mir_inline]` below, but we still want decent performance for cfg(bootstrap).
+            #[cfg_attr(all(debug_assertions, bootstrap), inline(always))]
+            #[cfg_attr(all(not(debug_assertions), bootstrap), inline(never))]
+
+            // This check is inlineable, but not by the MIR inliner.
+            // The reason for this is that the MIR inliner is in an exceptionally bad position
+            // to think about whether or not to inline this. In MIR, this call is gated behind `debug_assertions`,
+            // which will codegen to `false` in release builds. Inlining the check would be wasted work in that case and
+            // would be bad for compile times.
+            //
+            // LLVM on the other hand sees the constant branch, so if it's `false`, it can immediately delete it without
+            // inlining the check. If it's `true`, it can inline it and get significantly better performance.
+            #[cfg_attr(not(bootstrap), rustc_no_mir_inline)]
+            #[cfg_attr(not(bootstrap), inline)]
             #[rustc_nounwind]
             fn precondition_check($($name:$ty),*) {
                 if !$e {

tests/mir-opt/inline/rustc_no_mir_inline.caller.Inline.panic-abort.diff

@@ -0,0 +1,19 @@
+- // MIR for `caller` before Inline
++ // MIR for `caller` after Inline
+  
+  fn caller() -> () {
+      let mut _0: ();
+      let _1: ();
+  
+      bb0: {
+          StorageLive(_1);
+          _1 = callee() -> [return: bb1, unwind unreachable];
+      }
+  
+      bb1: {
+          StorageDead(_1);
+          _0 = const ();
+          return;
+      }
+  }
+  

tests/mir-opt/inline/rustc_no_mir_inline.caller.Inline.panic-unwind.diff

@@ -0,0 +1,19 @@
+- // MIR for `caller` before Inline
++ // MIR for `caller` after Inline
+  
+  fn caller() -> () {
+      let mut _0: ();
+      let _1: ();
+  
+      bb0: {
+          StorageLive(_1);
+          _1 = callee() -> [return: bb1, unwind continue];
+      }
+  
+      bb1: {
+          StorageDead(_1);
+          _0 = const ();
+          return;
+      }
+  }
+  

tests/mir-opt/inline/rustc_no_mir_inline.caller.PreCodegen.after.panic-abort.mir

@@ -0,0 +1,14 @@
+// MIR for `caller` after PreCodegen
+
+fn caller() -> () {
+    let mut _0: ();
+    let _1: ();
+
+    bb0: {
+        _1 = callee() -> [return: bb1, unwind unreachable];
+    }
+
+    bb1: {
+        return;
+    }
+}

tests/mir-opt/inline/rustc_no_mir_inline.caller.PreCodegen.after.panic-unwind.mir

@@ -0,0 +1,14 @@
+// MIR for `caller` after PreCodegen
+
+fn caller() -> () {
+    let mut _0: ();
+    let _1: ();
+
+    bb0: {
+        _1 = callee() -> [return: bb1, unwind continue];
+    }
+
+    bb1: {
+        return;
+    }
+}

tests/mir-opt/inline/rustc_no_mir_inline.rs

@@ -0,0 +1,17 @@
+// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+#![crate_type = "lib"]
+#![feature(rustc_attrs)]
+
+//@ compile-flags: -Zmir-opt-level=2 -Zinline-mir
+
+#[inline]
+#[rustc_no_mir_inline]
+pub fn callee() {}
+
+// EMIT_MIR rustc_no_mir_inline.caller.Inline.diff
+// EMIT_MIR rustc_no_mir_inline.caller.PreCodegen.after.mir
+pub fn caller() {
+    // CHECK-LABEL: fn caller(
+    // CHECK: callee()
+    callee();
+}