[Top][All Lists]
[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
- [Qemu-devel] [PATCH 0/5] target-arm: Implement UDIV/SDIV and fused multiply-accumulate, Peter Maydell, 2011/09/28
- [Qemu-devel] [PATCH 5/5] target-arm: Implement VFPv4 fused multiply-accumulate insns, Peter Maydell, 2011/09/28
- [Qemu-devel] [PATCH 3/5] target-arm: Add ARM UDIV/SDIV support, Peter Maydell, 2011/09/28
- [Qemu-devel] [PATCH 1/5] softfloat: Reinstate accidentally disabled target-specific NaN handling, Peter Maydell, 2011/09/28
- [Qemu-devel] [PATCH 2/5] target-arm: v6 media multiply space: UNDEF on unassigned encodings, Peter Maydell, 2011/09/28
- [Qemu-devel] [PATCH 4/5] softfloat: Implement fused multiply-add, Peter Maydell, 2011/09/28
- Re: [Qemu-devel] [PATCH 0/5] target-arm: Implement UDIV/SDIV and fused multiply-accumulate, Blue Swirl, 2011/09/28