Fix Dragon4 shortest formatting for exact powers of two#131131
Merged
tannergooding merged 2 commits intoJul 21, 2026
Merged
Conversation
hasUnequalMargins compared the mantissa against a 32-bit `1U` shift, which truncated for double (DenormalMantissaBits == 52) and forced equal margins for every exact power of two. This produced non-round-trippable shortest strings such as `2^-25` and `2^-958`. Use a 64-bit shift. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-numerics |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a correctness issue in the generic Number.Dragon4 floating-point shortest-formatting path where the implicit-bit comparison used a 32-bit shift (1U << ...), producing the wrong result for double and causing non-roundtrippable output for certain exact powers of two.
Changes:
- Widened the implicit-bit check to use a 64-bit shift (
1UL << TNumber.DenormalMantissaBits) when computinghasUnequalMargins. - Added a regression test covering positive/negative
doubleexact power-of-two cases that previously formatted to a non-roundtrippable shortest string.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Number.Dragon4.cs | Fixes hasUnequalMargins computation by using a 64-bit shift to correctly detect the implicit bit for double. |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DoubleTests.cs | Adds regression coverage ensuring affected exact power-of-two double values format and parse round-trip as expected. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
tannergooding
enabled auto-merge (squash)
July 21, 2026 02:44
PranavSenthilnathan
approved these changes
Jul 21, 2026
This was referenced Jul 21, 2026
This was referenced Jul 22, 2026
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
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.
Summary
Number.Dragon4computedhasUnequalMarginsby comparing the extracted mantissa against1U << TNumber.DenormalMantissaBits.1Uis 32-bit, and C# masks the shift count to 5 bits for a 32-bit operand, so fordouble(DenormalMantissaBits == 52) this evaluates to1U << (52 & 31)==1U << 20, not1UL << 52.As a result
hasUnequalMarginswas wronglyfalsefor every exact power of two indouble, so Dragon4 used equal rounding margins and produced a non-round-trippable shortest string for cases like2^-25and2^-958:2^-252.980232238769531E-082.9802322387695312E-082^-9584.104536801298376E-2894.1045368012983762E-289The "before" strings parse back to the adjacent lower-magnitude
double.The fix widens the shift to
1UL, which is correct for all supported types (float23,double52,Half10,BFloat167 � all< 64).This is a regression from #102683, which merged the per-type
double/float/Halfpaths into one generic method. The pre-refactordoublepath used1UL << DiyFp.DoubleImplicitBitIndex; the merge keptHalf's1U, which silently truncates fordouble.ExtractFractionAndBiasedExponentandDiyFp.GetBoundarieswere unaffected (both already use1UL).Testing
DoubleTests.ToString_ExactPowerOfTwo_Roundtripscovering �2^-25and �2^-958. It fails without the fix and passes with it.double/float/Half/BFloat16ToStringtests pass (checked runtime).float/doubleround-trips, plus exhaustiveHalf, all pass.Note
GitHub Copilot helped create this PR.