Skip to content

[LLVM 7] use funnel shift intrinsic to implement rotates #52457

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
Closed
hanna-kruppe opened this issue Jul 17, 2018 · 1 comment
Closed

[LLVM 7] use funnel shift intrinsic to implement rotates #52457

hanna-kruppe opened this issue Jul 17, 2018 · 1 comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-llvm Working group: LLVM backend code generation

Comments

@hanna-kruppe
Copy link
Contributor

https://reviews.llvm.org/rL337221 introduced new intrinsics in LLVM IR that make it easier to guarantee that integer rotates will make it to the backend intact and can be lowered to a hardware rotate instruction (instead of possibly being split up into shifts and bitwise ors). Since we already expose rotates as methods on integer types, we could just change the implementation of those methods to lower to these IR intrinsics. As soon as we're on an LLVM version that supports these intrinsics, that is, but I wanted to file this issue before I forget about them.

cc @eddyb @nox
@kennytm kennytm added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-llvm Working group: LLVM backend code generation labels Jul 17, 2018
@nagisa
Copy link
Member

nagisa commented Jul 17, 2018

This is definitely something we want to do. We already have a few issues around that complain about .rotate_xxx not optimising down to a rotate instruction. See e.g. https://bugs.llvm.org/show_bug.cgi?id=37461

@nikic nikic mentioned this issue Nov 3, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this issue Nov 8, 2018
@kennytm
Rollup merge of rust-lang#55650 - nikic:funnel-shift, r=nagisa
f558288 
Implement rotate using funnel shift on LLVM >= 7

Implement the rotate_left and rotate_right operations using
llvm.fshl and llvm.fshr if they are available (LLVM >= 7).

Originally I wanted to expose the funnel_shift_left and
funnel_shift_right intrinsics and implement rotate_left and
rotate_right on top of them. However, emulation of funnel
shifts requires emitting a conditional to check for zero shift
amount, which is not necessary for rotates. I was uncomfortable
doing that here, as I don't want to rely on LLVM to optimize
away that conditional (and for variable rotates, I'm not sure it
can). We should revisit that question when we raise our minimum
version requirement to LLVM 7 and don't need emulation code
anymore.

Fixes rust-lang#52457.
bors added a commit that referenced this issue Nov 10, 2018
@bors
Auto merge of #55650 - nikic:funnel-shift, r=nagisa
6e9b842 
Implement rotate using funnel shift on LLVM >= 7

Implement the rotate_left and rotate_right operations using
llvm.fshl and llvm.fshr if they are available (LLVM >= 7).

Originally I wanted to expose the funnel_shift_left and
funnel_shift_right intrinsics and implement rotate_left and
rotate_right on top of them. However, emulation of funnel
shifts requires emitting a conditional to check for zero shift
amount, which is not necessary for rotates. I was uncomfortable
doing that here, as I don't want to rely on LLVM to optimize
away that conditional (and for variable rotates, I'm not sure it
can). We should revisit that question when we raise our minimum
version requirement to LLVM 7 and don't need emulation code
anymore.

Fixes #52457.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-llvm Working group: LLVM backend code generation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kennytm @nagisa @hanna-kruppe