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.

Sign up for GitHub

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

Jump to bottom

[InstCombine] Failure to recognise hidden xor #57666

Closed
RKSimon opened this issue Sep 10, 2022 · 5 comments
Closed

[InstCombine] Failure to recognise hidden xor #57666

RKSimon opened this issue Sep 10, 2022 · 5 comments
Labels
llvm:instcombine missed-optimization

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented Sep 10, 2022 •
edited by VoltrexKeyva
Loading

https://alive2.llvm.org/ce/z/EFnf-a

define i1 @src(i1 %0, i1 %1) denormal-fp-math=ieee,ieee {
%2:
  %t5 = select i1 %0, i32 4, i32 1
  %t6 = select i1 %1, i32 4, i32 1
  %t7 = and i32 %t6, %t5
  %t8 = icmp eq i32 %t7, 0
  ret i1 %t8
}
=>
define i1 @tgt(i1 %0, i1 %1) denormal-fp-math=ieee,ieee {
%2:
  %foo = xor i1 %0, %1
  ret i1 %foo
}
Transformation seems to be correct!

Reported here: https://twitter.com/johnregehr/status/1568286233968656385
@marcauberer
Copy link
Member

@RKSimon I would like to work on this ...

@RKSimon
Copy link
Collaborator Author

RKSimon commented Sep 15, 2022

Go for it!

@rotateright
Copy link
Contributor

https://reviews.llvm.org/D133919

@rotateright
Copy link
Contributor

No recent activity on D133919, so I've tried to get this example with:
https://reviews.llvm.org/D142847
(enhancements to follow)

rotateright added a commit that referenced this issue Jan 30, 2023
@rotateright
[InstCombine] reduce icmp_eq0-of-and-of-select-of-constants
9885505 
This is the most basic patch to handle fixing issue #57666.

D133919 proposes to handle much more than this in a single patch,
but I've used 10 regression tests just to make sure this part is
doing what I expected and nothing more, and it already shows even
more potential TODO items.

The more general proofs from D133919 are correct, but I want to
enable this in smaller steps to reduce risk:
https://alive2.llvm.org/ce/z/RrVEyX

Differential Revision: https://reviews.llvm.org/D142847
@rotateright
Copy link
Contributor

We get the 2 basic cases of select-of-constants now, and there are TODO comments for enhancements.

There's potentially some overlap between these patterns and signum (spaceship) as seen in #60012.
We could convert a pair of selects glued together with bitwise-logic to independent selects like this:
https://alive2.llvm.org/ce/z/AKxYuw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
llvm:instcombine missed-optimization
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@RKSimon @rotateright @marcauberer