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 for flooring division of signed by power of two #57741

Closed
tspiteri opened this issue Sep 14, 2022 · 3 comments
Closed

Missed optimization for flooring division of signed by power of two #57741

tspiteri opened this issue Sep 14, 2022 · 3 comments

Comments

@tspiteri
Copy link

If a signed integer is divided by a power of two it is not a simple shift like for unsigned because division rounds to zero while the shift would round down. If flooring division is implemented, then flooring division by a power of two is equivalent to an arithmetic shift.

Here, floor_div4_a and floor_div4_b are equivalent, but the code generated for _a is not as good as _c.

inline int floor_div(int n, int d)
{
    int q = n / d;
    int r = n % d;
    if ((r < 0 && d > 0) || (r > 0 && d < 0))
        q -= 1;
    return q;
}

int floor_div4_a(int i)
{
    return floor_div(i, 4);
}

int floor_div4_b(int i)
{
    return i >> 2;
}

[Compiler explorer]

(This is similar to rustc issue rust-lang/rust#71096.)

@scottmcm
Copy link

scottmcm commented Oct 28, 2022

Alive2 proof: https://alive2.llvm.org/ce/z/vpidT9

EDIT: Updated to 16-bit because the 32-bit seems to time out often, https://alive2.llvm.org/ce/z/vgj3BD

@rotateright rotateright self-assigned this Dec 17, 2022
@rotateright
Copy link
Contributor

Generalized pattern and requirements:
https://alive2.llvm.org/ce/z/MJzlhl

I was hoping there was some sub-pattern we could squash within there, but I don't see it, so I'll try to add the large match.

@rotateright
Copy link
Contributor

I typo'd the bug number in the commit message, but this should be fixed with:
86b4a23

rotateright referenced this issue Jan 13, 2023
It's a bigger match than usual, but I have not found any
sub-patterns that reduce:
(X / DivC) + sext ((X & (SMin | (DivC - 1)) >u SMin) --> X >>s log2(DivC)

https://alive2.llvm.org/ce/z/MJzlhl

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

No branches or pull requests

5 participants