Fix UInt128 to double conversion for values >= 2^104 #122534
Open
+53
−3
Conversation
This file contains hidden or 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
dotnet-policy-service
bot
added
the
community-contribution
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the UInt128 to double conversion for values >= 2^104. The issue was caused by incorrect handling of precision loss during the conversion, which has been corrected by implementing proper sticky bit rounding.
- Fixed the rounding logic in the UInt128 to double explicit conversion operator
- Added a regression test to verify the fix
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/UInt128.cs | Changed the conversion logic to use a sticky bit (1 if any of the lower 24 bits are set, 0 otherwise) instead of OR-ing the lower 24 bits directly, ensuring proper rounding for large values |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/UInt128Tests.cs | Added a regression test that validates the conversion of a large UInt128 value (>= 2^104) to double produces the correct result |
huoyaoyuan
reviewed
Dec 14, 2025
Member
huoyaoyuan
left a comment
huoyaoyuan
left a comment
There was a problem hiding this comment.
Can you please check if the issue is also present for Int128?
| // lowest 24 bits and then or's them back to ensure rounding stays correct. | ||
|
|
||
| double lower = BitConverter.UInt64BitsToDouble(TwoPow76Bits | ((ulong)(value >> 12) >> 12) | (value._lower & 0xFFFFFF)) - TwoPow76; | ||
| double lower = BitConverter.UInt64BitsToDouble(TwoPow76Bits | ((ulong)(value >> 12) >> 12) | ((value._lower & 0xFFFFFF) != 0 ? 1UL : 0UL)) - TwoPow76; |
Member
There was a problem hiding this comment.
Is it correcting the behavior for some case? If not, it's a deoptimization.
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/UInt128Tests.cs
Outdated
Show resolved
Hide resolved
skyoxZ
reviewed
Dec 14, 2025
Author
|
@dotnet-policy-service agree |
Author
Yes it's also present for Int128, also this fix also applies to Int128 |
- Correctly calculate the sticky bit when converting large UInt128 values (>= 2^104) to double to prevent precision loss. - Optimize the upper bits extraction by accessing _upper directly instead of shifting. - Add regression tests for Int128 to ensure it is also fixed. - Update test comments to better explain the test cases. Fixes dotnet#122203
sdcb
force-pushed
the
fix-uint128-conversion
branch
from
c808380 to
5473f4b
Compare
huoyaoyuan
reviewed
Dec 16, 2025
Member
huoyaoyuan
left a comment
huoyaoyuan
left a comment
There was a problem hiding this comment.
The numbers can be constructed more intuitively (also Int128 ones)
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/UInt128Tests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/UInt128Tests.cs
Outdated
Show resolved
Hide resolved
Use decimal literals instead of scientific notation and hex constructors instead of bit shifts for clearer test value representation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 #122203