squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Auto merge of #104999 - saethlin:immediate-abort-inlining, r=thomcc

Browse source 
Adjust inlining attributes around panic_immediate_abort

The goal of `panic_immediate_abort` is to permit the panic runtime and formatting code paths to be optimized away. But while poking through some disassembly of a small program compiled with that option, I found that was not the case. Enabling LTO did address that specific issue, but enabling LTO is a steep price to pay for this feature doing its job.

This PR fixes that, by tweaking two things:
* All the slice indexing functions that we `const_eval_select` on get `#[inline]`. `objdump -dC` told me that originally some `_ct` functions could end up in an executable. I won't pretend to understand what's going on there.
* Normalize attributes across all `panic!` wrappers: use `inline(never) + cold` normally, and `inline` when `panic_immediate_abort` is enabled.

But also, with LTO and `panic_immediate_abort` enabled, this patch knocks ~709 kB out of the `.text` segment of `librustc_driver.so`. That is slightly surprising to me, my best theory is that this shifts some inlining earlier in compilation, enabling some subsequent optimizations. The size improvement of `librustc_driver.so` with `panic_immediate_abort` due to this patch is greater with LTO than without LTO, which I suppose backs up this theory.

I do not know how to test this. I would quite like to, because I think what this is solving was an accidental regression. This only works with `-Zbuild-std` which is a cargo flag, and thus can't be used in a rustc codegen test.

r? `@thomcc`

---

I do not seriously think anyone is going to use a compiler built with `panic_immediate_abort`, but I wanted a big complicated Rust program to try this out on, and the compiler is such.
This commit is contained in:
bors 2022-12-02 20:07:23 +00:00
commit 32e613bbaa
3 changed files with 26 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

25
library/core/src/panicking.rs
View file

@ -38,10 +38,9 @@ use crate::panic::{Location, PanicInfo};
/// site as much as possible (so that `panic!()` has as low an impact /// site as much as possible (so that `panic!()` has as low an impact
/// on (e.g.) the inlining of other functions as possible), by moving /// on (e.g.) the inlining of other functions as possible), by moving
/// the actual formatting into this shared place. /// the actual formatting into this shared place.
#[cold]
// If panic_immediate_abort, inline the abort call, // If panic_immediate_abort, inline the abort call,
// otherwise avoid inlining because of it is cold path. // otherwise avoid inlining because of it is cold path.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
#[lang = "panic_fmt"] // needed for const-evaluated panics #[lang = "panic_fmt"] // needed for const-evaluated panics
@ -67,8 +66,7 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
/// Like panic_fmt, but without unwinding and track_caller to reduce the impact on codesize. /// Like panic_fmt, but without unwinding and track_caller to reduce the impact on codesize.
/// Also just works on `str`, as a `fmt::Arguments` needs more space to be passed. /// Also just works on `str`, as a `fmt::Arguments` needs more space to be passed.
#[cold] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[rustc_nounwind] #[rustc_nounwind]
pub fn panic_str_nounwind(msg: &'static str) -> ! { pub fn panic_str_nounwind(msg: &'static str) -> ! {
@ -96,10 +94,9 @@ pub fn panic_str_nounwind(msg: &'static str) -> ! {
// above. // above.
/// The underlying implementation of libcore's `panic!` macro when no formatting is used. /// The underlying implementation of libcore's `panic!` macro when no formatting is used.
#[cold]
// never inline unless panic_immediate_abort to avoid code // never inline unless panic_immediate_abort to avoid code
// bloat at the call sites as much as possible // bloat at the call sites as much as possible
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
#[rustc_const_unstable(feature = "core_panic", issue = "none")] #[rustc_const_unstable(feature = "core_panic", issue = "none")]
@ -138,8 +135,8 @@ pub const fn panic_display<T: fmt::Display>(x: &T) -> ! {
panic_fmt(format_args!("{}", *x)); panic_fmt(format_args!("{}", *x));
} }
#[cold] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
#[lang = "panic_bounds_check"] // needed by codegen for panic on OOB array/slice access #[lang = "panic_bounds_check"] // needed by codegen for panic on OOB array/slice access
fn panic_bounds_check(index: usize, len: usize) -> ! { fn panic_bounds_check(index: usize, len: usize) -> ! {
@ -154,8 +151,8 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
/// ///
/// This function is called directly by the codegen backend, and must not have /// This function is called directly by the codegen backend, and must not have
/// any extra arguments (including those synthesized by track_caller). /// any extra arguments (including those synthesized by track_caller).
#[cold] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[inline(never)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function #[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function
#[rustc_nounwind] #[rustc_nounwind]
fn panic_no_unwind() -> ! { fn panic_no_unwind() -> ! {
@ -185,7 +182,8 @@ pub enum AssertKind {
} }
/// Internal function for `assert_eq!` and `assert_ne!` macros /// Internal function for `assert_eq!` and `assert_ne!` macros
#[cold] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
#[doc(hidden)] #[doc(hidden)]
pub fn assert_failed<T, U>( pub fn assert_failed<T, U>(
@ -202,7 +200,8 @@ where
} }
/// Internal function for `assert_match!` /// Internal function for `assert_match!`
#[cold] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
#[doc(hidden)] #[doc(hidden)]
pub fn assert_matches_failed<T: fmt::Debug + ?Sized>( pub fn assert_matches_failed<T: fmt::Debug + ?Sized>(
@ -221,6 +220,8 @@ pub fn assert_matches_failed<T: fmt::Debug + ?Sized>(
} }
/// Non-generic version of the above functions, to avoid code bloat. /// Non-generic version of the above functions, to avoid code bloat.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
fn assert_failed_inner( fn assert_failed_inner(
kind: AssertKind, kind: AssertKind,

21
library/core/src/slice/index.rs
View file

@ -31,9 +31,8 @@ where
} }
} }
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller] #[track_caller]
#[rustc_const_unstable(feature = "const_slice_index", issue = "none")] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")]
const fn slice_start_index_len_fail(index: usize, len: usize) -> ! { const fn slice_start_index_len_fail(index: usize, len: usize) -> ! {
@ -48,19 +47,20 @@ const fn slice_start_index_len_fail(index: usize, len: usize) -> ! {
} }
// FIXME const-hack // FIXME const-hack
#[inline]
#[track_caller] #[track_caller]
fn slice_start_index_len_fail_rt(index: usize, len: usize) -> ! { fn slice_start_index_len_fail_rt(index: usize, len: usize) -> ! {
panic!("range start index {index} out of range for slice of length {len}"); panic!("range start index {index} out of range for slice of length {len}");
} }
#[inline]
#[track_caller] #[track_caller]
const fn slice_start_index_len_fail_ct(_: usize, _: usize) -> ! { const fn slice_start_index_len_fail_ct(_: usize, _: usize) -> ! {
panic!("slice start index is out of range for slice"); panic!("slice start index is out of range for slice");
} }
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller] #[track_caller]
#[rustc_const_unstable(feature = "const_slice_index", issue = "none")] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")]
const fn slice_end_index_len_fail(index: usize, len: usize) -> ! { const fn slice_end_index_len_fail(index: usize, len: usize) -> ! {
@ -71,19 +71,20 @@ const fn slice_end_index_len_fail(index: usize, len: usize) -> ! {
} }
// FIXME const-hack // FIXME const-hack
#[inline]
#[track_caller] #[track_caller]
fn slice_end_index_len_fail_rt(index: usize, len: usize) -> ! { fn slice_end_index_len_fail_rt(index: usize, len: usize) -> ! {
panic!("range end index {index} out of range for slice of length {len}"); panic!("range end index {index} out of range for slice of length {len}");
} }
#[inline]
#[track_caller] #[track_caller]
const fn slice_end_index_len_fail_ct(_: usize, _: usize) -> ! { const fn slice_end_index_len_fail_ct(_: usize, _: usize) -> ! {
panic!("slice end index is out of range for slice"); panic!("slice end index is out of range for slice");
} }
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller] #[track_caller]
#[rustc_const_unstable(feature = "const_slice_index", issue = "none")] #[rustc_const_unstable(feature = "const_slice_index", issue = "none")]
const fn slice_index_order_fail(index: usize, end: usize) -> ! { const fn slice_index_order_fail(index: usize, end: usize) -> ! {
@ -92,27 +93,27 @@ const fn slice_index_order_fail(index: usize, end: usize) -> ! {
} }
// FIXME const-hack // FIXME const-hack
#[inline]
#[track_caller] #[track_caller]
fn slice_index_order_fail_rt(index: usize, end: usize) -> ! { fn slice_index_order_fail_rt(index: usize, end: usize) -> ! {
panic!("slice index starts at {index} but ends at {end}"); panic!("slice index starts at {index} but ends at {end}");
} }
#[inline]
#[track_caller] #[track_caller]
const fn slice_index_order_fail_ct(_: usize, _: usize) -> ! { const fn slice_index_order_fail_ct(_: usize, _: usize) -> ! {
panic!("slice index start is larger than end"); panic!("slice index start is larger than end");
} }
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller] #[track_caller]
const fn slice_start_index_overflow_fail() -> ! { const fn slice_start_index_overflow_fail() -> ! {
panic!("attempted to index slice from after maximum usize"); panic!("attempted to index slice from after maximum usize");
} }
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[cold]
#[track_caller] #[track_caller]
const fn slice_end_index_overflow_fail() -> ! { const fn slice_end_index_overflow_fail() -> ! {
panic!("attempted to index slice up to maximum usize"); panic!("attempted to index slice up to maximum usize");

4
library/std/src/panicking.rs
View file

@ -594,8 +594,8 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
// lang item for CTFE panic support // lang item for CTFE panic support
// never inline unless panic_immediate_abort to avoid code // never inline unless panic_immediate_abort to avoid code
// bloat at the call sites as much as possible // bloat at the call sites as much as possible
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cold] #[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller] #[track_caller]
#[rustc_do_not_const_check] // hooked by const-eval #[rustc_do_not_const_check] // hooked by const-eval
pub const fn begin_panic<M: Any + Send>(msg: M) -> ! { pub const fn begin_panic<M: Any + Send>(msg: M) -> ! {