-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add bitreverse
function
#34791
add bitreverse
function
#34791
Conversation
base/int.jl
Outdated
|
||
function bitreverse(x::Union{Int8,UInt8}) | ||
z = x % UInt8 | ||
z = ((z >>> 1) & 0x55) | ((z << 1) & 0xaa) |
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.
why the mix of arithmetic and logic shifts?
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.
There is no <<<
?
base/int.jl
Outdated
z = x % UInt16 | ||
z = ((z >>> 1) & 0x5555) | ((z << 1) & 0xaaaa) | ||
z = ((z >>> 2) & 0x3333) | ((z << 2) & 0xcccc) | ||
z = ((z >>> 4) & 0x0f0f) | ((z << 4) & 0xf0f0) | ||
z = ((z >>> 8) & 0x00ff) | ((z << 8) & 0xff00) |
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.
Sorry to jump in, but why not use bswap
for some of these operations? It's shorter, easier to understand, less reliant on magic numbers, and presumably compiles to efficient LLVM.
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 fact, one could use a single method for all ints, like this:
function bitreverse1(x::Base.BitInteger)
z = unsigned(x)
mask1 = 0x55555555555555555555555555555555 % typeof(z)
mask2 = 0x33333333333333333333333333333333 % typeof(z)
mask4 = 0x0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f % typeof(z)
z = ((z & mask1) << 1) | ((z >> 1) & mask1)
z = ((z & mask2) << 2) | ((z >> 2) & mask2)
z = ((z & mask4) << 4) | ((z >> 4) & mask4)
return bswap(z) % typeof(x)
end
On my machine, this generates more efficient code, too, but I don't know if it uses too much "compile time magic".
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 fact, LLVM already has a bitreverse
intrinsic: https://llvm.org/docs/LangRef.html#llvm-bitreverse-intrinsics. But it will probably compile to the same thing.
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.
But with the intrinsic, it's (more) not our problem to make sure it's fast on all the architectures.
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'll use this implementation, since adding intrinsics is harder and I'd rather avoid llvmcall.
cf621b1
to
627b7d2
Compare
627b7d2
to
246f0d3
Compare
This can be useful for parsing binary formats, and generally helps round out the set of bit-level operations.