Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement more cases for getMaxBits #2879
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
Implement more cases for getMaxBits #2879
Changes from all commits
ee08839
374c9b6
3a0f72e
091e15e
18a2aa2
168fed4
d04ae7d
c08376c
2dc792c
56146d3
3f68768
36f73e5
c77d5d4
d60b53e
0bba5b6
f43c8db
a6415e1
d979620
6685e6d
7726f18
a39091a
908eb96
df767f4
73ad57d
6b8d232
f387148
38ee4e7
74f10d5
fa13337
2e9ab76
3ea4530
252ce01
fd4e8cf
60b7b55
d93f65d
c9fe2cb
14ae90c
90f39d4
5a99214
64aded9
4b2061a
216b6fa
c631b94
d0a4938
543d239
5673101
ae827a8
8567487
957e945
d5d6d2b
709b8ed
569362f
9545f9e
9085d8e
422e46f
551a675
d7335a8
3a27ca3
e0739c3
74e568c
ae75346
6883ce7
0cc2e85
acd6518
df40a08
da177ae
d84f9ba
5c1d1c4
f319270
55d9824
f924008
790a05c
30b1842
341a3ef
b91ee8c
719d8c3
146c966
a74f10e
b4d614d
9b2fe4d
162c94d
8404d91
4be0956
ec62420
5fc687a
0be9076
33c9eab
277d159
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is there reason to believe that checking the right side first will save more work than checking the left side first, or is it an arbitrary choice?
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.
Yes, right side could be cheaper due to canonization which always force constant on the right. Also
x ^ -1
is quite often case.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.
It looks like it would be a lot simpler to just overload the CeilLog2 for uint32_t and uint64_t without using any templates. That probably applies to all these functions, but since they're already like that, this can be left for a follow up.
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.
It was my first attempt, but templates in C++ still a mystery to me sometimes=) I didn’t manage to make friends with
bits.h
header thisbits.cpp
part:Also it quite hard to restrict
T
to unsigned integer types only.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.
Yeah, no problem, let's leave cleaning that up to a follow-up PR.