rust-lang / rust-lang/rust

Bad codegen using `u16::to/from_be_bytes`

Open
#126,419 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-codegen C-bug C-optimization T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

I wrote code to increment a big-endian u16 within a circular buffer of u8 values. There are two implementations: one using u16::to/from_be_bytes, and the other using a naive mask-and-shift implementation.

pub fn incr_u16_a(slice: &mut [u8; 256], i: u8) {
    let hi = slice[i as usize];
    let lo = slice[i.wrapping_add(1) as usize];
    let mut v = u16::from_be_bytes([hi, lo]);
    v = v.wrapping_add(1);
    let [hi, lo] = v.to_be_bytes();
    slice[i as usize] = hi;
    slice[i.wrapping_add(1) as usize] = lo;
}

pub fn incr_u16_b(slice: &mut [u8; 256], i: u8) {
    let hi = slice[i as usize];
    let lo = slice[i.wrapping_add(1) as usize];
    let mut v = ((hi as u16) << 8) | (lo as u16);
    v = v.wrapping_add(1);
    let hi = (v >> 8) as u8;
    let lo = v as u8;
    slice[i as usize] = hi;
    slice[i.wrapping_add(1) as usize] = lo;
}

I was expecting both versions to be basically equivalent.

Instead, the version using u16::to/from_be_bytes generates a bunch of extra instructions

incr_u16_a:
        add     w8, w1, #1
        and     x9, x1, #0xff
        and     x8, x8, #0xff
        ldrb    w10, [x0, x9]
        ldrb    w11, [x0, x8]
        orr     w10, w10, w11, lsl #8
        rev     w10, w10
        lsr     w10, w10, #16
        add     w10, w10, #1
        rev     w10, w10
        lsr     w11, w10, #16
        lsr     w10, w10, #24
        strb    w11, [x0, x9]
        strb    w10, [x0, x8]
        ret

incr_u16_b:
        add     w8, w1, #1
        and     x9, x1, #0xff
        and     x8, x8, #0xff
        ldrb    w10, [x0, x9]
        ldrb    w11, [x0, x8]
        orr     w10, w11, w10, lsl #8
        add     w10, w10, #1
        strb    w10, [x0, x8]
        lsr     w11, w10, #8
        strb    w11, [x0, x9]
        ret

(Godbolt link, running using --edition 2021 -O --target=aarch64-unknown-linux-gnu)

There's similar inefficiency when compiling for x86

incr_u16_a:
        movzx   edx, sil
        mov     eax, edx
        inc     al
        movzx   eax, al
        movzx   esi, byte ptr [rdi + rdx]
        movzx   ecx, byte ptr [rdi + rax]
        shl     ecx, 8
        or      ecx, esi
        rol     cx, 8
        inc     ecx
        rol     cx, 8
        mov     byte ptr [rdi + rdx], cl
        mov     byte ptr [rdi + rax], ch
        ret

incr_u16_b:
        movzx   eax, sil
        mov     ecx, eax
        inc     cl
        movzx   ecx, cl
        movzx   edx, byte ptr [rdi + rcx]
        movzx   esi, byte ptr [rdi + rax]
        shl     esi, 8
        add     edx, esi
        inc     edx
        mov     byte ptr [rdi + rax], dh
        mov     byte ptr [rdi + rcx], dl
        ret

Looking at the extra instructions, I suspect that the call to intrinsics::bswap (deep in uint_macros.rs) is failing to be optimized out.

Meta

rustc --version --verbose:

rustc 1.78.0 (9b00956e5 2024-04-29)

This also happens on nightly (tested on Godbolt)

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.

Research direction

Start with the Godbolt reproducer and compare the AArch64 and x86 output for the two functions. Then inspect core/num/uint_macros.rs around the referenced intrinsics::bswap implementation. Done means the to/from_be_bytes version no longer emits the extra byte-swap and shift instructions shown in the report.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers, performance
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.