rounding in to_ufixed/to_sfixed(real) not so precise

Hi,

i wrote an entity that implements a lookuptable for sine-values from 0 to 90 degree. The result is of unconstrained type ufixed(), to be flexible in the digit precision. The values get set like this:

if angle_i <= to_sfixed( 0.0, angle_i) then
	result_o <= to_ufixed(0.0, result_o);
elsif angle_i <= to_sfixed(1.0, angle_i) then
	result_o <= to_ufixed(0.017452406, result_o);
elsif ...

I wrote a testbench to verify correct values. I chose ufixed(1 downto -5) as the type for result_o. When the input angle_i is between 0.0 and 1.0, 0.017452406 gets assigned. With 5 bits, an LSB is 1/32th.

What is expected

The value 0.017452406 represends approx 0.558476992 LSBs. So by an external estimated guess when knowing that the value gets rounded, one would expect the resulting value is (in bit-notation) 00.00001 because 0.55 gets rounded up to 1 LSB.

What is encountered

But the simulation reveals 00.00000.

Background

I already opened an issue in the ghdl bugtracker, since i simulated with this compiler: https://github.com/ghdl/ghdl/issues/2454 There they redirected me to here.

Explanation of the calculation

The fixed package uses 3 guard bits below signal'right. The real gets converted into the fixed value and then the decision for rounding is made. Rounding is implemented with ieee754 in mind. Recap: "when the value is exactly 0.5, then it is rounded to the even LSB. When LSB is 0, nothing is done, when it is 1, 1 LSB is added to the value".

The input with more precision in binary notation is 00.0000_0|100|_0111_0111_1100. The guard bits are framed with pipe characters. so after throwing all bits after -8 away, the guard bits remain. These are 100, which is exactly half an LSB which results in rounding to even, which does not change the value and the result is 00.00000.

Critique

The rounding is not done right. The implementation forces all values between 0.5 and 0.625 LSBs to be forced into "round to even". So 1/16 of all values gets rounded wrongly (1/8 is in between 0.5 and 0.625 and half of that gets not rounded up, because LSB is already 0).

Approach for a solution

Current implementation of to_ufixed(real): https://opensource.ieee.org/vasg/Packages/-/blob/release/ieee/fixed_generic_pkg-body.vhdl#L2561

So only when the value is 0.5 LSB, the round to even should be applied. The For-Loop sets all the bits with the guard-bits. The remainder stored in presult after the loop is completely ignored. In my example (-6 downto -8) are the guard bits. I will use their indexes here for reference.

The exact values of -7 and -8 is not relevant in this function. They are only relevant when -6 is set to '1' and then only the check is made, if they are all '0'. So to avoid the rounding down of 0.5 to 0.625 LSB, the following changes could be made:

--- fixed_generic_pkg-body.vhdl 2023-06-13 10:42:38.051416900 +0200
+++ fixed_generic_pkg-body_patch.vhdl   2023-06-13 10:54:50.866401800 +0200
@@ -2605,6 +2605,11 @@
       end if;
     end loop;
     if guard_bits > 0 and round_style = fixed_round then
+      if guard_bits > 1 and presult > 0.0 then
+        -- give the values > 0.5 LSB and < 0.625 LSB the little bump to be rounded
+        -- up instead of prbably rounding them down because of "round to even"
+        Xresult(Xresult'right) := '1';
+      end if;
       result := round_fixed (arg => Xresult (left_index
                                              downto right_index),
                              remainder => Xresult (right_index-1 downto

This sets the LSB of of the guard bits to 1 when the remainder is not 0, so this has only impact onto the the decision if we want to round up or make the round to even decision.

to_sfixed(real) has currently the same behaviour and has to be looked into, too.