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

Missed optimization with redundant overflow checks #40191

Closed
alex mannequin opened this issue Feb 24, 2019 · 3 comments
Closed

Missed optimization with redundant overflow checks #40191

alex mannequin opened this issue Feb 24, 2019 · 3 comments
Labels

Comments

@alex
Copy link
Mannequin

alex mannequin commented Feb 24, 2019

Bugzilla Link 40845
Version trunk
OS All
CC @alex,@nelhage

Extended Description

Godbolt link: https://godbolt.org/z/h2lkGE

The subtraction can be known not to overflow because it's derived from an overflow checked addition. This was reduced/converted to C from this rust bug: rust-lang/rust#58692

One commenter suggested they thought the issue was in how GVN handled overflow checked operations.

Code:

#include <stdlib.h>

extern void panic1();
extern void panic2();
extern void panic3();

size_t f(size_t a, size_t b) {
    size_t x;
    if (!__builtin_add_overflow(a, b, &x)) {
        panic1();
        __builtin_unreachable();
    }

    size_t r;
    if (!__builtin_sub_overflow(x, a, &r)) {
        panic2();
        __builtin_unreachable();
    }
    
    if (r < 0) {
        panic3();
        __builtin_unreachable();
    }

    return x;
}

Assembly:

f(unsigned long, unsigned long):                                 # @f(unsigned long, unsigned long)
        push    rax
        mov     rax, rsi
        add     rax, rdi
        jae     .LBB0_3
        cmp     rax, rdi
        jae     .LBB0_4
        pop     rcx
        ret
.LBB0_3:
        call    panic1()
.LBB0_4:
        call    panic2()
@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@alex
Copy link
Member

alex commented Sep 10, 2022

It looks like this is fixed on llvm/clang trunk, per godbolt: https://godbolt.org/z/8shWdvbeq

@alex
Copy link
Member

alex commented Dec 30, 2023

Can someone please close this, it's been fixed since llvm/clang 16.

@dtcxzyw dtcxzyw closed this as completed Dec 30, 2023
@alex
Copy link
Member

alex commented Dec 30, 2023

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants