-
Notifications
You must be signed in to change notification settings - Fork 212
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
FPU more fflags issues and a few logic bugs #791
Comments
Bug 1: fclass does not support classifying subnormals The following code snippet will correctly classify a subnormal number. The remaining classification code already supported subnormal usage.
This had some follow on side effects all over the code base as we now signal a subnormal class we need to expand the conditions in quite a few places. E.g. zero detection in the addsub.
Note the "FPU_SUBNORMAL_SUPPORT". This is used to select a path, as the existing zero detection code works correctly if we support subnormals. If we don't we need to add additional checks. |
Bug 4: fsgnj, fsgnjn, and fsgnjx are using the flushed subnormal and not the subnormal itself. Added a FPU_SUBNORMAL_SUPPORT gate, granted might not be needed as it will always work. Prefered to be consistent :)
|
bug 6: Rounding modes: Added two new floating point constants to the package file to assist with rounding functions:
Added support for round to nearest, ties to max magnitude.
Removed the '0' mask bit in the FRM CSR transfer.
Round towards +/- infinity did not take sign into account.
In case of overflow the normalizer would inject +/- infinity. This is incorrect behavior per IEEE754. Per chapter 4.3.2 Directed Rounding Attributes roundTowardZero can never produce and infinite result. For roundTowardPositiveInfinity can never product a negative infinity and the reverse for roundTowardNegativeInfinity.
With these changes all riscvof-arch fadd/fsub tests pass. |
Bug 5: Multiplier exception handling. |
Bug 7: Float to integer conversion has missing flag settings, some incorrect rounding and missing exception handling. |
Currently running a regression run to ensure all the bug fixes are working correctly. Will push a PR afterwards. There are some caveats to the passing test, which is the reference testing. As the Sail simulator used for the reference test runs the code with subnormal support, there are some compares that will fail because of this. Working on a fix to the reference code generation to prevent this. We have one test case: block_sim/rv32i_m_Zfinx_fcvt.wu.s_b24 which fails vs the Sail reference bit passes our Imperas reference model. The error here is a disagreement on the NX flag. Sail says NX we say no-NX. Its about 6 cases but need to review them to be able to waive them. |
PR #794 created. |
Another great input! Thank you very much, @mikaelsky! Give me some time to look through this... 😅 |
I know, its a lot :) |
Describe the bug
This is continuation of the previous fflags issue. This is a mix of various float corner cases.
Each bug will have a detailed comment
Bug 1: fclass.s does not recognize subnormals correctly
Bug 2: feq, fle and flt do not treat subnormals as zero, do not correctly report NV for unordered numbers (q/sNAN), and fle had a logic bug of both numbers are negative and equal.
Bug 3: fmin/fmax do not treat subnormals as zero and do not report NV when a number is sNAN [still work in progress]
Bug 4: fsgnj, fsgnjn, fsgnjx uses the flushed subnormal number and not the real number.
Bug 5: multiplier with the fixed fclass the multiplier needs to self detect flushed subnormals. When OF/UF is detected NX isn't asserted. Post multiplication updated with subnormal as zero detection. [Still a work in progress]
Bug 6: Rounding modes does not round correctly, round to nearest towards +MAX added, bypasses added for rounds that cannot create infinity results.
Bug 7: Float to integer conversion has multiple issues with setting flags and handling corner cases.
I added the following to help with implementing subnormal support in a future FPU variant. Default is no sub-normal support in hardware.
To Reproduce
See individual bug comments
Expected behavior
See individual bug comments
Screenshots
NA
Environment:
Hardware:
Additional context
See follow on comments for details
The text was updated successfully, but these errors were encountered: