fix(extF80): rem and sqrt mishandle pseudo-infinity inputs#40
Closed
johnwbyrd wants to merge 1 commit intoucb-bar:masterfrom
Closed
fix(extF80): rem and sqrt mishandle pseudo-infinity inputs#40johnwbyrd wants to merge 1 commit intoucb-bar:masterfrom
johnwbyrd wants to merge 1 commit intoucb-bar:masterfrom
Conversation
In extFloat80, infinity can be encoded two ways: the canonical form
{exp=0x7FFF, sig=0x8000000000000000} with J=1, and the pseudo-infinity
{exp=0x7FFF, sig=0} with J=0. Both represent the same value (infinity).
extF80_rem: when the divisor B is pseudo-infinity, the infinity handler
doubles expB (to force an early return of A), but leaves sigB=0. The
subsequent normalization guard sees sigB=0 and jumps to the invalid
label, returning NaN with the invalid flag. The correct result is the
dividend A, unchanged. Fix: set sigB to the canonical J=1 value in
the infinity handler so the normalization guard passes through.
extF80_sqrt: when the operand is pseudo-infinity, the positive-infinity
path does `return a`, passing the non-canonical encoding through as the
result. Fix: construct and return the canonical infinity encoding.
Demonstrated wrong results before this fix:
rem(1.0, {exp=0x7FFF, sig=0}) = NaN with invalid flag
Expected: 1.0 with no flags
({exp=0x7FFF, sig=0} is infinity; rem(finite, Inf) = finite)
sqrt({exp=0x7FFF, sig=0}) = {0x7FFF, 0x0000000000000000}
Expected: {0x7FFF, 0x8000000000000000} (canonical +Inf)
Author
|
Closing this PR. The fix here is superseded by a later, more comprehensive PR that addresses non-canonical extFloat80 encodings across all extF80 operations. |
2 tasks
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
The extFloat80 format stores the integer bit (J, bit 63 of the significand) explicitly. Every combination of exponent and significand is a valid encoding with a defined mathematical value. The original Intel 8087 hardware defined and correctly processed all such encodings -- unnormals, pseudo-denormals, pseudo-infinities, and pseudo-NaNs -- as described in Intel's US Patent 4,325,120 and in every subsequent Intel architecture manual covering the x87 FPU.
In extFloat80, infinity can be encoded two ways: the canonical form
{exp=0x7FFF, sig=0x8000000000000000}with J=1, and the pseudo-infinity{exp=0x7FFF, sig=0x0000000000000000}with J=0. Both represent the same value (infinity).Wrong results demonstrated
rem(1.0, {exp=0x7FFF, sig=0})1.0with no flags (rem(finite, Inf) = finite)sqrt({exp=0x7FFF, sig=0}){0x7FFF, 0x0000..0}(non-canonical){0x7FFF, 0x8000..0}(canonical +Inf)A standalone program demonstrating both: https://gist.github.com/johnwbyrd/cb75a1844661677fe614bc39f171fe39 (Bugs 7 and the sqrt case)
Root causes and fixes
extF80_rem.c: When the divisor B is pseudo-infinity, the infinity handler doubles
expB(to force an early return of A unchanged), but leavessigB=0. The subsequent normalization guard seessigB=0and jumps to theinvalidlabel, returning NaN. The fix setssigBto the canonical J=1 value in the infinity handler so the normalization guard passes through. The actual value ofsigBdoesn't matter... the hugeexpDiffcausesgoto copyAbeforesigBis used in any computation.extF80_sqrt.c: When the operand is pseudo-infinity, the positive-infinity path does
return a, passing the non-canonical encoding through as the result unchanged. The fix constructs and returns the canonical infinity encoding.Related
Suggested test plan
testsoftfloat -level 1 extF80_remandtestsoftfloat -level 1 extF80_sqrt-- both should pass across all rounding modestestsoftfloat -level 1 extF80_divas a regression checktestsoftfloat -level 1 f64_addto confirm non-extF80 operations are unaffected