Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
Loading