From owner-svn-src-head@FreeBSD.ORG Fri May 29 12:13:34 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id AC1D15BB; Fri, 29 May 2015 12:13:34 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 542761CB6; Fri, 29 May 2015 12:13:34 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id EAA681041DF8; Fri, 29 May 2015 22:13:21 +1000 (AEST) Date: Fri, 29 May 2015 22:13:19 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrew Turner cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r283693 - head/lib/libc/gen In-Reply-To: <201505290923.t4T9NLrf029425@svn.freebsd.org> Message-ID: <20150529204844.H1965@besplex.bde.org> References: <201505290923.t4T9NLrf029425@svn.freebsd.org> 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=L/MkHYj8 c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=StFR8Df7wEdXKxfvVd0A:9 a=FBKdIa_ITWnxkK1Z:21 a=ckpE6Dq_-Ae_lcXi:21 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 May 2015 12:13:34 -0000 On Fri, 29 May 2015, Andrew Turner wrote: > Log: > Fix __isinfl on architectures where double == long double. This is the > case on at least ARM and PowerPC. Urk. I rarely look at the parts of libm misplaced outside of libc. > Modified: head/lib/libc/gen/isinf.c > ============================================================================== > --- head/lib/libc/gen/isinf.c Fri May 29 09:17:59 2015 (r283692) > +++ head/lib/libc/gen/isinf.c Fri May 29 09:23:20 2015 (r283693) > @@ -26,6 +26,8 @@ > * $FreeBSD$ > */ > > +#include > + > #include > > #include "fpmath.h" > @@ -62,9 +64,9 @@ __isinfl(long double e) > > u.e = e; > mask_nbit_l(u); > -#ifndef __alpha__ > - return (u.bits.exp == 32767 && u.bits.manl == 0 && u.bits.manh == 0); > -#else > +#if LDBL_MANT_DIG == 53 > return (u.bits.exp == 2047 && u.bits.manl == 0 && u.bits.manh == 0); > +#else > + return (u.bits.exp == 32767 && u.bits.manl == 0 && u.bits.manh == 0); > #endif > } This is still quite bad (slow and unportable) code. It should be rarely executed, because is isinf() should be inlined (isinf() is a type-generic macro and the specific float, double and long double versions are only directly accessible as implementation details). clang inlines it well. However, isinf() is poorly implemented as a selection of one of the slow extern versions in libc, so the compiler can never see it to optimize it except using hacks like '#undef isinf' -- the compiler can only see the calls to the slow extern versions that it doesn't understand. This is hard to implement better. Translating isinf() to __builtin_isinf() is no good since __builtin_isinf() doesn't exist for some compilers and is not inlined by other compilers. Also, most uses of mask_nbit_l() are wrong, and the one above is no exception. In the above, masking out the normalization bit prevents detection of +-pseudo-Inf in ld80 format (same bits as +-Inf but normalization bit clear). IIRC, +-pseudo-Inf is treated as a signaling NaN by the hardware. The best implementation of isinf(x) seems to be (fabs(x) == INFINITY), with certain complications. clang's ininling gives exactly this on both amd64 and i386 IIRC. Complications: - INFINITY is float precision, but that is sufficiently type-generic - fabs() is unfortunately not type-generic, so selection of fabsf()/fabs()/fabsl() is needed. fabs*() doesn't suffer from the same inlining problems as isinf() so it gets inlined properly - perhaps fabs(x) can be avoided using (x) == -INFINITY || (x) == INFINITY. This requires more care to avoid multiple evaluation of (x) and a smarter compiler to convert it to fabs(x). A smart compiler is also needed to do the reverse conversion if fabs(x) is slow - problems with signaling NaNs. Classification should work without generating an exception for signaling NaNs. They sometimes do, and the code that looks at their bits as in the libc version has the best chance of avoiding exception. clang's inlining and the direct comparison may be invalid since they always give the exception. However, just loading a value or passing it to a function may raise an exception. This is very MD. On i386, loading a signaling-NaN float or double raises an exception, but loading a long double one doesn't; passing a signaling value to a function tends to raise an exception give a signal for floats and doubles by loading them to copy them, but copying through integer registers doesn't. On amd64, loading and parameter passing in the usual ways never raises an exception. It is easiest to ignore this problem and let signaling NaNs raise an exception in undocumented circumstances. Code that cares about efficiency, like most of libm, never uses isinf() or other classification functions because the functions are or might be too slow. libm mostly avoids isinf() by looking at bits, just like this libc code. Looking at bits tends to be slow for long doubles and also slow for doubles on 32-bit arches, but is the best possible method when there is more than one thing to classify. The extern versions of isinf() and classification macros lose mainly by being extern, so that if they are used then the bits would have to be extracted many times per function. Otherwise, libm uses: - in cproj*(), csqrt*(), and ctanh*() just the slow standard macro. These functions wouldn't have passed an adequate review. - in catrig*.c, a home made isinf(x) defined as fabs*(x) == INFINITY. isnan() is less important than isinf(), but has similar problems. Just about the same ones, except the one fixed in this commit. Somehow isnan() never had a libc version for isnanl(). The current mess for isnan() in libc is: - no isnan*() at all in libc in the !PIC case. This works because the mess in libm and math.h is smaller for isnan*() (see below) - __isnan() and __isnanf() in libc for the PIC case. The source code has a ~10 year old comment saying that they are only there until we can bump libm's major version number. This number has been bumped a few times. Symbol versioning might make this a non-problem too. Another part of the comment suggests that it is only a hack to not provide these functions in the !PIC case (to avoid linkage problems). I use old libc.a that still has these functions and see the linkage problems, and hack around them in libm instead. The current mess for isnan() in libm is: - isnan(x) is not so poorly implemented as a selection of one of the static inline versions in math.h. These use the simplistic (x) != (x) implementation. I now seem to remember that this is due to the compatibility problems from conflicts with historical mistakes in libc was even larger than for isinf(), and we use these static inline versions because this avoids all linkage problems for fresh compiles and the implementation is good enough. Home made versions of isnan() are not as easy to write as ones for isinf(), because the portability of (x) != (x) is unclear. This expression the same unwanted signals for signaling NaNs as (x) == INFINITY. I think that is the only problem with IEEE NaNs. Code that checks for NaNs is just more likely to care about the signaling case than code that checks for Infinities. Bruce