-
Notifications
You must be signed in to change notification settings - Fork 52
Add GSoC 2024 3-way comparison page #56
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
Conversation
@Poseydon42 any update? |
Sorry, completely forgot about this. I've added links to individual PRs in middle- and backend, that should cover the most interesting parts of the project. |
@nikic Will you please take a look content-wise? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! We could get a slightly nicer example if we wait until llvm/llvm-project#107314 has landed.
```C | ||
unsigned char ucmp_32_8(unsigned int a, unsigned int b) { | ||
return (a < b ? -1 : a > b); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to say that we would get an even more compelling example by using the spaceship operator, because it generates pretty terrible asm on 18.1 (8 instructions instead of 3): https://c.godbolt.org/z/WGP7vba14
But we need llvm/llvm-project#107314 to land before we actually get the good new codegen for this pattern :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The patch has landed, so now this example goes from
spaceship(unsigned int, unsigned int):
xor ecx, ecx
cmp edi, esi
mov eax, 0
sbb eax, eax
or al, 1
cmp edi, esi
movzx eax, al
cmove eax, ecx
ret
to
spaceship(unsigned int, unsigned int):
cmp edi, esi
seta al
sbb al, 0
ret
which is quite nice!
@@ -24,11 +24,11 @@ In the middle-end the following passes received some support for these intrinsic | |||
|
|||
I have also added folds of idiomatic ways that a 3-way comparison can be expressed to a call to the corresponding intrinsic. | |||
|
|||
In the backend there are two different ways of expanding the intrinsics: [as a nested select](https://github.com/llvm/llvm-project/pull/91871) (i.e. `(x < y) ? -1 : (x > y ? 1 : 0)`) or [as a subtraction of zero-extended comparisons](https://github.com/llvm/llvm-project/pull/98774) (`zext(x > y) - zext(x < y)`). The first option is the default one, but targets can choose to use the second one through a TLI hook. | |||
In the backend there are two different ways of expanding the intrinsics: [as a nested select](https://github.com/llvm/llvm-project/pull/91871) (i.e. `(x < y) ? -1 : (x > y ? 1 : 0)`) or [as a subtraction of zero-extended comparisons](https://github.com/llvm/llvm-project/pull/98774) (`zext(x > y) - zext(x < y)`). The second option is the default one, but targets can choose to use the second one through a TLI hook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the backend there are two different ways of expanding the intrinsics: [as a nested select](https://github.com/llvm/llvm-project/pull/91871) (i.e. `(x < y) ? -1 : (x > y ? 1 : 0)`) or [as a subtraction of zero-extended comparisons](https://github.com/llvm/llvm-project/pull/98774) (`zext(x > y) - zext(x < y)`). The second option is the default one, but targets can choose to use the second one through a TLI hook. | |
In the backend there are two different ways of expanding the intrinsics: [as a nested select](https://github.com/llvm/llvm-project/pull/91871) (i.e. `(x < y) ? -1 : (x > y ? 1 : 0)`) or [as a subtraction of zero-extended comparisons](https://github.com/llvm/llvm-project/pull/98774) (`zext(x > y) - zext(x < y)`). The second option is the default one, but targets can choose to use the first one through a TLI hook. |
@Poseydon42 Will you please update the post with all recent suggestions so it will be possible to schedule it on next Monday? |
Sorry for the big delay. I've updated the example, it should be good to go now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scheduled for next Monday (October 7)
A blog post briefly outlining the work I had done over the course of GSoC 2024.
cc @asl