Codegen regression involving `assume`/`unreachable_unchecked`

[WaffleLapkin]

Code

I tried this code:

pub fn f(a: Option<u32>) -> u32 {
    match a {
        None => {}
        Some(_) => unsafe { std::hint::unreachable_unchecked() }
    }

    a.unwrap()
}

I expected to see this happen: no branch instructions emitted, unreachable_unchecked makes an assumption that a is None, so unwrap should be inlined and optimized as a panic.

Instead, this happened: there is a discriminant check still emitted.

Version it worked on

It most recently worked on: nightly-2023-03-26

Version with regression

; rustc +nightly-2023-03-27 --version --verbose
rustc 1.70.0-nightly (db0cbc48d 2023-03-26)
binary: rustc
commit-hash: db0cbc48d4aaa300713a95d9b317a365a474490c
commit-date: 2023-03-26
host: aarch64-unknown-linux-gnu
release: 1.70.0-nightly
LLVM version: 16.0.0

ASM diff

See godbolt: https://godbolt.org/z/5WKTY99Te

Bisection results

Regression occurred in #106428 (cc @saethlin).

searched nightlies: from nightly-2023-03-01 to nightly-2023-04-01
regressed nightly: nightly-2023-03-27
searched commit range: 0c61c7a...db0cbc4
regressed commit: 2420bd3

bisected with cargo-bisect-rustc v0.6.6

Host triple: aarch64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2023-03-01 --end=2023-04-01 --regress=success --script=./t.sh

t.sh

#!/bin/sh

set -ex

rustc t.rs --crate-type=lib --emit=asm -O && cat ./t.s | grep -E "cbz\s+w0, .LBB0_2"

(if you want to test on x86_64 replace "cbz\s+w0, .LBB0_2" with "testl\s+%edi, %edi")

t.rs

pub fn f(a: Option<i32>) -> i32 {
    unsafe { if !a.is_none() { std::hint::unreachable_unchecked() } }

    return a.unwrap();
}

History

@heckad originally noticed the problem and opened #110456, then @WaffleLapkin suggested that this is a compiler issue, then @bugadani noticed that this is a regression, then @WaffleLapkin bisected and opened this issue.

[saethlin]

searched nightlies: from nightly-2023-03-01 to nightly-2023-04-01

The end of this regression window is over 2 weeks from now. Why?

Godbolt usually has pretty old nightlies. If I add --emit=mir: https://godbolt.org/z/YfxfhETfb you can see we have duplicate switch targets, which means that either this commit is broken or godbolt doesn't have it: 2a628bd

[lqd]

godbolt seems to have rustc 1.71.0-nightly (d0f204e4d 2023-04-16) ?

[WaffleLapkin]

searched nightlies: from nightly-2023-03-01 to nightly-2023-04-01

The end of this regression window is over 2 weeks from now. Why?

Because I knew that 2023-04-01 reproduces the regression, and wanted to cut some time bisecting. For reference I can reproduce the same issue with +nightly-2023-04-19.

+nightly-2023-04-19 --emit=mir

// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
fn f(_1: Option<u32>) -> u32 {
    debug a => _1;                       // in scope 0 at t.rs:1:10: 1:11
    let mut _0: u32;                     // return place in scope 0 at t.rs:1:29: 1:32
    let mut _2: isize;                   // in scope 0 at t.rs:3:9: 3:13
    scope 1 {
        scope 2 (inlined unreachable_unchecked) { // at t.rs:4:28: 4:62
            scope 3 {
                scope 4 (inlined unreachable_unchecked::runtime) { // at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/intrinsics.rs:2495:13: 2495:80
                }
            }
        }
    }
    scope 5 (inlined #[track_caller] Option::<u32>::unwrap) { // at t.rs:7:7: 7:15
        debug self => _1;                // in scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:947:25: 947:29
        let mut _3: isize;               // in scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:949:13: 949:22
        let mut _4: !;                   // in scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:950:21: 950:73
        scope 6 {
            debug val => _0;             // in scope 6 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:949:18: 949:21
        }
    }

    bb0: {
        _2 = discriminant(_1);           // scope 0 at t.rs:2:11: 2:12
        switchInt(move _2) -> [0: bb2, 1: bb1, otherwise: bb1]; // scope 0 at t.rs:2:5: 2:12
    }

    bb1: {
        unreachable;                     // scope 3 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/intrinsics.rs:2480:9: 2496:10
    }

    bb2: {
        _3 = discriminant(_1);           // scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:948:15: 948:19
        switchInt(move _3) -> [0: bb3, 1: bb4, otherwise: bb1]; // scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:948:9: 948:19
    }

    bb3: {
        _4 = core::panicking::panic(const "called `Option::unwrap()` on a `None` value"); // scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:950:21: 950:73
                                         // mir::Constant
                                         // + span: /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:950:21: 950:26
                                         // + literal: Const { ty: fn(&'static str) -> ! {core::panicking::panic}, val: Value(<ZST>) }
                                         // mir::Constant
                                         // + span: /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:950:27: 950:72
                                         // + literal: Const { ty: &str, val: Value(Slice(..)) }
    }

    bb4: {
        _0 = ((_1 as Some).0: u32);      // scope 5 at /rustc/c609da59d9fc05b1c7dc879d79700ccd8140b5fc/library/core/src/option.rs:949:18: 949:21
        return;                          // scope 0 at t.rs:8:2: 8:2
    }
}

[saethlin]

To be clear, separate from this issue I believe that this MIR:

    bb0: {
        _2 = discriminant(_1);           // scope 0 at t.rs:2:11: 2:12
        switchInt(move _2) -> [0: bb2, 1: bb1, otherwise: bb1]; // scope 0 at t.rs:2:5: 2:12
    }

is a bug.

I do not know what impact resolving that would have on this issue. But the InstCombine helper that combines switch targets was (notionally) supposed to prevent this.

[saethlin]

Yeah, I'm pretty sure that is the root cause.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium