Getting rid of unbounded shift #241
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martun
changed the title
martun
force-pushed
the
get_rid_of_unbounded_shift
branch
from
fec1077 to
b90241c
Compare
Gcc Test Results 162 files - 1 162 suites - 1 16m 25s ⏱️ +8s For more details on these failures, see this check. Results for commit b90241c. ± Comparison against base commit 1fed3a7. This pull request removes 5 and adds 8 tests. Note that renamed tests count towards both. |
martun
force-pushed
the
get_rid_of_unbounded_shift
branch
2 times, most recently
from
d09d2bf to
79d8a26
Compare
vo-nil
reviewed
Jan 8, 2025
| has_encode<T>::value && has_decode<T>::value; | ||
| typedef T type; | ||
| }; | ||
| //template<typename T> |
There was a problem hiding this comment.
Is this intentional? Should we remove the total block?
vo-nil
reviewed
Jan 8, 2025
| k = k + data[i] * (shft % L); | ||
| shft *= 0x10000000000000000_big_uint512; | ||
| k = k + data[i] * shft; | ||
| shft = wrapping_mul(shft, 0x10000000000000000_big_uint512) % L; |
There was a problem hiding this comment.
I think we can keep original code, just skip *= on the last step.
Why don't we use shift <<= 64; here in first place?
There was a problem hiding this comment.
We want modulo L here, not modulo 0x10000000000000000_big_uint512, no?
martun
force-pushed
the
get_rid_of_unbounded_shift
branch
from
fc050e2 to
b25cd40
Compare
… access bugs. We should not shift over the size of values and hide it with this class.
martun
force-pushed
the
get_rid_of_unbounded_shift
branch
from
b25cd40 to
321188a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Getting rid of unbounded shift. It looked nice, but introduced memory access bugs. We should not shift over the size of values and hide it with this class.