From owner-freebsd-numerics@FreeBSD.ORG Tue Dec 10 05:26:27 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 2328CFA0 for ; Tue, 10 Dec 2013 05:26:27 +0000 (UTC) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id A9CE7138D for ; Tue, 10 Dec 2013 05:26:26 +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 mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 22DB442E040; Tue, 10 Dec 2013 16:26:16 +1100 (EST) Date: Tue, 10 Dec 2013 16:26:12 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Steve Kargl Subject: Re: dead code in lgamma_r[f]? In-Reply-To: <20131209223720.GA91828@troutmask.apl.washington.edu> Message-ID: <20131210155635.J1022@besplex.bde.org> References: <20131205182324.GA65135@troutmask.apl.washington.edu> <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> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=Kejr72oD c=1 sm=1 tr=0 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 a=PO7r1zJSAAAA:8 a=m8NKqXhiB1EA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=ElNGO9NIuDsA:10 a=XGf6sHeGM9skIQnvz40A:9 a=CjuIK1q_8ugA:10 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 05:26:27 -0000 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. > . 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. 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. > ... > 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. Bruce