Round a value below the smallest subnormal in each rounding mode - #1477
Merged
Merged
Conversation
ibmibmibm
force-pushed
the
worktree-ctor-subnormal
branch
from
September 29, 2026 07:34
4d970cc to
bd11148
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1477 +/- ##
=========================================
+ Coverage 98.6% 98.7% +0.1%
=========================================
Files 312 313 +1
Lines 26265 26251 -14
Branches 2264 2249 -15
=========================================
+ Hits 25892 25896 +4
+ Misses 373 355 -18
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
ibmibmibm
marked this pull request as ready for review
September 29, 2026 12:10
mborland
approved these changes
Sep 29, 2026
mborland
left a comment
Member
There was a problem hiding this comment.
In the test file you only have testing for 3 of the 5 rounding modes. Can you please add paths so that all are hit? Thanks
mborland
requested changes
Sep 29, 2026
mborland
left a comment
Member
There was a problem hiding this comment.
In the test file you only have testing for 3 of the 5 rounding modes. Can you please add paths so that all are hit? Thanks
- The constructors of decimal32_t, decimal64_t and decimal128_t had a special branch for a significand of one digit below the smallest exponent. It rounded a tie up, and it ignored the upward and the downward modes below the first step. The branch is removed. The general branch after it rounds these values correctly. - coefficient_rounding set the coefficient to zero when all its digits drop, and it did not round. It now rounds a zero with a sticky bit through fenv_round, thus a directed mode can give the smallest subnormal. A zero input still gives zero. - decimal128_t called coefficient_rounding only for a coefficient type of 128 bits or more. A narrower coefficient far below the smallest exponent read past the end of the pow10 table. This is undefined behavior, and it can stop the program with SIGFPE. It now takes the same path. - For a 128-bit coefficient with few digits, coefficient_rounding divided in the narrow significand type. A shift of more than 9 or 19 digits did not fit in that type, thus the power of ten lost its high bits. The quotient was wrong, or the divide by zero stopped the program. The narrow divide now also needs a shift that fits. - Add a test for the four cases.
ibmibmibm
force-pushed
the
worktree-ctor-subnormal
branch
from
September 30, 2026 02:10
bd11148 to
4d43e0f
Compare
Contributor
Author
|
All rounding mode tests were added. |
This was referenced Oct 1, 2026
mborland
pushed a commit
that referenced
this pull request
Oct 2, 2026
- The `decimal128_t` multiply has a separate path for an exponent sum below `-6142`. It does not expand the significands, so the product can have 1 to 68 digits. - The path rounded the product only above 38 digits, the `digits10` of the 128-bit significand type. A product of 35 to 38 digits then went to `pack_in_range` unrounded. The encoder wrote its extra digits into bits outside the significand field. - The limit is now `detail::precision_v<ReturnType>`, 34 digits for `decimal128_t`. - The new test `github_issue_1482.cpp` multiplies products of 35 to 38 digits in each rounding mode. Before the change, each check fails except one product that rounds to zero. A sweep of 20000 such products gives 962 wrong results in each of the five rounding modes on `develop`, and none after the change. The change needs #1477, which is in `develop`. Issue #1482 gives the values, the sweep, the comparison with `develop` and the benchmark. Fixes #1482
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.
decimal32_t,decimal64_tanddecimal128_thad a special branch fora significand of one digit below the smallest exponent. It rounded a tie up, and it ignored
the upward and the downward modes below the first step. The branch is removed. The general
branch after it rounds these values correctly in each mode.
coefficient_roundingset it to zero, and it did notround. It now rounds a zero with a sticky bit through
fenv_round. A directed mode can thengive the smallest subnormal, and a zero input still gives zero.
decimal128_tcalledcoefficient_roundingonly for a coefficient type of 128 bits or more.A narrower coefficient far below the smallest exponent read past the end of the
pow10table, and
decimal128_t{10, -6231}stopped the program with SIGFPE. It now takes the samepath.
coefficient_roundingdivided in the narrowsignificand type. A large shift did not fit in that type. The quotient was wrong, or the
divide by zero stopped the program, for example in a conversion from
decimal128_ttodecimal32_t. The narrow divide now also needs a shift that fits.github_issue_1476.cppchecks the four cases for the three types, in themodes with a defect. Before the change it stops with SIGFPE.
A sweep checks values below the smallest exponent through the constructor, the string parser
and the multiply. After the change, it gives no wrong result in the five rounding modes. Issue #1476
gives the values, the sweeps, the comparison with
developand the benchmark.Fixes #1476