rust-lang / rust-lang/rust

Matching on ASCII digits no longer optimized

Open
#123,305 2 comments 2 reactions 1 assignee View on GitHub

@krtab is already working on this.

Since Mar 31, 2024.

A-codegen A-LLVM C-bug C-optimization I-slow P-medium regression-from-stable-to-stable T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Code

This

pub fn f(character: u8) -> Option<u8> {
    match character {
        b'0' => Some(0),
        b'1' => Some(1),
        b'2' => Some(2),
        b'3' => Some(3),
        b'4' => Some(4),
        b'5' => Some(5),
        b'6' => Some(6),
        b'7' => Some(7),
        b'8' => Some(8),
        b'9' => Some(9),
        _ => None,
    }
}

used in 1.76.0 to be optimized to

f:
        lea     edx, [rdi - 48]
        cmp     dl, 10
        setb    al
        ret

but in 1.77 and nightly, this remains

f:
        movzx   eax, dil
        add     eax, -48
        cmp     eax, 9
        ja      .LBB0_3
        lea     rcx, [rip + .LJTI0_0]
        movsxd  rax, dword ptr [rcx + 4*rax]
        add     rax, rcx
        jmp     rax
.LBB0_2:
        mov     al, 1
        xor     edx, edx
        ret
.LBB0_3:
        xor     eax, eax
        ret
.LBB0_5:
        mov     dl, 1
        mov     al, 1
        ret
.LBB0_6:
        mov     al, 1
        mov     dl, 2
        ret
.LBB0_7:
        mov     al, 1
        mov     dl, 3
        ret
.LBB0_8:
        mov     al, 1
        mov     dl, 4
        ret
.LBB0_9:
        mov     al, 1
        mov     dl, 5
        ret
.LBB0_10:
        mov     al, 1
        mov     dl, 6
        ret
.LBB0_11:
        mov     al, 1
        mov     dl, 7
        ret
.LBB0_12:
        mov     al, 1
        mov     dl, 9
        ret
.LBB0_13:
        mov     al, 1
        mov     dl, 8
        ret
.LJTI0_0:
        .long   .LBB0_2-.LJTI0_0
        .long   .LBB0_5-.LJTI0_0
        .long   .LBB0_6-.LJTI0_0
        .long   .LBB0_7-.LJTI0_0
        .long   .LBB0_8-.LJTI0_0
        .long   .LBB0_9-.LJTI0_0
        .long   .LBB0_10-.LJTI0_0
        .long   .LBB0_11-.LJTI0_0
        .long   .LBB0_13-.LJTI0_0
        .long   .LBB0_12-.LJTI0_0

Interestingly,

pub fn f(character: u8) -> Option<u32> {
    match character {
        b'0' => Some(0),
        b'1' => Some(1),
        b'2' => Some(2),
        b'3' => Some(3),
        b'4' => Some(4),
        b'5' => Some(5),
        b'6' => Some(6),
        b'7' => Some(7),
        b'8' => Some(8),
        b'9' => Some(9),
        _ => None,
    }
}

and

pub fn f(character: u8) -> Option<u8>
{
    match character {
        b'0' => Some(0),
        b'1' => Some(1),
        b'2' => Some(2),
        b'3' => Some(3),
        b'4' => Some(4),
        b'5' => Some(5),
        b'6' => Some(6),
        b'7' => Some(7),
        //b'8' => Some(8),
        //b'9' => Some(9),
        _ => None,
    }
}

Get well optimized.

Bisected to #118991

@rustbot modify labels: +regression-from-stable-to-stable

I'd like to have a go at this so @rustbot claim

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.