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 bounds check optimization #46516

Closed
davidbolvansky opened this issue Aug 14, 2020 · 4 comments
Closed

Missed bounds check optimization #46516

davidbolvansky opened this issue Aug 14, 2020 · 4 comments
Labels
bugzilla Issues migrated from bugzilla llvm:optimizations

Comments

@davidbolvansky
Copy link
Collaborator

davidbolvansky commented Aug 14, 2020

Bugzilla Link 47172
Version trunk
OS Linux
CC @efriedma-quic,@fhahn,@jrmuizel,@nikic

Extended Description

From rust-lang github (rust-lang/rust#75525):

#include <stddef.h>
#include <stdint.h>


uint8_t f1(size_t idx) {
    if (idx < 8) {
        // it's ok that this has a bounds check
        return (idx - 1) < 10;
    } else {
        return 0;
    }
}

Clang:

f1:                                     # @&#8203;f1
        cmp     rdi, 8
        setb    cl
        add     rdi, -1
        cmp     rdi, 10
        setb    al
        and     al, cl
        ret

GCC:

f1:
        sub     rdi, 1
        cmp     rdi, 6
        setbe   al
        ret

Modified test case:

uint8_t f2(size_t idx) {
    if (idx <= 9) {
       return (idx - 1) > 10;
    } else {
        return 0;
    }
}

Clang:

f2:                                     # @&#8203;f2
        cmp     rdi, 10
        setb    cl
        add     rdi, -1
        cmp     rdi, 10
        seta    al
        and     al, cl
        ret

GCC:

f2:
        test    rdi, rdi
        sete    al
        ret

https://godbolt.org/z/vahzbo

@fhahn
Copy link
Contributor

fhahn commented Aug 14, 2020

Variants of this should be able to be handled using the approach suggested here https://reviews.llvm.org/D84547

Although in the C source, idx - 1 can wrap. I'm not entirely sure about the rust case, but I assume there might be some wrapping flags?

@efriedma-quic
Copy link
Collaborator

gcc is optimizing assuming the subtraction can wrap.

rustc generally doesn't generate nsw/nuw flags in the frontend; integer overflow is defined to either wrap or trap.

@davidbolvansky
Copy link
Collaborator Author

davidbolvansky commented Aug 14, 2020

One more test case for CE pass:

uint8_t f3(size_t idx) {
    if (idx <= 9) {
       return (idx + 1) > 16;
    } else {
        return 0;
    }
}

Clang:

f3:                                    
        cmp     rdi, 10
        setb    cl
        add     rdi, 1
        cmp     rdi, 16
        seta    al
        and     al, cl
        ret

GCC:

f3:
        xor     eax, eax
        ret

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@fhahn
Copy link
Contributor

fhahn commented Feb 4, 2022

Looks like Clang trunk optimizes this now as expected: https://godbolt.org/z/a55674xhe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla llvm:optimizations
Projects
None yet
Development

No branches or pull requests

4 participants