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

Failure to properly optimize successive > 0 identities to unsigned comparisons #46435

Open
GabrielRavier opened this issue Aug 10, 2020 · 3 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@GabrielRavier
Copy link
Contributor

GabrielRavier commented Aug 10, 2020

Bugzilla Link 47091
Version trunk
OS Linux
CC @davidbolvansky,@fhahn,@RKSimon,@rotateright

Extended Description

void bar(int, int);

void foo(int a, int b)
{
    if (a >= 0 && b >= 0 && a < 32 && b < 128)
        bar(a, b);
}

The if's condition can be optimized to (unsigned)a < 32 && (unsigned)b < 128. This transformation is done by GCC, but not by LLVM.

@rotateright
Copy link
Contributor

rotateright commented Aug 12, 2020

In IR, it looks something like this:

define i1 @foo(i32 %a, i32 %b) {
  %t0 = or i32 %b, %a
  %t1 = icmp sgt i32 %t0, -1
  %cmp3 = icmp slt i32 %a, 32
  %or.cond1 = and i1 %cmp3, %t1
  %cmp5 = icmp slt i32 %b, 128
  %r = and i1 %cmp5, %or.cond1
  ret i1 %r

We get the expected optimization if the source was:

void foo(int a, int b) {
    if (a >= 0 && a < 32 && b >=0 && b < 128)
        bar(a, b);
}

So this is an ordering or reassociation problem. In the original code, we see the 2 initial sign-bit checks and combine those. Then, we don't have the optimization power to unwind the and+or logic tree to see the bigger, better optimization.

@davidbolvansky
Copy link
Collaborator

Maybe these cases could be optimized with https://reviews.llvm.org/D84547

@rotateright
Copy link
Contributor

rotateright commented Aug 12, 2020

Maybe these cases could be optimized with https://reviews.llvm.org/D84547

That would be great - cc'ing @​fhahn.

To amend my earlier comment: if we are going to solve this generally, then it's not a reassociation problem (the source code could also have been written in the form that directly corresponds to the IR in comment 1).

If this is not something that ConstraintElimination could be made to handle, we could view this as missing pieces of and-of-icmps logic like:

define i1 @src(i32 %a, i32 %b) {
  %t0 = or i32 %b, %a
  %t1 = icmp sgt i32 %t0, -1
  %cmp3 = icmp ult i32 %a, 32 ; sign-bit test of %a is implied here
  %and1 = and i1 %cmp3, %t1
  ret i1 %and1
}

define i1 @tgt(i32 %a, i32 %b) {
  %t1 = icmp sgt i32 %b, -1
  %cmp3 = icmp ult i32 %a, 32
  %and1 = and i1 %cmp3, %t1
  ret i1 %and1
}

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

3 participants