Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiler fails to remove jumps using jump table if every label in it is same #110797

Closed
AngelicosPhosphoros opened this issue Apr 25, 2023 · 5 comments · Fixed by #125347
Closed
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@AngelicosPhosphoros
Copy link
Contributor

I tried this code:

pub enum K{
    A(Box<[i32]>),
    B(Box<[u8]>),
    C(Box<[String]>),
    D(Box<[u16]>),
}

pub fn get_len(arg: &K)->usize{
    match arg {
        K::A(ref lst)=>lst.len(),
        K::B(ref lst)=>lst.len(),
        K::C(ref lst)=>lst.len(),
        K::D(ref lst)=>lst.len(),
    }
}

I expected to see this happen:

Since layout of each enum variant is same, I expected to get machine code without any jumps, e.g.

example::get_len:
        mov     rax, qword ptr [rdi + 16]
        ret

Instead, this happened:
Compiler generates jump table, every branch of it points to the same code, and code jumps using it despite being it redundant.

example::get_len:
        mov     rax, qword ptr [rdi]
        lea     rcx, [rip + .LJTI0_0]
        movsxd  rax, dword ptr [rcx + 4*rax]
        add     rax, rcx
        jmp     rax
.LBB0_1:
        mov     rax, qword ptr [rdi + 16]
        ret
.LJTI0_0:                                     ; Note that each of it jumps to same label at the end.
        .long   .LBB0_1-.LJTI0_0
        .long   .LBB0_1-.LJTI0_0
        .long   .LBB0_1-.LJTI0_0
        .long   .LBB0_1-.LJTI0_0

Meta

rustc --version --verbose:

rustc 1.68.0 (2c8cc3432 2023-03-06)
binary: rustc
commit-hash: 2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74
commit-date: 2023-03-06
host: x86_64-unknown-linux-gnu
release: 1.68.0
LLVM version: 15.0.6
Compiler returned: 0

Also, godbolt link.

@AngelicosPhosphoros AngelicosPhosphoros added the C-bug Category: This is a bug. label Apr 25, 2023
@taiki-e
Copy link
Member

taiki-e commented Apr 25, 2023

This seems a regression introduced in Rust 1.65: https://godbolt.org/z/7vnT1z67E

@rustbot label +regression-from-stable-to-stable

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 25, 2023
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium +T-compiler +I-slow

@rustbot rustbot added I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 25, 2023
@nikic nikic added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Apr 25, 2023
@nikic
Copy link
Contributor

nikic commented Apr 25, 2023

It's another typed GEP issue.

@AngelicosPhosphoros
Copy link
Contributor Author

llvm/llvm-project#62570

@erikdesjardins
Copy link
Contributor

Fixed in 1.73, presumably due to the LLVM 17 upgrade (#114048): https://godbolt.org/z/WTcKvcfh4

This specific example seems to be a backend optimization (the optimized IR is the same in 1.72 and 1.73); the general case is fixed in nightly by #121665: https://godbolt.org/z/cxnsWoWzx.

@rustbot label +E-needs-test

@rustbot rustbot added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Mar 5, 2024
tesuji added a commit to tesuji/rustc that referenced this issue May 20, 2024
tesuji added a commit to tesuji/rustc that referenced this issue Jun 8, 2024
tesuji added a commit to tesuji/rustc that referenced this issue Jun 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 10, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 11, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 13, 2024
@bors bors closed this as completed in 7ac6c2f Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-slow Issue: Problems and improvements with respect to performance of generated code. P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants