Skip to content

Commit

Permalink
Auto merge of rust-lang#121114 - Nilstrieb:no-inline!, r=saethlin
Browse files Browse the repository at this point in the history
Add `#[rustc_no_mir_inline]` for standard library UB checks

should help with rust-lang#121110 and also with rust-lang#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`
  • Loading branch information
bors committed Feb 25, 2024
2 parents 89d8e31 + 81d7069 commit e9f9594
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 3 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 15 additions & 3 deletions library/core/src/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}

Original file line number Diff line number Diff line change
@@ -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;
}
}

Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
17 changes: 17 additions & 0 deletions tests/mir-opt/inline/rustc_no_mir_inline.rs
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit e9f9594

Please sign in to comment.