-
Notifications
You must be signed in to change notification settings - Fork 1.6k
<charconv>: simplify _Assemble_floating_point_value and optimize _Right_shift_with_rounding #1220
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
<charconv>: simplify _Assemble_floating_point_value and optimize _Right_shift_with_rounding #1220
Conversation
When the significand carries over to a higher bit after rounding up, we need to renormalize the significand and increase the exponent to keep the significand within the normalized range. (example: 0x1.fffffffffffff8p+0 rounds to 0x1.0000000000000p+1) In some cases, the new exponent becomes greater than the maximum exponent of the floating point format, so the result overflows. (example: 0x1.fffffffffffff8p+1023 overflows to inf) In some cases, the mantissa of a subnormal value becomes normalized after rounding up, so the result becomes a normal value. (example: 0x0.fffffffffffff8p-1022 rounds to 0x1.0000000000000-p1022) This commit modifies the behavior of _Assemble_floating_point_value_t (and renames it to _Assemble_floating_point_value_no_shift to avoid potential ODR issues) to gracefully handle the cases above, and removes special case handling code from _Assemble_floating_point_value.
Use the branchless rounding technique in hex to_chars with minor modifications to handle input tail bits.
StephanTLavavej
left a comment
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.
Thanks! I think I understand the overall approach here, and the new assembly process makes more sense than the previous control flow. I'm marking this as Request Changes for the "tail bits" bug that I believe I found (plus testing). There's an additional question about space/time improvements for the rounding technique.
|
Here are my benchmark results (Intel Core i5-8400, fixed CPU clock speed at 2.7 GHz, VS 2019 16.8 Preview 2, Clang/LLVM 11.0.0-rc1). The measured times are average nanoseconds per floating-point string. Times with default dynamic CPU clock speed setting are around 0.7x the values below.
|
StephanTLavavej
left a comment
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.
Thanks for the detailed perf data - nice improvements for MSVC! I'll push a one-line change to use logical AND with bool. FYI @cbezault as you had previously approved.
|
Thanks again for improving |
Modify the behavior of
_Assemble_floating_point_value_no_shift(renamed to avoid potential ODR issue) to gracefully handle the special cases below, and remove special case handling code from_Assemble_floating_point_value.(example: 0x1.fffffffffffff8p+0 rounds to 0x1.0000000000000p+1)
(example: 0x1.fffffffffffff8p+1023 overflows to ∞)
(example: 0x0.fffffffffffff8p-1022 rounds to 0x1.0000000000000-p1022)
Optimize
_Right_shift_with_roundingwith the branchless rounding technique in hexto_chars.