openhwgroup / cvfpu

Parametric floating-point unit with support for standard RISC-V formats and operations as well as transprecision formats.
Apache License 2.0
418 stars 112 forks source link

[BUG] Incorrect Accumulation of ‘OF’ Flag in fflags After Executing fsqrt.d on Infinity #111

Open youzi27 opened 4 months ago

youzi27 commented 4 months ago

Bug Description

When executing the fsqrt.d instruction on a double-precision floating-point value representing positive infinity (0x7ff0000000000000), the Overflow (OF) flag in the fflags register is erroneously set. According to the IEEE 754 standard and the RISC-V specification, the fsqrt.d operation should not lead to an overflow situation when the input is infinity. This also results in inconsistency with Spike's output

Expected Behavior: The OF flag in the fflags register should remain clear (i.e., not set) after performing a square root operation on an infinite value, as the result is well-defined and should be positive infinity.

Actual Behavior: The OF flag is set in the fflags register, indicating an overflow, which contradicts the expected behavior defined by the IEEE 754 standard and RISC-V floating-point operation guidelines.

Steps to Reproduce:

  1. Load a double-precision floating-point register with the value 0x7ff0000000000000 (positive infinity).
  2. Execute the fsqrt.d instruction on this register.
  3. Check the fflags register; observe that the OF flag is incorrectly set.

EDIT: A PR has been submitted https://github.com/pulp-platform/fpu_div_sqrt_mvp/issues/25

EDIT: And See: https://github.com/openhwgroup/cva6/issues/2058

pascalgouedo commented 1 week ago

Same answer than #110

youzi27 commented 1 week ago

Hi @pascalgouedo,

Thank you very much for your detailed explanation.

The bug occurs in the commit version of cvfpu: https://github.com/openhwgroup/cvfpu/commit/79e453139072df42c9ec8f697132ba485d74e23d (v0.8.1), and as you mentioned, I noticed that this version has not yet included "THMULTI."

Additionally, the issue was raised in April, and from checking the commit records, it seems that "THMULTI" was only introduced to cvfpu in May.

Furthermore, I have reviewed the bugs that have been reported previously and did not see this mentioned, which is why I raised an issue. I will verify whether adding "THMULTI" resolves the bug.

Thank you very much.