-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fixed int128 implementation #5564
Conversation
@@ -509,7 +541,7 @@ class String | |||
end | |||
end | |||
|
|||
private def to_u64_info(base, whitespace, underscore, prefix, strict) | |||
private def to_u128_info(base, whitespace, underscore, prefix, strict) |
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'm concerned about all to_i
calls having to go through 128bit mathematics: it probably slows down the conversion process on processors without 128bit maths support (i.e. most of them).
I think it's a good idea to duplicate this method, and have both to_u64_info
and to_u128_info
. This can probably be done quite easily without extra boilerplate with macros.
If only we had a benchmark suite :)
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, I agree. Also two test are failed because I extend to_u64_info method to support 128 bit integer and now they works incorrect with 64 bit.
I will fix it.
when :u128 | ||
# TODO: implement String#to_u128 and use it | ||
@last = int128(node.value.to_u64) | ||
@last = int128(node.value.to_u128) |
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.
Remove the TODO above
Related to #5545 |
@omotorin Thank you for this! Some things:
|
@omotorin Are you willing to continue? |
Closing stale PR. Int128 support is tracked in #8373 |
I found some bugs in implementation of int128. It related with LLVMConstInt that works only with 64 bit integer and 128 bit will always cut to 64 bit. Also I've implemented to_int128/to_uint128 that needed to correct works codegen for 128 bit integer