qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/5] softfloat: Implement fused multiply-add


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 4/5] softfloat: Implement fused multiply-add
Date: Fri, 30 Sep 2011 17:12:01 +0100

On 30 September 2011 16:28, Richard Henderson <address@hidden> wrote:
> On 09/28/2011 10:27 AM, Peter Maydell wrote:
>>  
>> /*----------------------------------------------------------------------------
>> +| Select which NaN to propagate for a three-input operation.
>> +| For the moment we assume that no CPU needs the 'larger significand'
>> +| information.
>> +| Return values : 0 : a; 1 : b; 2 : c; 3 : default-NaN
>> +*----------------------------------------------------------------------------*/
>> +#if defined(TARGET_ARM)
>> +static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag 
>> bIsSNaN,
>> +                         flag cIsQNaN, flag cIsSNaN, flag infzero 
>> STATUS_PARAM)
> ...
>> +#elif defined(TARGET_PPC)
>> +static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag 
>> bIsSNaN,
>> +                         flag cIsQNaN, flag cIsSNaN, flag infzero 
>> STATUS_PARAM)
>> +{
>
> The function declaration should be outside the #if, so that the interface is
> forcibly consistent across the platforms.

I disagree, in that I dislike #ifdefs which break up a function and
its body like that. (The way I have it here also matches with how
pickNaN() is done.)

>> +    cSig64 = (uint64_t)cSig << 39;
>
> This might be more readable as << (62 - 23), since you've just mentioned the
> explicit bit at bit 62 above.

Sure.

>> +    if (pSign == cSign) {
>> +        /* Addition */
> ...
>> +        shift64RightJamming(zSig64, 32, &zSig64);
>> +        return roundAndPackFloat32(zSign, zExp, zSig64 STATUS_VAR);
>> +    } else {
>> +        /* Subtraction */
> ...
>> +        shift64RightJamming(zSig64, 32, &zSig64);
>> +        return roundAndPackFloat32(zSign, zExp, zSig64 STATUS_VAR);
>> +    }
>> +}
>
> Push those two calls down after the IF?

No objection. (NB that this only applies to float32_muladd, float64_muladd
doesn't have any common calls.)

> Similar comments wrt float64_muladd.  But I don't see any actual logic errors
> wrt the handling of any of the edge cases.

Thanks for the review, the arithmetic and edge cases were a bit
painful to get right so I appreciate somebody else checking it.

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]