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

Function calls in logic expressions sometimes cause optimization misses #93279

Open
BlaCoiso opened this issue Jan 24, 2022 · 3 comments
Open
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

@BlaCoiso
Copy link
Contributor

Calling functions, including const ones, on a logic expression (eg a || b) sometimes cause certain optimizations to be missed

I tried this code:

#[inline]
pub const fn a1(b: u8) -> bool {
    !b.is_ascii()
}

#[inline]
pub const fn a2(b: u8) -> bool {
    b.is_ascii_control()
}

pub const fn check1(b: u8) -> bool {
    let a1 = a1(b);
    let a2 = a2(b);
    a1 || a2
}

// this should generate the exact same code as check1 but it doesn't
pub const fn check2(b: u8) -> bool {
    a1(b) || a2(b)
}

Expected result: both functions (check1 and check2) generate the same assembly code

Actual result:

example::check1:
        add     dil, -32
        cmp     dil, 95
        setae   al
        ret

example::check2:
        test    dil, dil
        js      .LBB1_1
        cmp     dil, 32
        setb    cl
        cmp     dil, 127
        sete    al
        or      al, cl
        ret
.LBB1_1:
        mov     al, 1
        ret

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

Additionally, changing the comparison order affects optimization:

// this optimizes correctly and generates same code as check1 despite being equivalent
// const fns have no side effects and logical or is commutative
pub const fn check3(b: u8) -> bool {
    a2(b) || a1(b)
}

Meta

rustc --version --verbose:

rustc 1.60.0-nightly (84322efad 2022-01-23)
binary: rustc
commit-hash: 84322efad553c7a79c80189f2d1b9197c1aa005f
commit-date: 2022-01-23
host: x86_64-pc-windows-msvc
release: 1.60.0-nightly
LLVM version: 13.0.0
@BlaCoiso BlaCoiso added the C-bug Category: This is a bug. label Jan 24, 2022
@nikic nikic added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-slow Issue: Problems and improvements with respect to performance of generated code. labels Jan 24, 2022
@the8472
Copy link
Member

the8472 commented Jan 25, 2022

Replacing || with | seems to solve this. It looks similar to #88204

@BlaCoiso
Copy link
Contributor Author

BlaCoiso commented Jan 25, 2022

This could be related to short circuiting, where the compiler is trying to only evaluate the second expression when the first one is false, even when the second expression has no side effects and evaluating both expressions at the same time would generate more optimized code. That would explain why | solves this, as | doesn't do short circuiting and always evaluates both sides of the expression, making these optimizations possible.

Edit: Could be related to #83623

This code is generating the same assembly as check2:

pub const fn check3(b: u8) -> bool {
    if a1(b) { true } else { a2(b) }
}

@workingjubilee
Copy link
Contributor

workingjubilee commented Jul 30, 2022

Correct.

It's actually somewhat well-known, unfortunately, that the optimization you want is often made invalid by the language semantics, including here:

I do not see an obvious way to resolve this in the general case, as the optimizer is not allowed to simply resolve the second function always. I assume we could try to use our knowledge of the by-value, side-effect-free nature of the operations in this case.

@Nilstrieb Nilstrieb added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2023
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. 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

5 participants