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

Comparison of add of extended booleans not folded to xor #64859

Closed
arsenm opened this issue Aug 21, 2023 · 11 comments · Fixed by #67895
Closed

Comparison of add of extended booleans not folded to xor #64859

arsenm opened this issue Aug 21, 2023 · 11 comments · Fixed by #67895

Comments

@arsenm
Copy link
Contributor

arsenm commented Aug 21, 2023

I was looking at some code that tried to compute a sort of enum scheme by adding booleans, and comparing to constant values which failed to form nice boolean code. This is really just an xor.

https://alive2.llvm.org/ce/z/RSnV-d

define i1 @src(i1 %arg, i1 %arg1) {
bb:
  %i = zext i1 %arg to i32
  %i2 = zext i1 %arg1 to i32
  %i3 = add nuw nsw i32 %i2, %i
  %i4 = icmp eq i32 %i3, 1
  ret i1 %i4
}

define i1 @tgt(i1 %arg, i1 %arg1) {
bb:
  %xor = xor i1 %arg, %arg1
  ret i1 %xor
}

searlmc1 pushed a commit to ROCm/llvm-project that referenced this issue Aug 25, 2023
There are apparently some missing optimizations surrounding
comparisons to the previous pseudo-enum. The compare of the
conditional add of boolean compared to the constant 1 did not fold
out.

We would need to implement an optimization such as
  icmp eq (add (zext i1 x), (zext i1 y)), 1 => xor x, y

which I filed here: llvm#64859

Just do this manually since it's more legible anyway. Saves 5
instructions for the f32 case.

Change-Id: Iee7befb093561cf66b72a9df6b37d0cacb2154ee
@elhewaty
Copy link
Member

elhewaty commented Sep 4, 2023

Can I fix this? But I will need some help, as I am not familiar with LLVM system yet

@elhewaty
Copy link
Member

elhewaty commented Sep 4, 2023

%i4 = icmp eq i32 %i3, 1
what if the 1 were 0?

@dc03
Copy link
Member

dc03 commented Sep 4, 2023

%i4 = icmp eq i32 %i3, 1 what if the 1 were 0?

That would be equivalent to not(or(x, y)): https://alive2.llvm.org/ce/z/8z7Q4G

@elhewaty
Copy link
Member

elhewaty commented Sep 4, 2023

so the xor case holds for eq, 1, and zext only

@dc03
Copy link
Member

dc03 commented Sep 4, 2023

so the xor case holds for eq and 1 only

Yes, if you sketch out the truth table for this function you will see it is equivalent to the table for an xor, and with 0 it is equivalent to the table for a nor.

@elhewaty
Copy link
Member

elhewaty commented Sep 4, 2023

So we need to create a new function foldICmpAddExtI1 and handle all our cases? or do we have something like this already?

@dc03
Copy link
Member

dc03 commented Sep 4, 2023

@elhewaty
Copy link
Member

elhewaty commented Sep 6, 2023

@arsenm Here is a candidate patch : https://reviews.llvm.org/D159464

@elhewaty
Copy link
Member

Can you please review the patch again.

@elhewaty
Copy link
Member

elhewaty commented Sep 20, 2023

@arsenm
ping

@elhewaty
Copy link
Member

@arsenm @nikic @goldsteinn Here's a candidate pull: #67895

dtcxzyw pushed a commit that referenced this issue Oct 6, 2023
- Add test coverage for sext/zext boolean additions
- [InstCombine] Fold comparison of adding two z/sext booleans

Fixes #64859.
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.

3 participants