Fix compile warnings and refactor ftos_converter.cuh #1679
Merged
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
Signed-off-by: Haoyang Li <[email protected]>
ttnghia
reviewed
Jan 5, 2024
src/main/cpp/src/ftos_converter.cuh
Outdated
| @@ -461,7 +461,7 @@ __device__ inline uint32_t mulPow5divPow2(uint32_t const m, uint32_t const i, in | |||
|
|
|||
| //===== d2s.c and f2s.c from ryu ===== | |||
|
|
|||
| __device__ inline uint32_t decimalLength17(uint64_t const v) | |||
| __device__ inline uint32_t decimalLength(uint64_t const v) | |||
There was a problem hiding this comment.
How about this? Combining both decimalLength functions into:
template<typename T>
__device inline uint32_t decimal_length(T const v) {
static_assert(std::is_integral_v<T> && !std::is_signed_v<T>);
if constexpr(sizeof(T) == sizeof(int64_t)) {
// The average output length is 16.38 digits, so we check high-to-low.
// Function precondition: v is not an 18, 19, or 20-digit number.
// (17 digits are sufficient for round-tripping.)
assert(v < 100000000000000000L);
if (v >= 10000000000000000L) { return 17; }
if (v >= 1000000000000000L) { return 16; }
if (v >= 100000000000000L) { return 15; }
if (v >= 10000000000000L) { return 14; }
if (v >= 1000000000000L) { return 13; }
if (v >= 100000000000L) { return 12; }
if (v >= 10000000000L) { return 11; }
if (v >= 1000000000L) { return 10; }
} else {
// Function precondition: v is not a 10-digit number.
// (f2s: 9 digits are sufficient for round-tripping.)
// (d2fixed: We print 9-digit blocks.)
assert(v < 1000000000);
}
if (v >= 100000000) { return 9; }
if (v >= 10000000) { return 8; }
if (v >= 1000000) { return 7; }
if (v >= 100000) { return 6; }
if (v >= 10000) { return 5; }
if (v >= 1000) { return 4; }
if (v >= 100) { return 3; }
if (v >= 10) { return 2; }
return 1;
}
ttnghia
reviewed
Jan 5, 2024
| @@ -493,7 +493,7 @@ __device__ inline floating_decimal_64 d2d(uint64_t const ieeeMantissa, uint32_t | |||
| uint64_t m2; | |||
| if (ieeeExponent == 0) { | |||
| // We subtract 2 so that the bounds computation has 2 additional bits. | |||
| e2 = 1 - DOUBLE_BIAS - DOUBLE_MANTISSA_BITS - 2; | |||
| e2 = static_cast<int32_t>(1 - DOUBLE_BIAS - DOUBLE_MANTISSA_BITS - 2); | |||
There was a problem hiding this comment.
Please also modify line 499 below, using static_cast instead of C-style cast.
There was a problem hiding this comment.
OMG, there are too many instances of such C-style cast :(
There was a problem hiding this comment.
Modified every C-style cast I found
ttnghia
reviewed
Jan 5, 2024
ttnghia
reviewed
Jan 5, 2024
ttnghia
reviewed
Jan 5, 2024
src/main/cpp/src/ftos_converter.cuh
Outdated
| * @param result: output string | ||
| * @param digits: number of digits after decimal point | ||
| */ | ||
| template <typename T, typename U> |
There was a problem hiding this comment.
Since U depends on T, we can just remove it here, then:
__device__ to_formatted_chars(...) {
static_assert(std::is_same_v<T, floating_decimal_32> || std::is_same_v<T, floating_decimal_64>);
using U = std::conditional_t<std::is_same_v<T, floating_decimal_32>, uint32_t, uint64_t>;
There was a problem hiding this comment.
The same can be applied to the function format_size below.
Co-authored-by: Nghia Truong <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
ttnghia
approved these changes
Jan 8, 2024
|
build |
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.
Fixes #1678
Fix compile warnings in ftos_converter.cuh
Also do some refactoring according to comments from #1676