`if matches!(...)` results in worse mir than `if let ...`

[lcnr]

This is probably something we can and should optimize:

pub enum Foo {
    A,
    B,
    C,
    D,
    E,
    F,
}

pub fn matches(num: Foo) -> u32 {
    if matches!(num, Foo::B | Foo::C) {
        23
    } else {
        42
    }
}


pub fn if_let(num: Foo) -> u32 {
    if let Foo::B | Foo::C = num {
        23
    } else {
        42
    }
}

results in

fn matches(_1: Foo) -> u32 {
    debug num => _1;                     // in scope 0 at ./example.rs:10:16: 10:19
    let mut _0: u32;                     // return place in scope 0 at ./example.rs:10:29: 10:32
    let mut _2: bool;                    // in scope 0 at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/macros/mod.rs:246:9: 249:10
    let mut _3: isize;                   // in scope 0 at ./example.rs:11:22: 11:28

    bb0: {
        StorageLive(_2);                 // scope 0 at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/macros/mod.rs:246:9: 249:10
        _3 = discriminant(_1);           // scope 0 at ./example.rs:11:22: 11:28
        switchInt(move _3) -> [1_isize: bb2, 2_isize: bb2, otherwise: bb1]; // scope 0 at ./example.rs:11:22: 11:28
    }

    bb1: {
        _2 = const false;                // scope 0 at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/macros/mod.rs:248:18: 248:23
        goto -> bb3;                     // scope 0 at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/macros/mod.rs:246:9: 249:10
    }

    bb2: {
        _2 = const true;                 // scope 0 at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/macros/mod.rs:247:48: 247:52
        goto -> bb3;                     // scope 0 at /rustc/7f7a1cbfd3b55daee191247770627afab09eece2/library/core/src/macros/mod.rs:246:9: 249:10
    }

    bb3: {
        switchInt(_2) -> [false: bb4, otherwise: bb5]; // scope 0 at ./example.rs:11:5: 15:6
    }

    bb4: {
        _0 = const 42_u32;               // scope 0 at ./example.rs:14:9: 14:11
        goto -> bb6;                     // scope 0 at ./example.rs:11:5: 15:6
    }

    bb5: {
        _0 = const 23_u32;               // scope 0 at ./example.rs:12:9: 12:11
        goto -> bb6;                     // scope 0 at ./example.rs:11:5: 15:6
    }

    bb6: {
        StorageDead(_2);                 // scope 0 at ./example.rs:16:1: 16:2
        return;                          // scope 0 at ./example.rs:16:2: 16:2
    }
}

fn if_let(_1: Foo) -> u32 {
    debug num => _1;                     // in scope 0 at ./example.rs:19:15: 19:18
    let mut _0: u32;                     // return place in scope 0 at ./example.rs:19:28: 19:31
    let mut _2: isize;                   // in scope 0 at ./example.rs:20:12: 20:18

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

    bb1: {
        _0 = const 42_u32;               // scope 0 at ./example.rs:23:9: 23:11
        goto -> bb3;                     // scope 0 at ./example.rs:20:5: 24:6
    }

    bb2: {
        _0 = const 23_u32;               // scope 0 at ./example.rs:21:9: 21:11
        goto -> bb3;                     // scope 0 at ./example.rs:20:5: 24:6
    }

    bb3: {
        return;                          // scope 0 at ./example.rs:25:2: 25:2
    }
}

https://godbolt.org/z/8K79o8

[scottmcm]

Any opinions on what the best fix here would be? Maybe look for gotos to blocks that are only terminators, and duplicate the terminator? (But maybe not for calls, because more call sites can make other things worse?)

Then other existing simplification and const prop passes could do the rest...

[simonvandel]

I don't know if the following is the best fix, but at least it's some form of simplication.

If we look for bbs with the pattern "set bool, go to bb that only switches on that bool", we would find bb1 and bb2 that both jump to bb3. It should then be fairly easy to see that bb1 can just jump to bb4 and bb2 can jump to bb5.

I don't know if this is enough of a simplification to allow the other passes to do the rest, but at least it should allow the removal of bb3 and probably also _2 which then seems unused. By that time I would think simplifycfg would replace bb1 with bb4 and bb2 with bb5.

[lcnr]

see the relevant discussion on zulip: https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/optimizing.20matches/near/211759874

@oli-obk proposed extending SimplifyComparisonIntegral to deal with a == constant || b == constant which would then allow us to optimize this further, I think that's similar to what @simonvandel mentioned here.