Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 5 Feb 2015 17:08:03 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Steve Kargl <sgk@troutmask.apl.washington.edu>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, "Pedro F. Giffuni" <pfg@freebsd.org>, src-committers@freebsd.org, Bruce Evans <brde@optusnet.com.au>
Subject:   Re: svn commit: r278154 - head/lib/msun/src
Message-ID:  <20150205164453.B1011@besplex.bde.org>
In-Reply-To: <20150203191355.GA13447@troutmask.apl.washington.edu>
References:  <201502031417.t13EHU9g074687@svn.freebsd.org> <20150204012614.T1972@besplex.bde.org> <20150203163329.GA12706@troutmask.apl.washington.edu> <20150204035337.Y2718@besplex.bde.org> <20150203191355.GA13447@troutmask.apl.washington.edu>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 3 Feb 2015, Steve Kargl wrote:

> On Wed, Feb 04, 2015 at 04:37:14AM +1100, Bruce Evans wrote:
>> On Tue, 3 Feb 2015, Steve Kargl wrote:
>>
>>> -#include <limits.h>
>>> +#include <float.h>
>>> #include <math.h>
>>>
>>> +#define	FLT_LARGE	FLT_MAX_EXP - FLT_MIN_EXP + FLT_MANT_DIG
>>> +...
>>
>> This is a lot of code and bugs (missing parentheses) for negative gain.
>
> My crystal ball isn't function too well lately.

I thought I mentioned this in my first reply.  (I didn't write lines
like the above because they are too verbose, and I didn't write down
hard limits because I didn't have time to choose them carefully.)

> Given that the widest field width in IEEE754-2008 for
> binary128 is 15, which gives a maximum unbiased exponent
> of 2**15, then using +-65536 should cover our needs.  The
> functions scalbn[fl] can then deal with values of n
> that may result in exponents that are too small or too
> large.  Untested diff below.

Doesn't IEEE allow extensions?  More than 15 is just not currently
needed, and also not likely to be common soon.

I agree that it is enough.  Elwhere, we have many checks like
LDBL_MAX_EXP == 0x4000 since we only support long doubles if they are
either mere doubles or have a 15-exponent with the usual encoding.
So anyone enlarging the exponent long doubles would have a lot of
work to do and shouldn't need to be reminded to do it here too.

> Index: s_scalbln.c
> ===================================================================
> --- s_scalbln.c	(revision 278165)
> +++ s_scalbln.c	(working copy)
> @@ -27,17 +27,17 @@
> #include <sys/cdefs.h>
> __FBSDID("$FreeBSD$");
>
> -#include <limits.h>
> #include <math.h>
>
> +#define	LARGE	65536
> +#define	SMALL	-65536
> +
> double
> scalbln (double x, long n)
> {
> 	int in;
>
> -	in = (int)n;
> -	if (in != n)
> -		in = (n > 0) ? INT_MAX: INT_MIN;
> +	in = n > LARGE ? LARGE : n < SMALL ? SMALL : n;
> 	return (scalbn(x, in));
> }
>

You also removed extra parentheses around the comparison operator.  I put
them in since I'm trying to sell conditional operators but some people
find conditional operators more confusing without extra parentheses.
The extra parentheses around the inner comparisons are possibly the
ones least needed for technical reasons, but seem to improve readability.

After shortening it with shorter limit variable names and no extra
parentheses, it fits in 1 line:

double
scalbln (double x, long n)
{

 	return (scalbn(x, n > LARGE ? LARGE : n < SMALL ? SMALL : n);
}

Not using the temporary variable makes it more readable IMO.

I would also drop the strict KNF of having an empty line after the
null local declarations, and restore the cast on the final n.  In
previous versions, there was an explicit conversion of the expression's
value from long to int by assignment to 'in'.  Now there is only an
implicit conversion by the prototype.  Casting the final n would make
the expression have type int.

Also fix the gnu-style parentheses in 'scalbln ('.  This bug apparently
came from fdlibm for scalbn().  It has been cloned to several other
scal*'s but isn't in all, even for fdlibm.

Bruce



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