Bad codegen using `u16::to/from_be_bytes`
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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