On 30 September 2011 16:28, Richard Henderson <r...@twiddle.net> 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
