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

CVFPU incorrectly suppresses overflow flag on I2F conversions #123

Open
michael-platzer opened this issue May 24, 2024 · 2 comments · May be fixed by #125
Open

CVFPU incorrectly suppresses overflow flag on I2F conversions #123

michael-platzer opened this issue May 24, 2024 · 2 comments · May be fixed by #125

Comments

@michael-platzer
Copy link
Contributor

michael-platzer commented May 24, 2024

According to IEEE 754-2008, converting an integer to a floating-point value should trigger an overflow exception in case the rounded value exceeds the range of the floating-point type.

image

However, CVFPU suppresses the overflow flag when the source operand is an integer (i.e., on I2F conversions), and sets the invalid flag instead:

assign fp_regular_status.NV = src_is_int_q & (of_before_round | of_after_round); // overflow is invalid for I2F casts
assign fp_regular_status.DZ = 1'b0; // no divisions
assign fp_regular_status.OF = ~src_is_int_q & (~info_q.is_inf & (of_before_round | of_after_round)); // inf casts no OF
assign fp_regular_status.UF = uf_after_round & fp_regular_status.NX;
assign fp_regular_status.NX = src_is_int_q ? (| fp_round_sticky_bits) // overflow is invalid in i2f
: (| fp_round_sticky_bits) | (~info_q.is_inf & (of_before_round | of_after_round));
assign int_regular_status = '{NX: (| int_round_sticky_bits), default: 1'b0};

The comment on line 721 is incorrect: an overflow should not trigger an invalid exception on I2F conversions.

I believe this has been confused with the opposite case, converting a floating-point value to an integer, which indeed is not supposed to produce an overflow exception:

image

For single-precision and double-precision types an overflow cannot happen, because the exponent range is always large enough such that any 32-bit or 64-bit integer value does not exceed it. However, for half-precision values an overflow can occur on I2F conversions.

@lucabertaccini @pascalgouedo @stmach Please let me know whether my assessment is correct. I will then move forward with a PR to fix the behavior.

@pascalgouedo
Copy link

Hi @michael-platzer
As long as it doesn't change the behavior when using only Single-Precision format, it should be fine.
But you will have to prove it 😀.

@michael-platzer
Copy link
Contributor Author

Hi @pascalgouedo, I assume you are referring to the changes in PR #125. Those do change the behavior: they replace the current non-IEEE-compliant behavior with IEEE-compliant behavior.

The reason you are not seeing this issue when using only the single-precision format is that there can be no overflow on I2F conversions for this format. This is easy to prove: the largest normal value that can be represented by a single-precision floating-point number is $2^{127} × (2 − 2^{−23})$, which is way larger than any value that can be represented by a 32 or 64-bit integer number.

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 a pull request may close this issue.

2 participants