[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] softfloat: Handle float_muladd_negate_c when pr
From: |
Richard Sandiford |
Subject: |
Re: [Qemu-devel] [PATCH] softfloat: Handle float_muladd_negate_c when product is zero |
Date: |
Mon, 21 Jan 2013 22:20:40 +0000 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Peter Maydell <address@hidden> writes:
> On 21 January 2013 21:32, Richard Sandiford <address@hidden> wrote:
>> Honour float_muladd_negate_c in the case where the product is zero and
>> c is nonzero. Previously we would fail to negate c.
>>
>> Seen in (and tested against) the gfortran testsuite on MIPS.
>>
>> Signed-off-by: Richard Sandiford <address@hidden>
>> ---
>> fpu/softfloat.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index ac3d150..0028415 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -2234,6 +2234,9 @@ float32 float32_muladd(float32 a, float32 b, float32
>> c, int flags STATUS_PARAM)
>> }
>> }
>> /* Zero plus something non-zero : just return the something */
>> + if (flags & float_muladd_negate_c) {
>> + signflip ^= 1;
>> + }
>> return make_float32(float32_val(c) ^ (signflip << 31));
>
> This is a correct change in that it fixes a definite bug and gives
> the right results, but I wonder if it might be clearer to instead
> change the return to read:
>
> return packFloat32(cSign ^ signflip, cExp, cSig);
>
> ?
>
> That would mean we consistently handle the negate_c flag by:
> * flip cSign as soon as we split c into its component fields
> * never refer to c again
>
> (the source of the bug here is me trying to be clever and avoid
> reassembling the float, but forgetting that cSign might have
> changed.)
Heh, wondered about that, but not knowing the code, I went for the
"safe" option. I'll do it as you suggest, thanks.
Richard