Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 May 2015 23:13:33 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Andrew Turner <andrew@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r283694 - head/lib/libc/gen
Message-ID:  <20150529221743.D1965@besplex.bde.org>
In-Reply-To: <201505290926.t4T9QBVE030151@svn.freebsd.org>
References:  <201505290926.t4T9QBVE030151@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 29 May 2015, Andrew Turner wrote:

> Log:
>  Fix __fpclassifyl when double == long double. As with r283693 this is
>  needed on ARM and PowerPC.
>
>  MFC after:	1 Week
>
> Modified:
>  head/lib/libc/gen/fpclassify.c
>
> Modified: head/lib/libc/gen/fpclassify.c
> ==============================================================================
> --- head/lib/libc/gen/fpclassify.c	Fri May 29 09:23:20 2015	(r283693)
> +++ head/lib/libc/gen/fpclassify.c	Fri May 29 09:26:10 2015	(r283694)
> @@ -29,6 +29,8 @@
>
> #include <sys/endian.h>
>
> +#include <machine/float.h>
> +
> #include <math.h>
> #include <stdint.h>
>
> @@ -84,10 +86,18 @@ __fpclassifyl(long double e)
> 		return (FP_SUBNORMAL);
> 	}
> 	mask_nbit_l(u);		/* Mask normalization bit if applicable. */
> +#if LDBL_MANT_DIG == 53
> +	if (u.bits.exp == 2047) {
> +		if ((u.bits.manl | u.bits.manh) == 0)
> +			return (FP_INFINITE);
> +		return (FP_NAN);
> +	}
> +#else
> 	if (u.bits.exp == 32767) {
> 		if ((u.bits.manl | u.bits.manh) == 0)
> 			return (FP_INFINITE);
> 		return (FP_NAN);
> 	}
> +#endif
> 	return (FP_NORMAL);
> }

This can be written without any ifdefs by spelling 2047 and 32767 as
LBDL_MUMBLE.

However, I prefer explicit values, with masks spelled in hex as 0x7fff
or 7ff, not as above or as (2 * LDBL_MAX_EXP - 1).

However2, grepping for the usual spelling of such masks shows that the
above is a wrong fix -- a style bug.  All long double functions where
long double is just double should be weak aliases to the double
functions.  This method is used for all nontrivial functions.

The above code is pessimized to support the intermediate version with
everything looking the same but parametrized by macros and struct
layouts.  bits.exp, bits.manh and bits.manl have all sorts of sizes
so that they look the same, but this gives pessimal code for arches
where the sizes are unnatural and the compiler can't repack to better
ones.

There are no examples of ifdefs for the weak aliases that apply directly
here.  Usually the long double version is in a file by itself and is
ommitted for arches without real long doubles by not putting it in the
MD Makefile.  Here and for isinf*(), all the libc functions are in the
same file, so ifdefs would be needed.  You may also need to keep the
extern __*l() functions for library compatibility.  This is for
functions that are only in libc as 10 year old compatibility hacks
for 20 year old mistakes :-(,

If the above is ifdefed, then it may as well handle the normalization
bit correctly using another ifdef.  There are just 3 supported cases:
- 64-bit long doubles (really doubles).  Normalization is implicit;
   there are no invalid formats and tests can ignore the normalization
   bit
- 80-bit long doubles.  Don't clobber the normalization bit, but
   check that it is consistent
- 128-bit long doubles.  Like 64-bit ones for normalization.

__isnanl() seems to be broken in one more place (unless you fixed
this too).  libm has another extern copy of __isnanl() in s_isnanl.c.
This is compiled in both the PIC and !PIC cases, while the one in libc
is only compiled in the PIC case.  I think this copy is even more
unreachable than the one in libc.

As mentioned in a previous reply, math.h has a copy of __isnanl()
which is the one normally used, and it is missing most of the bugs.

Bruce



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