From owner-freebsd-numerics@FreeBSD.ORG Wed Jun 19 22:06:40 2013 Return-Path: Delivered-To: freebsd-numerics@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 83D7EC1E for ; Wed, 19 Jun 2013 22:06:40 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 4C2391233 for ; Wed, 19 Jun 2013 22:06:39 +0000 (UTC) Received: from c122-106-156-23.carlnfd1.nsw.optusnet.com.au (c122-106-156-23.carlnfd1.nsw.optusnet.com.au [122.106.156.23]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 155E21041C6F; Thu, 20 Jun 2013 08:06:32 +1000 (EST) Date: Thu, 20 Jun 2013 08:06:31 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eitan Adler Subject: Re: operation precedence bug: lib/msun/src In-Reply-To: Message-ID: <20130620071226.L1067@besplex.bde.org> References: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.0 cv=Q6eKePKa c=1 sm=1 a=JW1MbwKkIIsA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=vqevWRgFEOoA:10 a=4iUgboIlSR6cpcxLKlUA:9 a=CjuIK1q_8ugA:10 a=fxujCl1df8CZ5cHc:21 a=AExpOoHPGUjUhLgS:21 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 Cc: freebsd-numerics@freebsd.org X-BeenThere: freebsd-numerics@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Discussions of high quality implementation of libm functions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Jun 2013 22:06:40 -0000 On Wed, 19 Jun 2013, Eitan Adler wrote: > Does the following look correct? No :-). > Fix operation precedence bug: != comes before ^ so add required parens According to das, the code was correct, though unclear. The change adds unrequired parentheses. Since these are in the wrong place, they change the object code and probably break it. > diff --git a/lib/msun/src/s_fma.c b/lib/msun/src/s_fma.c > index 452bece..406ff47 100644 > --- a/lib/msun/src/s_fma.c > +++ b/lib/msun/src/s_fma.c > @@ -117,7 +117,7 @@ add_and_denormalize(double a, double b, int scale) > if (sum.lo != 0) { > EXTRACT_WORD64(hibits, sum.hi); > bits_lost = -((int)(hibits >> 52) & 0x7ff) - scale + 1; > - if (bits_lost != 1 ^ (int)(hibits & 1)) { > + if (bits_lost != (1 ^ (int)(hibits & 1))) { > /* hibits += (int)copysign(1.0, sum.hi * sum.lo) */ > EXTRACT_WORD64(lobits, sum.lo); > hibits += 1 - (((hibits ^ lobits) >> 62) & 2); > diff --git a/lib/msun/src/s_fmal.c b/lib/msun/src/s_fmal.c > index 9271901..ec4cc4d 100644 > --- a/lib/msun/src/s_fmal.c > +++ b/lib/msun/src/s_fmal.c > @@ -113,7 +113,7 @@ add_and_denormalize(long double a, long double b, int scale) > if (sum.lo != 0) { > u.e = sum.hi; > bits_lost = -u.bits.exp - scale + 1; > - if (bits_lost != 1 ^ (int)(u.bits.manl & 1)) > + if (bits_lost != (1 ^ (int)(u.bits.manl & 1))) > sum.hi = nextafterl(sum.hi, INFINITY * sum.lo); > } > return (ldexp(sum.hi, scale)); I use the following fix (put the parentheses in the right place), but haven't actually tested it: % diff -u2 s_fma.c~ s_fma.c % --- s_fma.c~ Thu May 30 18:15:37 2013 % +++ s_fma.c Thu May 30 18:15:38 2013 % @@ -118,5 +118,5 @@ % EXTRACT_WORD64(hibits, sum.hi); % bits_lost = -((int)(hibits >> 52) & 0x7ff) - scale + 1; % - if (bits_lost != 1 ^ (int)(hibits & 1)) { % + if ((bits_lost != 1) ^ (int)(hibits & 1)) { % /* hibits += (int)copysign(1.0, sum.hi * sum.lo) */ % EXTRACT_WORD64(lobits, sum.lo); % diff -u2 s_fmal.c~ s_fmal.c % --- s_fmal.c~ Thu May 30 18:16:08 2013 % +++ s_fmal.c Thu May 30 18:16:09 2013 % @@ -114,5 +114,5 @@ % u.e = sum.hi; % bits_lost = -u.bits.exp - scale + 1; % - if (bits_lost != 1 ^ (int)(u.bits.manl & 1)) % + if ((bits_lost != 1) ^ (int)(u.bits.manl & 1)) % sum.hi = nextafterl(sum.hi, INFINITY * sum.lo); % } Bruce