Add functions for reversing the bit pattern in an integer by Amanieu · Pull Request #48573 · rust-lang/rust

Amanieu · 2018-02-27T04:38:41Z

I'm reviving PR #32798 now that the LLVM issues have been resolved.

This adds the bitreverse intrinsic and adds a reverse_bits function to all integer types.

rust-highfive · 2018-02-27T04:38:53Z

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

pitdicker · 2018-02-27T07:26:28Z

Should this also support i128/u128/isize/usize?

alexcrichton · 2018-02-27T09:22:20Z

Should this be reverse_bits?

Amanieu · 2018-02-27T09:24:45Z

Should this also support i128/u128/isize/usize?

Yes these should all be supported. (I fixed the missing intrinsic for i128)

Mark-Simulacrum · 2018-02-27T14:52:45Z

@GuillaumeGomez @QuietMisdreavus I feel like I've recently heard something about us being able to generate dedicated examples with the right types for primitives...

Yep we did here.

I just copied the docs for swap_bytes and made minor adjustments.

Well, in this case it is very complicated to be able to write a fully generic example.

hanna-kruppe · 2018-02-27T23:31:43Z

Could you add a test for u/i128? It would not surprise me at all if some LLVM backends have trouble with this intrinsic on 128 bit integers, so it would be good to have at least some test to demonstrate that it works at all. (And if it works at all, it's probably reasonably robust given how LLVM is structured.)

Amanieu · 2018-02-28T21:19:35Z

All comments have been addressed. I will create a tracking issue when this is approved.

emilyalbini · 2018-03-05T15:11:29Z

Ping from the release team @sfackler! This PR needs your review.

sfackler · 2018-03-05T17:08:33Z

There were some questions about use cases in the original PR - where would these functions be used?

Amanieu · 2018-03-05T17:20:25Z

I need these because I am emulating an architecture that has a bit reverse instruction. I would like to be able to use native bit-reverse instructions if they are available.

AFAIK LLVM won't optimize normal code into a native bit-reverse instruction, you need to use the intrinsic for that.

sfackler · 2018-03-06T00:44:20Z

LGTM on the API side, but I'm not super familiar with trans - @alexcrichton does that side of things look okay to you?

alexcrichton · 2018-03-06T03:06:27Z

Looks good! Want to open a tracking issue and this can be r+'d?

Amanieu · 2018-03-06T03:32:26Z

Tracking issue created: #48763

sfackler · 2018-03-06T04:13:17Z

@bors r+

bors · 2018-03-06T04:13:17Z

📌 Commit 88aec91 has been approved by sfackler

Add functions for reversing the bit pattern in an integer I'm reviving PR rust-lang#32798 now that the LLVM issues have been resolved. > This adds the bitreverse intrinsic and adds a reverse_bits function to all integer types.

Rollup of 14 pull requests - Successful merges: #48403, #48432, #48546, #48573, #48590, #48657, #48727, #48732, #48753, #48754, #48761, #48474, #48507, #47463 - Failed merges:

gnzlbg · 2018-03-14T08:43:16Z

@Amanieu aren't these exposed by stdsimd already? If not, could you fill an issue there mentioning which instructions you were missing for which architecture?

gnzlbg · 2018-03-14T08:47:46Z

I've opened an issue on stdsimd to start migrating the intrinsics that were already using llvm's bitreverse intrinsic to use the rustc intrinsic.

Amanieu · 2018-03-14T08:51:00Z

They are only exposed as vendor-specific intrinsics in stdsimd. I wanted to add a generic solution that worked on all architectures.

gnzlbg · 2018-03-14T10:21:05Z

@Amanieu makes sense, just keep in mind that the performance of LLVM's generic bitreverse solution can vary greatly depending on the types involved, your target architecture, and what the rest of your code does around it: https://bugs.llvm.org/show_bug.cgi?id=31810

If your target has a bit reverse instruction (only ARMv8 as far as I know) then everything should be fine.

rust-highfive assigned sfackler Feb 27, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 27, 2018

Amanieu force-pushed the bitreverse2 branch 2 times, most recently from 9467b30 to 616ea71Compare February 27, 2018 06:51

Amanieu force-pushed the bitreverse2 branch from 616ea71 to 6fcf4efCompare February 27, 2018 08:51

alexcrichton reviewed Feb 27, 2018
View reviewed changes

Amanieu force-pushed the bitreverse2 branch from 6fcf4ef to 343bbfcCompare February 27, 2018 09:23

Amanieu force-pushed the bitreverse2 branch from 343bbfc to e7fe645Compare February 27, 2018 11:27

Mark-Simulacrum reviewed Feb 27, 2018
View reviewed changes

hanna-kruppe reviewed Feb 27, 2018
View reviewed changes

Amanieu force-pushed the bitreverse2 branch from e7fe645 to 7c9b16cCompare February 28, 2018 13:57

Add bitreverse intrinsic
02e021b

Amanieu force-pushed the bitreverse2 branch from 7c9b16c to 2126d94Compare February 28, 2018 18:24

Amanieu mentioned this pull request Mar 6, 2018
Tracking issue for reversing the bit pattern in an integer #48763
Closed

Amanieu added 2 commits March 6, 2018 03:31

Add reverse_bits to integer types
24fb4b7

Add i128 tests for intrinsics
88aec91

Amanieu force-pushed the bitreverse2 branch from 2126d94 to 88aec91Compare March 6, 2018 03:32

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2018

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 6, 2018

kennytm mentioned this pull request Mar 6, 2018
Rollup of 14 pull requests #48768
Merged

bors added a commit that referenced this pull request Mar 6, 2018
Auto merge of#48768- kennytm:rollup, r=kennytm
2789b06
Rollup of 14 pull requests - Successful merges: #48403, #48432, #48546, #48573, #48590, #48657, #48727, #48732, #48753, #48754, #48761, #48474, #48507, #47463 - Failed merges:

alexcrichton merged commit 88aec91 into rust-lang:masterMar 6, 2018

gnzlbg mentioned this pull request Mar 14, 2018
Update bitreverse intrinsics rust-lang/stdarch#372
Closed

Uh oh!

Uh oh!

Conversation

Amanieu commented Feb 27, 2018

Uh oh!

rust-highfive commented Feb 27, 2018

Uh oh!

pitdicker commented Feb 27, 2018

Uh oh!

alexcrichtonFeb 27, 2018

Choose a reason for hiding this comment

Uh oh!

AmanieuFeb 27, 2018

Choose a reason for hiding this comment

Uh oh!

Amanieu commented Feb 27, 2018

Uh oh!

Mark-SimulacrumFeb 27, 2018

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomezFeb 27, 2018

Choose a reason for hiding this comment

Uh oh!

AmanieuFeb 27, 2018

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomezFeb 28, 2018

Choose a reason for hiding this comment

Uh oh!

hanna-kruppeFeb 27, 2018

Choose a reason for hiding this comment

Uh oh!

Amanieu commented Feb 28, 2018

Uh oh!

emilyalbini commented Mar 5, 2018

Uh oh!

sfackler commented Mar 5, 2018

Uh oh!

Amanieu commented Mar 5, 2018

Uh oh!

sfackler commented Mar 6, 2018

Uh oh!

alexcrichton commented Mar 6, 2018

Uh oh!

Amanieu commented Mar 6, 2018

Uh oh!

sfackler commented Mar 6, 2018

Uh oh!

bors commented Mar 6, 2018

Uh oh!

gnzlbg commented Mar 14, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnzlbg commented Mar 14, 2018

Uh oh!

Amanieu commented Mar 14, 2018

Uh oh!

gnzlbg commented Mar 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

gnzlbg commented Mar 14, 2018•
edited
Loading