Why is panic code inlined?

1

When looking at RefCell implementation, I stumbled upon those lines rust/library/core/src/cell.rs at master · rust-lang/rust · GitHub, particularly this comment:

This ensures the panicking code is outlined from borrow_mut for RefCell.

In other words, RefCell::borrow/RefCell::borrow_mut are marked with #[inline], but for best performance I guess, the developer had to extract the panic operation into a dedicated #[cold] #[inline(never)] function, instead of using Result::unwrap or panic! directly.

That made me wonder: why isn't panicking code outlined by default? I mean, panicking path should always be cold, and shouldn't blow up the instruction cache.
I've made a simple comparison on godbolt: using panic! instead of a custom outlined panic wrapper increase my example function assembly by 13 instructions! And I don't see any benefit for this.

Really, I don't understand why inlining is the default behavior. Does someone has an explanation?

2

Are you asking why this isn't covered by an existing optimization pass, or are you hinting at a possible change in the implementation of panic! itself?

3

I know neither how panic! is implemented, nor if their is optimization passes after panic! replacement, so I don't have any answer to your question; I'm just asking if this 13 additional assembly instructions are worth it compared to a non-inlined cold function call, as I'm surprised about the comment I've found in the standard library.

4

Using cargo expand I can see that this

panic!();
panic!("payload");

expands to this

{
    #[cold]
    #[track_caller]
    #[inline(never)]
    const fn panic_cold_explicit() -> ! {
        ::core::panicking::panic_explicit()
    }
    panic_cold_explicit();
};
{
    ::core::panicking::panic_fmt(format_args!("payload"));
};

So a payload-less call to panic! is cold and inline-never.

Similarly when a payload is provided, panic! delegates to panic_fmt which also appears to be cold and inline-never. However, note that the call to format_args! will be inlined, and that probably explains the code size difference in your godbolt example.

6 Likes
5

And you can see that if I remove the panic message, it does indeed leave the panic code cold. It's only ever the panic_fmt that's problematic, and that's because the formatting code is inlined, followed by a call to the out-of-line ::core::panicking::panic_fmt.

I don't believe that there's a good way to force the format_args! call to be out-of-line, unfortunately.

2 Likes
6

The macro could expand into "call this cold closure which captures the necessary arguments":

(#[cold] move || panic!("{err:?}"))()

godbolt

8 Likes
7

I think the comment is leading you slightly astray.

The more important thing that function is doing is not being generic.

You don't want every RefCell<T>::borrow_mut to have its own copy of the panic -- even if it's outlined -- you want one copy of the panicking code, called by all of them.

So I would consider it more like Monomorphize less code in fs::{read|write} by scottmcm · Pull Request #58530 · rust-lang/rust · GitHub -- avoiding monomorphization being the important part.

4 Likes
8

This was actually tried: Outline formatting code from the panic! macro by Zoxc · Pull Request #115562 · rust-lang/rust · GitHub.

5 Likes
9

Right, naively expanding to a closure would break something like

panic!("{}", return 42)

Also, capturing the arguments into a closure ends up doing unnecessary work just moving stuff around into whatever layout and calling convention is chosen for the closure. But that could potentially be improved if such single-local-use closures could have a calling convention of "wherever the captures are in the parent's frame".

10

Would it break downstream code like this if the arguments were passed to the closure as arguments, instead of being captured?

11

maybe

return (#[cold] move || panic!("{}", return 42))()

is enough.

#![feature(stmt_expr_attributes)]
fn test(i:i32)->i32{
    return (#[cold] move || panic!("expect 42, got {}",if i==42 {return 42} else {i}))()
}
fn main(){
    let a=test(42);
    println!("{a}");
    test(0);
}
12

I don't think there's any transformation using a closure that will work in all cases, because it makes things inter-procedural, and there are rust analyses that are only intra-procedural.

2 Likes
13

Indeed, closure will make things like break and continue unusable.

14

Would something like this works?

panic!("{} - {named:?} - {:?} - {other}", 0, named = return 42, 1, other = 2);
// converted into
{
    #[inline(never)]
    #[cold]
    fn panic(
        __arg1: impl ::core::fmt::Display,
        named: impl ::core::fmt::Debug,
        __arg2: impl ::core::fmt::Debug,
        other: impl ::core::fmt::Display
    ) -> ! {
        ::core::panicking::panic_fmt(
            format_args!("{} - {named:?} - {:?} - {other}", __arg1, named = named, __arg2, other = other)
        );
    }
    panic(0, return 42, 1, 2);
}

EDIT: Ok, it seems that's more or less what someone tried to do in #115562, but also taking constness in account. So I guess we just need to wait #115562 to be resolved.

15

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.