-
Notifications
You must be signed in to change notification settings - Fork 125
Add suppor for bigint to bitwise BIFs #1738
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
base: feature/bigint
Are you sure you want to change the base?
Conversation
db12deb
to
80a3a7e
Compare
803e9e5
to
106850c
Compare
Refactor `bitwise_helper` so it can be changed later for bigint support. Signed-off-by: Davide Bettio <[email protected]>
Add: - `intn_bormn` - `intn_bandmn` - `intn_bxormn` Signed-off-by: Davide Bettio <[email protected]>
Update `bitwise_helper` in order to use new bitwise functions from `intn`. Signed-off-by: Davide Bettio <[email protected]>
Add: - `intn_bsl` - `intn_bsr` Signed-off-by: Davide Bettio <[email protected]>
Update `bsl` and `bsr` functions in order to support bigints. This also removes overflows and undefined behaviors from shift functions. Signed-off-by: Davide Bettio <[email protected]>
Signed-off-by: Davide Bettio <[email protected]>
Just use `intn_bnot` in order to handle bigint arguments. Signed-off-by: Davide Bettio <[email protected]>
106850c
to
aa0245c
Compare
carry = temp >> 32; | ||
} | ||
if (carry) { | ||
out[i] = carry; |
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 am confused by this. Does it overflow out? i is equal to len at this point.
Signedness <- [unsigned] | ||
], | ||
% TODO: make this test work again | ||
% Explaination: |
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.
s/Explaination/Explanation/
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.
Fixed.
|
||
size_t intn_bsl(const uint32_t num[], size_t len, size_t n, uint32_t *out); | ||
size_t intn_bsr( | ||
const uint32_t num[], size_t len, intn_integer_sign_t num_sign, size_t n, uint32_t *out); |
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.
s/uint32_t/intn_digit_t/
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.
Fixed.
size_t intn_bnot(const intn_digit_t m[], size_t m_len, intn_integer_sign_t m_sign, | ||
intn_digit_t out[], intn_integer_sign_t *out_sign); | ||
|
||
size_t intn_bsl(const uint32_t num[], size_t len, size_t n, uint32_t *out); |
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.
s/uint32_t/intn_digit_t/
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.
Fixed.
|
||
if (arg1_size <= BOXED_TERMS_REQUIRED_FOR_INT64) { | ||
uint64_t a = (uint64_t) term_maybe_unbox_int64(arg1); | ||
int64_t result = int64_bsr_safe(a, b); |
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.
int64_bsr_safe takes int64_t
so you don't need to cast the result of term_maybe_unbox_int64
to uint64_t
uint32_t maybe_last_out = (last_digit >> right_shift_n); | ||
|
||
if (initial_zeros + i > new_digits_count) { | ||
abort(); |
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 this assert or should we raise?
intn_digit_t *n; | ||
size_t n_len; | ||
intn_integer_sign_t n_sign; | ||
args_to_bigint(arg1, arg2, tmp_buf1, tmp_buf2, &m, &m_len, &m_sign, &n, &n_len, &n_sign); |
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.
n is not used here, we already decided it's a smaller int. So term_to_bigint(arg1, tmp_buf1, &m, &m_len, &m_sign);
is sufficient.
intn_digit_t *n; | ||
size_t n_len; | ||
intn_integer_sign_t n_sign; | ||
args_to_bigint(arg1, arg2, tmp_buf1, tmp_buf2, &m, &m_len, &m_sign, &n, &n_len, &n_sign); |
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.
n is not used here either, see comment on bsl
Add support for bigint to:
bsl
/bsr
band
/bor
/bxor
bnot
Also
bsl
/bsr
do not depend anymore or undefined behaviors (and they may raise overflow exception).These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).
SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later