From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45720) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9fgx-0004K5-Rn for qemu-devel@nongnu.org; Fri, 30 Sep 2011 12:12:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R9fgw-0001hs-P8 for qemu-devel@nongnu.org; Fri, 30 Sep 2011 12:12:03 -0400 Received: from mail-wy0-f173.google.com ([74.125.82.173]:46004) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9fgw-0001hl-Gr for qemu-devel@nongnu.org; Fri, 30 Sep 2011 12:12:02 -0400 Received: by wyh22 with SMTP id 22so1423277wyh.4 for ; Fri, 30 Sep 2011 09:12:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4E85E003.1030901@twiddle.net> References: <1317230853-24970-1-git-send-email-peter.maydell@linaro.org> <1317230853-24970-5-git-send-email-peter.maydell@linaro.org> <4E85E003.1030901@twiddle.net> Date: Fri, 30 Sep 2011 17:12:01 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/5] softfloat: Implement fused multiply-add List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org, patches@linaro.org On 30 September 2011 16:28, Richard Henderson wrote: > On 09/28/2011 10:27 AM, Peter Maydell wrote: >> =C2=A0/*----------------------------------------------------------------= ------------ >> +| 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, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 flag cIsQNaN, flag cIsSNaN, flag infzero STATUS_PARAM) > ... >> +#elif defined(TARGET_PPC) >> +static int pickNaNMulAdd(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag= bIsSNaN, >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 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.) >> + =C2=A0 =C2=A0cSig64 =3D (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. >> + =C2=A0 =C2=A0if (pSign =3D=3D cSign) { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Addition */ > ... >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0shift64RightJamming(zSig64, 32, &zSig64); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0return roundAndPackFloat32(zSign, zExp, zSi= g64 STATUS_VAR); >> + =C2=A0 =C2=A0} else { >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Subtraction */ > ... >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0shift64RightJamming(zSig64, 32, &zSig64); >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0return roundAndPackFloat32(zSign, zExp, zSi= g64 STATUS_VAR); >> + =C2=A0 =C2=A0} >> +} > > 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. =C2=A0But I don't see any actual log= ic 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