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

Conversion from FP32 -2^31 to INT32 sets NV status #83

Closed
pashenden opened this issue Mar 31, 2023 · 5 comments
Closed

Conversion from FP32 -2^31 to INT32 sets NV status #83

pashenden opened this issue Mar 31, 2023 · 5 comments

Comments

@pashenden
Copy link

In v0.7.0, converting the FP32 value -2147483648.0 to INT32 produces the result 0x80000000 and sets the NV status flag. However, the input value can be exactly represented as an INT32.

Test case:

  operands_i = '{0: 32'hcf000000, 1: 32'h00000000, 2: 32'h00000000};
  rnd_mode_i = RNE;
  op_i = F2I;
  op_mod_i = 1'b0;
  src_fmt_i = FP32;
  int_fmt_i = INT32;

Root cause:
These lines do not take account of the fact that for the valid input value, the exponent is exactly 31. If the sign is negative and the mantissa is 0x80000000 in this case, there should not be overflow.

@pascalgouedo
Copy link

Hi @pashenden

It seems same kind of bug as already reported here.
But maybe not the same source line to correct.

Thanks for reporting.

@pashenden
Copy link
Author

Yes, same bug. In that issue, the operand value is 0xcf000000, and the fcvt.w.s instruction would invoke the F2I operation in the FPU. So fixing the FPU should address the bug in the processor core.

gregdavill added a commit to gregdavill/cvfpu that referenced this issue Aug 2, 2023
Correctly convert signed integers of negative max-magnitude value
without erroneously flagging an overflow condition

issue: openhwgroup#83

Signed-off-by: Greg Davill <greg.davill@gmail.com>
@gregdavill
Copy link
Contributor

@pascalgouedo Can this issue be closed now with #97 merged?

@pascalgouedo
Copy link

Hi @gregdavill
The work in on-going on bug resolution w/ formal verification guy.
We will close all resolved opened issue once definitely confirmed resolved by formal runs launched by him.

@zarubaf
Copy link
Contributor

zarubaf commented Oct 26, 2023

Fixed with release v0.8.1.

@zarubaf zarubaf closed this as completed Oct 26, 2023
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

No branches or pull requests

4 participants