Why is the code size of catch_unwind so large ?

[gnzlbg]

While filling #64222 I noticed that we generate more code than C++ for catch_unwind. That did not feel right, since C++'s catch can do much more than Rust's catch unwind, e.g., filtering different types of exceptions, etc.

MWE: C++ (https://gcc.godbolt.org/z/z_dgPg):

extern "C" void foo();

int bar() {
    try {
        foo();
        return 42;
    } catch(...) {
        return 13;
    }
}

generates

bar(): # @bar()
  push rbx
  mov ebx, 42
  call foo
  mov eax, ebx
  pop rbx
  ret
  mov rdi, rax
  call __cxa_begin_catch
  call __cxa_end_catch
  mov ebx, 13
  mov eax, ebx
  pop rbx
  ret

while Rust (https://gcc.godbolt.org/z/4sbc6k):

#![feature(unwind_attributes)]

extern "C" {
    // can unwind:
    #[unwind(allow)] fn foo(); 
}

pub unsafe fn bar() -> i32 {
    std::panic::catch_unwind(|| { foo(); 42 }).unwrap_or(13)
}

generates

example::bar:
  push rbp
  push r14
  push rbx
  sub rsp, 32
  mov qword ptr [rsp + 16], 0
  mov qword ptr [rsp + 24], 0
  lea rdi, [rip + std::panicking::try::do_call]
  lea rsi, [rsp + 12]
  lea rdx, [rsp + 16]
  lea rcx, [rsp + 24]
  call qword ptr [rip + __rust_maybe_catch_panic@GOTPCREL]
  test eax, eax
  je .LBB2_1
  mov rdi, -1
  call qword ptr [rip + std::panicking::update_panic_count@GOTPCREL]
  mov r14, qword ptr [rsp + 16]
  mov rbx, qword ptr [rsp + 24]
  mov rdi, r14
  call qword ptr [rbx]
  mov rsi, qword ptr [rbx + 8]
  mov ebp, 13
  test rsi, rsi
  je .LBB2_5
  mov rdx, qword ptr [rbx + 16]
  mov rdi, r14
  call qword ptr [rip + __rust_dealloc@GOTPCREL]
  jmp .LBB2_5
.LBB2_1:
  mov ebp, dword ptr [rsp + 12]
.LBB2_5:
  mov eax, ebp
  add rsp, 32
  pop rbx
  pop r14
  pop rbp
  ret
  mov rbp, rax
  mov rdi, r14
  mov rsi, rbx
  call alloc::alloc::box_free
  mov rdi, rbp
  call _Unwind_Resume@PLT
  ud2

This appears to be a constant overhead every time catch_unwind is used (e.g. see https://gcc.godbolt.org/z/bAvN24). Maybe we are inlining too much ?

[Mark-Simulacrum]

I think the first example with try/catch is a bit misleading. std::panic::catch_unwind essentially implements something like the try_run function here. This generates assembly that is relatively similar to the Rust assembly.

One notable difference between try/catch and std::panic::catch_unwind is that the first provides an option to not actually capture the exception (like your code illustrates). That is something that Rust today cannot do -- which is quite unfortunate, as that is a nonzero codesize burden due to needing to deallocate the Box. If we forget it explicitly instead, then we get somewhat nicer assembly: https://gcc.godbolt.org/z/vs6URt.

I'm not sure it matters too much in practice as I'd guess that catch_unwind which ignores the exception itself is somewhat rare, though it might be worth trying to figure out a way that we can thread that information through (it would probably need a second catch_unwind function -- otherwise, I don't think we can thread the information through into the function in anyway, just due to its return type).

Separately, C++ also can get away without the separate do_call function/symbol that Rust currently uses. This is because we currently thread through a function pointer to the lambda, whereas C++ gets away without this overhead due to being able to jump directly to a separate basic block from it's equivalent of a try intrinsic. I think this would be quite hard to replicate in Rust; we need essentially something like a naked function or so (but that's not quite right either), and perhaps to inform LLVM that said function will always be called, i.e., it can be generated directly into the instruction stream, vs. a call instruction to jump to it. I think it is not implausible that we could better express this if __rust_maybe_catch_panic was not a libary-implemented function, but rather an intrinsic.

I'm not sure it's worth optimizing this, though, as catch_unwind is much more rarely called than try/catch in C++, I expect. I have opened a PR (#67502) that slightly optimizes the ABI of __rust_maybe_catch_panic which reduces the stack space utilization and simplifies the code a little; this likely has essentially no runtime impact though.

[gnzlbg]

I'm not sure it matters too much in practice as I'd guess that catch_unwind which ignores the exception itself is somewhat rare

Good point. Would there be a way to insert the forget automatically if the result is not used?

and perhaps to inform LLVM that said function will always be called

When would this function not be unconditionally called with the current codegen ? (i.e. is this an LLVM bug, in which LLVM is not recognizing that this function is unconditionally called?).

[Mark-Simulacrum]

Well, the forget is a memory leak, so I doubt you'd want to auto insert it.

Unfortunately I don't think we can really blame llvm here - we're indirecting through an extern C function so llvm basically can't inline anything here (which would be needed I imagine to see that the function is always called). Furthermore it's not even the first thing to be called in the non-aborting case.

We do likely have a way out - move the whole catch function to an intrinsic. Then we'd likely get much better results, particularly with -Cpanic=abort, and it wouldn't even be all that hard I imagine to do this.

I continue to be unsure that it's worth it. I do recall that Servo mentioned that catch unwind used to be a performance problem for them; I'm not sure if it still is. Maybe @Manishearth knows, or can find out?

[Manishearth]

I don't really recall much about that.

[Mark-Simulacrum]

Hm, maybe @jdm can say more here (they filed #34727 way back). We got catch_unwind down to a "pretty fast" function there, though.

#64222 is basically the same as this issue we can't optimize out the unwind code path in the "does not unwind" case because we don't tell Rust that that's the case.

[jdm]

I have no data showing that catch_unwind is currently a performance problem, if that's the question being asked.

[gnzlbg]

The problem being reported here is that Rust's catch_unwind generates 3x as much code as a C++ try/catch in one particular case at least - for the data, see the first post of this issue. Even for cases like this one suggested by @Mark-Simulacrum above, we still generate 2x as much code as C++.

[Amanieu]

Could we get better codegen if we let __rust_maybe_catch_panic get inlined? LLVM should then be able to move most of the work into the catch path and improve the performance of the fast path. I'm not sure if this will help with code size though.

[Mark-Simulacrum]

__rust_maybe_catch_panic is what dispatches between panic=abort and panic=unwind currently; inlining it is possible but not entirely trivial. If we care about code size here, I have a number of ideas I can explore, but I would like to avoid spending the time unless we can find someone who has at least some level of "I care" about this :)

[Amanieu]

Here's what I got by inlining __rust_maybe_catch_panic: https://gcc.godbolt.org/z/AS2i4a

example::bar:
  push r14
  push rbx
  sub rsp, 24
  mov ebx, 42
  call qword ptr [rip + foo@GOTPCREL]
.LBB2_6:
  mov eax, ebx
  add rsp, 24
  pop rbx
  pop r14
  ret
  mov rbx, qword ptr [rax + 256]
  mov r14, qword ptr [rax + 264]
  mov qword ptr [rax + 256], 0
  mov rdi, rax
  call qword ptr [rip + _Unwind_DeleteException@GOTPCREL]
  mov qword ptr [rsp + 8], rbx
  mov qword ptr [rsp + 16], r14
  test rbx, rbx
  je .LBB2_2
  mov edi, -1
  call qword ptr [rip + update_panic_count@GOTPCREL]
  mov ebx, 13
  jmp .LBB2_6
.LBB2_2:
  lea rdi, [rip + .L__unnamed_1]
  lea rdx, [rip + .L__unnamed_2]
  mov esi, 43
  call qword ptr [rip + core::panicking::panic@GOTPCREL]
  ud2
  mov rbx, rax
  lea rdi, [rsp + 8]
  call core::ptr::real_drop_in_place
  mov rdi, rbx
  call _Unwind_Resume@PLT
  ud2

You'll note that the fast path is now almost as efficient as the C++ one (we're just allocating more stack space), and I'm sure we can reduce the slow path by forcing it out to a separate function.

I think we should make catch_unwind independent of the panic runtime and simply always catch unwinding. With inlining LLVM will be able to eliminate the catch path when compiling with panic=abort.

[Mark-Simulacrum]

The inlining (and some further work) worked out to produce a PR that @Amanieu and I independently came up with, though we've decided to utilize my previous PR as a base (#67502). That PR is marked as resolving this issue, as well as #64222. Interested parties can take a look at the implementation there which we're still iterating on.