rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.26k stars 12.57k forks source link

Inefficient assembly for semantically equivalent way of matching on enum #85841

Open e00E opened 3 years ago

e00E commented 3 years ago

https://godbolt.org/z/fYfohcsax

#[derive(Clone, Copy)]
pub enum E {
    E0,
    E1,
    E2,
    E3,
}

impl E {
    pub fn next0(self) -> Option<Self> {
        match self {
            E::E0 => Some(E::E1),
            E::E1 => Some(E::E2),
            E::E2 => Some(E::E3),
            E::E3 => None,
        }
    }

    pub fn next1(self) -> Option<Self> {
        Some(match self {
            E::E0 => E::E1,
            E::E1 => E::E2,
            E::E2 => E::E3,
            E::E3 => return None,
        })
    }
}

Optimized assembly on stable 1.52.0 and nightly:

example::E::next0:
        lea     eax, [rdi + 1]
        ret

example::E::next1:
        mov     al, 4
        mov     cl, 1
        movzx   edx, dil
        lea     rsi, [rip + .LJTI1_0]
        movsxd  rdx, dword ptr [rsi + 4*rdx]
        add     rdx, rsi
        jmp     rdx
.LBB1_1:
        mov     cl, 2
        jmp     .LBB1_3
.LBB1_2:
        mov     cl, 3
.LBB1_3:
        mov     eax, ecx
.LBB1_4:
        ret
.LJTI1_0:
        .long   .LBB1_3-.LJTI1_0
        .long   .LBB1_1-.LJTI1_0
        .long   .LBB1_2-.LJTI1_0
        .long   .LBB1_4-.LJTI1_0

I expected next1 to compile to the same assembly as next0.

Edit 2022-12-01: Not fixed on current stable 1.65 and nightly.

JulianKnodt commented 3 years ago

If you look at the emitted mir, I think what's causing this is that an extra temporary is used to store the enum type and is not const as is marked in next0. I'm curious why the 2nd implementation doesn't have E::E1-3 marked as const?

nikic commented 1 year ago

Upstream issue: https://github.com/llvm/llvm-project/issues/63876