bpo-31031: Unify duplicate bits_in_digit and bit_length#2866
Conversation
Sorry, something went wrong.
auvipy
left a comment
auvipy
left a comment
There was a problem hiding this comment.
check the build errors plz
Sorry, something went wrong.
niklasf
force-pushed
the
bpo-31031-consolidate-bit-length
branch
from
ccdbe59 to
900f7a8
Compare
June 2, 2019 14:15
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
niklasf
force-pushed
the
bpo-31031-consolidate-bit-length
branch
2 times, most recently
from
f225c8e to
dcaa962
Compare
June 5, 2019 10:07
|
Thanks for reviewing. I have made the requested changes; please review again. |
Sorry, something went wrong.
|
Thanks for making the requested changes! @vstinner: please review the changes made to this pull request. |
Sorry, something went wrong.
niklasf
force-pushed
the
bpo-31031-consolidate-bit-length
branch
from
dcaa962 to
7b4198c
Compare
June 12, 2019 14:42
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM.
Sorry, something went wrong.
|
Thanks @niklasf, sorry for slow reviews and for being so pedantic :-) But it was worth it, I like the final change ;-) I looked again at rejected https://bugs.python.org/issue29782 and #594 I was going to ask further change on this PR to prepare the code in case if someone wants to experiment again more efficient functions... But nah, the code is ok and can be updated later. |
Sorry, something went wrong.
Add _Py_bit_length() to unify duplicate bits_in_digit() and bit_length().
In pythonGH-2866, _Py_Bit_Length() was added to pymath.h for lack of a better location. pythonGH-20518 added a more appropriate header file for bit utilities. It also shows how to properly use intrinsics. This allows reconsidering bpo-29782. * Move the function to the new header. * Changed return type to match __builtin_clzl and reviewed usage. * Use intrinsics where available. * Pick a (mostly theoretical) fallback implementation suitable for inlining.
In GH-2866, _Py_Bit_Length() was added to pymath.h for lack of a better location. GH-20518 added a more appropriate header file for bit utilities. It also shows how to properly use intrinsics. This allows reconsidering bpo-29782. * Move the function to the new header. * Changed return type to match __builtin_clzl() and reviewed usage. * Use intrinsics where available. * Pick a fallback implementation suitable for inlining.
Reviewers
+1 more reviewer
7 participants
https://bugs.python.org/issue31031