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 an option to avoid merging of calls to panic!() #63105

Open
jrmuizel opened this issue Jul 29, 2019 · 3 comments
Open

Add an option to avoid merging of calls to panic!() #63105

jrmuizel opened this issue Jul 29, 2019 · 3 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@jrmuizel
Copy link
Contributor

Given code like:

pub fn f(v: Option<u32>, k: Option<u32>)  -> u32{
    v.unwrap() + k.unwrap()
}

Rust generates

example::f:
        push    rax
        test    edi, edi
        je      .LBB0_3
        test    edx, edx
        je      .LBB0_3
        mov     eax, ecx
        add     eax, esi
        pop     rcx
        ret
.LBB0_3:
        lea     rdi, [rip + .L__unnamed_1]
        call    qword ptr [rip + core::panicking::panic@GOTPCREL]
        ud2

The two calls to wrap are unified and it's not possible to tell which one failed at runtime.

I requested a way to tell LLVM not to do this in https://bugs.llvm.org/show_bug.cgi?id=42783 and Reid pointed me at https://cs.chromium.org/chromium/src/base/immediate_crash.h?l=10&rcl=92d78c90964200453b80fe098b367e0838681d09 which is Chrome's attempt to achieve this.

It turns out that this approach is pretty easily adapted to Rust. Here's a quick rework of the code above showing it off:

#![feature(asm)]

pub enum FOption<T> {
    Some(T),
    None
}

impl<T> FOption<T> {
     pub fn unwrap(self) -> T {
        match self {
            FOption::Some(val) => val,
            FOption::None => {
                unsafe { asm!("" : : : : "volatile") }
                panic!("called `Option::unwrap()` on a `None` value")
            }
        }
    }
}

pub fn f(v: FOption<u32>, k: FOption<u32>)  -> u32{
    v.unwrap() + k.unwrap()
}

which generates:

example::f:
        push    rax
        test    edi, edi
        jne     .LBB7_3
        test    edx, edx
        jne     .LBB7_4
        mov     eax, ecx
        add     eax, esi
        pop     rcx
        ret
.LBB7_3:
        call    std::panicking::begin_panic
        ud2
.LBB7_4:
        call    std::panicking::begin_panic
        ud2
@Gankra
Copy link
Contributor

Gankra commented Jul 29, 2019

Note that naively this doesn't work, because unwrap is just a normal function and it's pre-compiled before the end-user gets to specify optimization flags. The only way this could sort of work is to have a Very Magic intrinsic which is only fully evaluated at monomorphization time. Then we would have a chance to look at the optimization flags and determine whether we should have it expand to a nop or asm. Note that even with this, unwraps inside std would still get folded, but user-defined ones wouldn't, which I think matters more.

There's also a bit of an issue that there's no good place to slot it in our cargo profiles, since we basically only have "full debug" or "full release". Like no matter what it should be its own -C flag, but it would be nice if there was a commonly used Turbo Release vs Mostly Release (O2 vs O3) dichotomy that we could slip this difference into.

@cramertj
Copy link
Member

#47809 could potentially help with this.

@jonas-schievink jonas-schievink added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Aug 6, 2019
@jrmuizel
Copy link
Contributor Author

jrmuizel commented Oct 7, 2023

LLVM has a nomerge attribute that can be used to achieve this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants