Re: [Qemu-devel] [PATCH] fix setting the FPSCR[FR] bit

[Programmingkid]

> On Sep 17, 2018, at 5:25 PM, Peter Maydell <address@hidden> wrote:
> 
> On 17 September 2018 at 22:18, John Arbuckle <address@hidden> wrote:
>> https://www.nxp.com/files-static/product/doc/MPCFPE32B.pdf
>> Page 2-8 in table 2-4 is where the description of this bit can be found.
>> 
>> It is described as:
>> 
>> Floating-point fraction rounded. The last arithmetic, rounding, or 
>> conversion instruction incremented the fraction. This bit is NOT sticky.
>> 
>> This patch actually implements the setting and unsetting of this bit.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> ---
>> fpu/softfloat.c               | 12 ++++++++++--
>> include/fpu/softfloat-types.h |  1 +
>> target/ppc/fpu_helper.c       | 12 ++++++++++++
>> 3 files changed, 23 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 59ca356d0e..c5378ae9e8 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -751,9 +751,17 @@ float64 __attribute__((flatten)) float64_add(float64 a, 
>> float64 b,
>> {
>>     FloatParts pa = float64_unpack_canonical(a, status);
>>     FloatParts pb = float64_unpack_canonical(b, status);
>> -    FloatParts pr = addsub_floats(pa, pb, false, status);
>> +    FloatParts intermediate_parts = addsub_floats(pa, pb, false, status);
>> 
>> -    return float64_round_pack_canonical(pr, status);
>> +    float64 rounded_result = 
>> float64_round_pack_canonical(intermediate_parts,
>> +                                                          status);
>> +    FloatParts rounded_parts = float64_unpack_canonical(rounded_result, 
>> status);
>> +
>> +    if (rounded_parts.frac != intermediate_parts.frac) {
>> +        float_raise(float_flag_round, status);
>> +    }
>> +
>> +    return rounded_result;
>> }
> 
> Only changing the add instruction looks very definitely wrong.
> Doing a pack-unpack-compare looks dubious.

Now that I think about it I see it could be done differently (more efficiently).

> I think that you can do this by using the existing overflow and
> inexact flags.

I'm not sure how I would do this. Do you think they are mutually exclusive?

> If that is not possible (but I'm pretty sure it should
> be doable), then the next best approach is probably to have the new
> float flag be set at the right point in the round-and-pack code.

This sounds like a good idea. The round_canonical() function is where I would 
place such code. This function is used by several arithmetic instructions so it 
might be able to more than just the add function.

Thank you for reviewing my patch.