Skip to content

Unnecessary comparison and conditional branch #65333

New issue

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

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

Already on GitHub? # to your account

Closed
tspiteri opened this issue Oct 12, 2019 · 3 comments
Closed

Unnecessary comparison and conditional branch #65333

tspiteri opened this issue Oct 12, 2019 · 3 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tspiteri
Copy link
Contributor

This is a reopening of #37114 which was fixed in 1.28.0 but seems to have regressed again since 1.30.0.

pub fn foo(v: &mut Vec<i32>) {
    let len = v.len();
    assert!(len > 1);
    v[len - 2] += 1;
    v.pop();
}

The asm for 1.38 is

example::foo:
        push    rax
        mov     rax, qword ptr [rdi + 16]
        cmp     rax, 2
        jb      .LBB7_4
        mov     rcx, qword ptr [rdi]
        add     dword ptr [rcx + 4*rax - 8], 1
; 1.28 does not generate these 3 instructions:
        mov     rax, qword ptr [rdi + 16]
        test    rax, rax
        je      .LBB7_3
; end of unnecessary code
        add     rax, -1
        mov     qword ptr [rdi + 16], rax
.LBB7_3:
        pop     rax
        ret
.LBB7_4:
        call    std::panicking::begin_panic
        ud2

https://godbolt.org/z/SLPlgo

@tspiteri
Copy link
Contributor Author

Hmm, this actually regressed in 1.29.2, so it's probably due to #54639.

@jonas-schievink jonas-schievink added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 12, 2019
@bluss
Copy link
Member

bluss commented Oct 12, 2019

Yes, -Z mutable-noalias makes the difference here. Will be fixed by #54878 again the same way :)

@nikic
Copy link
Contributor

nikic commented Apr 2, 2021

Fixed in nightly now that mutable-noalias is enabled again.

@nikic nikic closed this as completed Apr 2, 2021
# for free to join this conversation on GitHub. Already have an account? # 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. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants