From owner-freebsd-numerics@FreeBSD.ORG Tue Dec 10 07:38:41 2013 Return-Path: Delivered-To: freebsd-numerics@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B4443905 for ; Tue, 10 Dec 2013 07:38:41 +0000 (UTC) Received: from troutmask.apl.washington.edu (troutmask.apl.washington.edu [128.95.76.21]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 885B61CF1 for ; Tue, 10 Dec 2013 07:38:41 +0000 (UTC) Received: from troutmask.apl.washington.edu (localhost.apl.washington.edu [127.0.0.1]) by troutmask.apl.washington.edu (8.14.7/8.14.7) with ESMTP id rBA7ceQZ094134; Mon, 9 Dec 2013 23:38:40 -0800 (PST) (envelope-from sgk@troutmask.apl.washington.edu) Received: (from sgk@localhost) by troutmask.apl.washington.edu (8.14.7/8.14.7/Submit) id rBA7ceaZ094133; Mon, 9 Dec 2013 23:38:40 -0800 (PST) (envelope-from sgk) Date: Mon, 9 Dec 2013 23:38:40 -0800 From: Steve Kargl To: Bruce Evans Subject: Re: dead code in lgamma_r[f]? Message-ID: <20131210073840.GA94002@troutmask.apl.washington.edu> References: <20131205195225.GA65732@troutmask.apl.washington.edu> <20131206102724.W1033@besplex.bde.org> <20131206114426.I1329@besplex.bde.org> <20131207064602.GA76042@troutmask.apl.washington.edu> <20131208012939.GA80145@troutmask.apl.washington.edu> <20131208141627.F1181@besplex.bde.org> <20131208183339.GA83010@troutmask.apl.washington.edu> <20131208185941.GA83484@troutmask.apl.washington.edu> <20131209223720.GA91828@troutmask.apl.washington.edu> <20131210155635.J1022@besplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20131210155635.J1022@besplex.bde.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-numerics@freebsd.org, Filipe Maia X-BeenThere: freebsd-numerics@freebsd.org X-Mailman-Version: 2.1.17 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: Tue, 10 Dec 2013 07:38:41 -0000 On Tue, Dec 10, 2013 at 04:26:12PM +1100, Bruce Evans wrote: > On Mon, 9 Dec 2013, Steve Kargl wrote: > > > I would like to commit the attached patch. I will do it in > > 3 passes: > > 1) whitespace fixes, > > 2) literal constants changes (checked by md5), > > 3) the code re-organization, threshold change, and dead code removed. > > > > * lib/msun/src/e_lgamma_r.c > > . Remove trailing space and trailing blank line in Copyright. > > This undoes rev.1.8, in which previous trailing whitespace removal in > 1.2 was backed out to reduce diffs against the vendor version. It reduces the diff between e_lgamma_r.c and e_lgammaf.c. In addition, I think that at this point in time, the original vendor is irrelevant. >From what I can glean from OpenBSD, NetBSD, Openlibm, and dragonflybsd, FreeBSD is the vendor. > > . Fix prototype for sin_pi to agree with the intended commit > > r97413 done some 11 years 6 month ago. > > . Remove dead code in sin_pi and remove 'if(ix<0x43300000)' as it is > > always true. > > . In the domain [0,2), move the three minimax approximations embedded > > within __ieee75_lgamma_r() into three 'static inline double' functions. > > This is too invasive. It is only a style change to a slighly worse style. It's only a style change to a much better style in that I now have some idea of what to do get working ld80 and ld128 versions. > > The other (unrelated) changes seem reasonable. > > > --- /usr/src/lib/msun/src/e_lgamma_r.c 2013-12-06 07:58:39.000000000 -0800 > > +++ src/e_lgamma_r.c 2013-12-09 13:05:22.000000000 -0800 > > ... > > double > > __ieee754_lgamma_r(double x, int *signgamp) > > { > > - double t,y,z,nadj,p,p1,p2,p3,q,r,w; > > + double t,y,z,nadj,p,q,r,w; > > int32_t hx; > > - int i,lx,ix; > > + int i,ix,lx; > > Also avoid fixing sorting errors when not changing much. It would be a > more substantive change to fix the types of these variables (they should > be uint32_t or int32_t). I should have done more or less in rev.1.9 > where I split up this declaration to change only hx from int to int32_t. > > > > > EXTRACT_WORDS(hx,lx,x); > > > > @@ -235,51 +265,36 @@ > > if((((ix-0x3ff00000)|lx)==0)||(((ix-0x40000000)|lx)==0)) r = 0; > > /* for x < 2.0 */ > > else if(ix<0x40000000) { > > - if(ix<=0x3feccccc) { /* lgamma(x) = lgamma(x+1)-log(x) */ > > + if (ix <= 0X3FECCCCC) { /* lgamma(x) = lgamma(x+1)-log(x) */ > > Like whitespace fixes. > > > r = -__ieee754_log(x); > > - if(ix>=0x3FE76944) {y = one-x; i= 0;} > > - else if(ix>=0x3FCDA661) {y= x-(tc-one); i=1;} > > - else {y = x; i=2;} > > + if (ix >= 0x3FE76944) > > + r += func0(1 - x); > > + else if (ix >= 0x3FCDA661) > > + r += func1(x - (tc - 1)); > > + else > > + r += func2(x); > > The simplification from using func*() seems to be mainly avoiding a case > statement and the case selector variable i. It also allows one to clearly see that here func[0-2] are evaluations of lgamma(1+x) where subdomains in [0,2) are being mapped into other domains. > > ... > > switch(i) { > > - case 7: z *= (y+6.0); /* FALLTHRU */ > > - case 6: z *= (y+5.0); /* FALLTHRU */ > > - case 5: z *= (y+4.0); /* FALLTHRU */ > > - case 4: z *= (y+3.0); /* FALLTHRU */ > > - case 3: z *= (y+2.0); /* FALLTHRU */ > > + case 7: z *= (y+6); /* FALLTHRU */ > > + case 6: z *= (y+5); /* FALLTHRU */ > > + case 5: z *= (y+4); /* FALLTHRU */ > > + case 4: z *= (y+3); /* FALLTHRU */ > > + case 3: z *= (y+2); /* FALLTHRU */ > > The comment indentation would be wrong now, except this file already uses > totally inconsistent comment indentation. The float version blindly moved > the indentation in the other direction by adding float casts. The change reduces the diff between float and double versions. -- Steve