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

[Programmingkid]

> On Sep 17, 2018, at 7:46 PM, Peter Maydell <address@hidden> wrote:
> 
> On 18 September 2018 at 00:18, Programmingkid <address@hidden> wrote:
>> 
>>> 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?
> 
> See my previous email -- the spec suggests that "round" is
> "inexact but not overflow".

I couldn't find anything in my pdf document about round being defined as 
inexact but not overflow. Could you send me a link to your document? 

> If so you should be able to implement
> it purely in the target/ppc code.

The Fraction Rounded (FR) flag is set when the intermediate result of an IEEE 
754 calculation does not match the rounded answer. The only way to find out if 
the intermediate result is not equal to the rounded answer is to do a 
comparison in the code that does the actual calculations. For the floating 
point add instruction the code that handles the calculation is the 
fpu/softfloat.c:float64_add() function. One way to implement the Fraction 
Rounded flag in target/ppc only would be to implement the actual calculation 
code in the target/ppc/fpu_helper.c file. @David Gibson - did you want to do 
that?