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

Optimize C++ spaceship comparison #49210

Closed
llvmbot opened this issue Apr 6, 2021 · 6 comments
Closed

Optimize C++ spaceship comparison #49210

llvmbot opened this issue Apr 6, 2021 · 6 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 6, 2021

Bugzilla Link 49866
Resolution FIXED
Resolved on Apr 12, 2021 13:40
Version trunk
OS Linux
Reporter LLVM Bugzilla Contributor
CC @davidbolvansky,@dwblaikie,@LebedevRI,@RKSimon,@rotateright

Extended Description

Similar to gcc's https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94589 , it would be nice to optimize (x<=>y)@0 to x@y for all comparisons @, and clang/llvm seems to manage for some but not all.

#include
int n(int x){return (x<=>0)<0;}

Debian clang version 13.0.0-++20210129063721+010b176cdefb-1~exp1
x86_64-pc-linux-gnu

movl	%edi, %eax
shrl	$24, %eax
testl	%edi, %edi
setne	%cl
testb	%al, %al
sets	%al
andb	%cl, %al
movzbl	%al, %eax
retq
@llvmbot
Copy link
Collaborator Author

llvmbot commented Apr 6, 2021

assigned to @rotateright

@LebedevRI
Copy link
Member

Can you please post relevant standalone snippets here, as links to godbolt?

@davidbolvansky
Copy link
Collaborator

#include

bool test1(int x, int y){return (x<=>y)<0;}

bool test2(int x){return (x<=>1)<0;}

bool test3(int x){return (x<=>0)>0;}

bool test4(int x){return (x<=>0)<0;}

define dso_local zeroext i1 @​_Z5test1ii(i32 %0, i32 %1) local_unnamed_addr #​0 {
%3 = icmp slt i32 %0, %1
ret i1 %3
}

define dso_local zeroext i1 @​_Z5test2i(i32 %0) local_unnamed_addr #​0 {
%2 = icmp slt i32 %0, 1
ret i1 %2
}

define dso_local zeroext i1 @​_Z5test3i(i32 %0) local_unnamed_addr #​0 {
%2 = icmp sgt i32 %0, 0
ret i1 %2
}

define dso_local zeroext i1 @​_Z5test4i(i32 %0) local_unnamed_addr #​0 {
%2 = lshr i32 %0, 24
%3 = trunc i32 %2 to i8
%4 = icmp ne i32 %0, 0
%5 = icmp slt i8 %3, 0
%6 = and i1 %4, %5
ret i1 %6
}

https://godbolt.org/z/n78v1dahz

@davidbolvansky
Copy link
Collaborator


define i1 @​src(i32 %0) {
%1:
%2 = icmp slt i32 %0, 0
%3 = select i1 %2, i8 255, i8 1
%4 = icmp eq i32 %0, 0
%5 = select i1 %4, i8 0, i8 %3
%6 = icmp slt i8 %5, 0
ret i1 %6
}
=>
define i1 @​tgt(i32 %0) {
%1:
%2 = icmp slt i32 %0, 0
ret i1 %2
}
Transformation seems to be correct!

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

We somehow miss this transformation for SLT,
https://godbolt.org/z/8KrnKj1sh
for SGT it works now:
https://godbolt.org/z/PaKbY48Pv

@rotateright
Copy link
Contributor

Looks like we're just missing a basic icmp fold for a truncated value:
https://alive2.llvm.org/ce/z/6vQvrP

I'll post a patch.

@rotateright
Copy link
Contributor

This should get the example in the description:
https://reviews.llvm.org/rG5354a213a0e3

If there are other examples, please reopen or file more bugs. Thanks!

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
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

4 participants