Skip to content
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

Fix resizeRawFloat preserving the correct sExp for NaN/Infty inputs #85

Closed
wants to merge 1 commit into from

Conversation

jerryz123
Copy link
Contributor

The sAdjustedSexp calculation does not correctly represent the sExp field when the input is NaN or infinity. This corrects that, I believe.

Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious where this is cropping up.

The HardFloat spec says that this should not be needed, as the other fields in the RawFloat are don't-cares if isInf or isNaN is true (excepting NaN payload propagation, which doesn't matter for us). See Sec. 5.3 of https://www.jhauser.us/arithmetic/HardFloat-1/doc/HardFloat-Verilog.html

@jerryz123
Copy link
Contributor Author

I'm using resizeRawFloat to upconvert sub-F64 values such that I can use the same F64 RecFNToIN to do IEEE fp-to-int conversions.

The problem I'm seeing is that the conversion of a rawFN to a recFN expressed here:

rawIn.sign ##
(Mux(rawIn.isZero, 0.U(3.W), rawIn.sExp(expWidth, expWidth - 2)) |
Mux(rawIn.isNaN, 1.U, 0.U)) ##
rawIn.sExp(expWidth - 3, 0) ##
rawIn.sig(sigWidth - 2, 0)
does not construct a correct recFN representation of a infinity or NaN when the input rawFloat is resized up (the high non-sign bits of the exponent are not 11).

Perhaps this expression is not intended to work with resized rawFNs?

@aswaterman
Copy link
Member

Yeah, that expression is only valid in its context; it is (legitimately) making assumptions about what rawFloatFromFN might return.

@jerryz123 jerryz123 closed this Sep 11, 2024
@aswaterman aswaterman deleted the resize_raw_nans branch September 11, 2024 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants