Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 14 Sep 2014 17:10:59 -0700
From:      Steve Kargl <sgk@troutmask.apl.washington.edu>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        freebsd-numerics@freebsd.org
Subject:   Re: lgammal[_r] patch
Message-ID:  <20140915001058.GA7352@troutmask.apl.washington.edu>
In-Reply-To: <20140915072530.T6209@besplex.bde.org>
References:  <20140913193634.GA2323@troutmask.apl.washington.edu> <20140915072530.T6209@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Sep 15, 2014 at 07:36:49AM +1000, Bruce Evans wrote:
> On Sat, 13 Sep 2014, Steve Kargl wrote:
> 
> > Following my .sig is a patch that contains the ld80 and ld128
> > implementations of lgamma[_r]().  It also contains an update
> > to lgammaf_r() and lgamma_r().  Specifically, it has
> 
> Looks good.  It passed all my tests so far.  I haven't tested ld128
> or compared ld80 with more than d64.

I tested ld128 on flame, but it is far from exhaustive (during
development, 10000, 20000, or 1000000 values in an interval. :-)

> You even matched the fdlibm style bugs almost perfectly :-).  Do you
> know agree that copying a reasonable method is easiest?
 
It was painful.  I tend to now try to conform to style(9). But,
having contribute to gfortran for years, I got use to GNU style.
fdlibm style is just different.

> > Index: ld80/e_lgammal_r.c
> > ===================================================================
> > --- ld80/e_lgammal_r.c	(revision 0)
> > +++ ld80/e_lgammal_r.c	(working copy)
> > @@ -0,0 +1,343 @@
> > +/* @(#)e_lgamma_r.c 1.3 95/01/18 */
> > +/*
> > + * ====================================================
> > + * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
> > + *
> > + * Developed at SunSoft, a Sun Microsystems, Inc. business.
> > + * Permission to use, copy, modify, and distribute this
> > + * software is freely granted, provided that this notice
> > + * is preserved.
> > + * ====================================================
> > + */
> > +
> > +#include <sys/cdefs.h>
> > +__FBSDID("$FreeBSD");
> > +/*
> > + * See s_lgamma_r.c for complete comments.
> > + *
> > + * Converted to long double by Steven G. Kargl.
> > + */
> > +#include <float.h>
> > +#ifdef __i386__
> > +#include <ieeefp.h>
> > +#endif
> 
> Missing blank line before the <float.h> include here and in most other
> places.

OK.  I'll fix this before I commit.

> > Index: src/math.h
> > ===================================================================
> > --- src/math.h	(revision 271479)
> > +++ src/math.h	(working copy)
> > @@ -496,8 +496,12 @@
> > long double	tanl(long double);
> > long double	tgammal(long double);
> > long double	truncl(long double);
> > +#endif /* __ISO_C_VISIBLE >= 1999 */
> >
> > -#endif /* __ISO_C_VISIBLE >= 1999 */
> > +#if __BSD_VISIBLE
> > +long double	lgammal_r(long double, int *);
> > +#endif	/* __BSD_VISIBLE */
> 
> This comment has extra indentation and is not needed.  Only larger
> sections or more complicated conditionals need a comment on #endif.
> The rest of the file mostly follows this rule.  One large section
> is missing a comment.
> 
> The BSD extras aren't very well sorted.  Neither are the float and
> double sections.  It was only useful to keep the float and double
> functions in separate sections in 1990 when they weren't standard.

OK. I'll delete the comment.  Note, this section will grow when
sincosl, j0l, j1l, etc are added.  I'm inclined to coalesce
and sort the various __BSD_VISIBLE sections into one.

-- 
Steve



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140915001058.GA7352>