-
Notifications
You must be signed in to change notification settings - Fork 290
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
More carry calculation fixing #835
More carry calculation fixing #835
Conversation
A small observation: This change leads to five mult-operations for the product, |
@tomwiel Right – and that's because carry is calculated separately, which is leftover from a previous version that bailed out after a straightforward (carry-ignoring) multiplication, in case the result was not equal. Good catch, let me try to improve on this front... |
I think I've finally gotten my head around this ...
What I hadn't appreciated until now is the addition c. above can potentially overflow by 2. |
…, and another for `c*d`)
This should be fixed now. |
Yes, just realized that much later than you. 😅 And found out that that uint64_t extra_carry = (((aHiShr * bLo) & 0xFFFFFFFF) +
((bHiShr * aLo) & 0xFFFFFFFF) +
((aLo * bLo) >> 32)) >> 32; |
Neat. Thanks. |
Add test cases provided by @azrafe7, and update carry calculation to a method that seems to actually work.
I can't say that I fully understand why exactly, but I too compared this against the naive method (using a native uint128 type), and this approach seems to work when the previous one does not always give exactly the same results.