Repository navigation
Common branch folding for enum types fails when there are more than three enum variants. #121719
Description
I tried this code:
https://godbolt.org/z/Yh7Px49jh
#[repr(C)]
pub struct A {
x: f64,
y: u64,
}
#[repr(C)]
pub struct B {
x: f64,
y: u32,
}
#[repr(C)]
pub struct C {
x: f64,
y: u16,
}
#[repr(C)]
pub struct D {
x: f64,
y: u8,
}
pub enum E {
A(A),
B(B),
C(C),
D(D),
}
impl E {
#[inline(never)]
pub fn x(&self) -> &f64 {
match self {
E::A(A { x, .. }) | E::B(B { x, .. }) | E::C(C { x, .. }) | E::D(D { x, .. }) => x,
}
}
}I expected to see this happen:
The E::x function gets optimized into a single add instruction since all the field has the same offset
Instead, this happened:
When the enum has four or more variants, the asm generated is
example::E::x:
mov rax, rdi
mov rcx, qword ptr [rdi]
lea rdx, [rip + .LJTI0_0]
movsxd rcx, dword ptr [rdx + 4*rcx]
add rcx, rdx
jmp rcx
.LBB0_1:
add rax, 8
ret
.LJTI0_0:
.long .LBB0_1-.LJTI0_0
.long .LBB0_1-.LJTI0_0
.long .LBB0_1-.LJTI0_0
.long .LBB0_1-.LJTI0_0
When the enum has three variants, the folding took effects but still generates strange stuff
example::E::x:
mov rax, rdi
mov rcx, qword ptr [rdi]
test rcx, rcx
je .LBB0_2
cmp ecx, 1
.LBB0_2:
add rax, 8
ret
When it has two variants, it finally produces idiomatic code
example::E::x:
lea rax, [rdi + 8]
ret
Interestingly, rustc will generate idiomatic code when:
- Enum variants not exceeding three
- Enum variants share the same layout. As in the demo the field
yis used to disrupt the layout. - (Edit) Rustc version <=1.64.
Meta
godbold rustc 1.76.0 with -C opt-level=3 -C target-cpu=x86-64-v3
Edit: Tried more godbolt settings. This is a regression introduced sometime between rust 1.64 and rust 1.65.
Activity
ZhennanWu commentedon Feb 28, 2024
@jieyouxu Please add regression label as rust 1.64 can optimize this code. Rust 1.64 even managed to produce idiomatic code for my project with 11 complex enum variants.
dianqk commentedon Feb 28, 2024
#121665 might fix this issue.
erikdesjardins commentedon Feb 28, 2024
Indeed it does, added a test in 4016510
apiraino commentedon Feb 28, 2024
WG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-medium +I-slow
Auto merge of rust-lang#121665 - erikdesjardins:ptradd, r=nikic
Auto merge of rust-lang#121665 - erikdesjardins:ptradd, r=nikic
Auto merge of rust-lang#121665 - erikdesjardins:ptradd, r=nikic