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

[InstCombine] a ? (a < 0) : 1 --> (a <= 0) #68227

Open
k-arrows opened this issue Oct 4, 2023 · 2 comments · May be fixed by #69961
Open

[InstCombine] a ? (a < 0) : 1 --> (a <= 0) #68227

k-arrows opened this issue Oct 4, 2023 · 2 comments · May be fixed by #69961

Comments

@k-arrows
Copy link

k-arrows commented Oct 4, 2023

define i32 @src(i32 %0) {
%1:
  %2 = icmp eq i32 %0, 0
  %3 = lshr i32 %0, 31
  %4 = select i1 %2, i32 1, i32 %3
  ret i32 %4
}
=>
define i32 @tgt(i32 %0) {
%1:
  %2 = icmp slt i32 %0, 1
  %3 = zext i1 %2 to i32
  ret i32 %3
}
Transformation seems to be correct!

https://godbolt.org/z/evaE94d7o
https://alive2.llvm.org/ce/z/6ra3nj

@XChy
Copy link
Member

XChy commented Oct 4, 2023

Similar problems associated with the fold zext(a < 0) => lshr a, BW-1 have happened many times. For bit-level operations, the latter would be better. Conversely, for logical operations, the former would be better. I don't know which one is better and how to generalize it. But to solve similar problems with current canonicalization, we must consider a << (BW-1) when we fold operation with other (hidden) icmp.
What can we do if we don't want reduntant similar work? My 2c here is, if when we use pattern match, we can match a whole equivalent class like a < 0, a << BW - 1, then we can solve similar problems completely. That's a theoretical solution from my individual view and it's a systematic work (may change architecture) in need of other's approval and efforts.

@elhewaty
Copy link
Member

@dtcxzyw I will work on this, too.

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

Successfully merging a pull request may close this issue.

5 participants