From owner-freebsd-current@FreeBSD.ORG Thu Jul 11 17:10:20 2013 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id DD671DEF; Thu, 11 Jul 2013 17:10:20 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id 71BFC1AB4; Thu, 11 Jul 2013 17:10:20 +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 mail108.syd.optusnet.com.au (Postfix) with ESMTPS id EA3171A060B; Fri, 12 Jul 2013 02:38:06 +1000 (EST) Date: Fri, 12 Jul 2013 02:38:05 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: David Chisnall Subject: Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity In-Reply-To: Message-ID: <20130712013458.X85470@besplex.bde.org> References: <20130710155809.0f589c22@thor.walstatt.dyndns.org> <20130710183315.725dfde0@thor.walstatt.dyndns.org> <20130710203200.5359fd18@thor.walstatt.dyndns.org> <51DDC04B.6040209@FreeBSD.org> <20957.49978.73666.392417@khavrinen.csail.mit.edu> <20130711130043.R920@besplex.bde.org> <20130711202908.L84170@besplex.bde.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.0 cv=eqSHVfVX c=1 sm=1 a=GSpoo125mhwA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=mlUDQFikY8EA:10 a=5RSJzs5NDUsr1fZtrzAA:9 a=CjuIK1q_8ugA:10 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 X-Mailman-Approved-At: Thu, 11 Jul 2013 18:39:30 +0000 Cc: FreeBSD CURRENT , Garrett Wollman , "freebsd-toolchain@FreeBSD.org" , Bruce Evans , Tijl Coosemans , "freebsd-standards@FreeBSD.org" X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jul 2013 17:10:21 -0000 On Thu, 11 Jul 2013, David Chisnall wrote: > On 11 Jul 2013, at 13:11, Bruce Evans wrote: > >> The error message for the __builtin_isnan() version is slightly better up >> to where it says more. >> >> The less-unportable macro can do more classification and detect problems >> at compile time using __typeof(). > > The attached patch fixes the related test cases in the libc++ test suite. Please review. OK if the ifdefs work and the style bugs are fixed. > This does not use __builtin_isnan(), but it does: > > - Stop exposing isnan and isinf in the header. We already have __isinf in libc, so this is used instead. > > - Call the static functions for isnan __inline__isnan*() so that they don't conflict with the ones in libm. > > - Add an __fp_type_select() macro that uses either __Generic(), __builtin_choose_expr() / __builtin_choose_expr(), or sizeof() comparisons, depending on what the compiler supports. > > - Refactor all of the type-generic macros to use __fp_type_select(). % Index: src/math.h % =================================================================== % --- src/math.h (revision 253148) % +++ src/math.h (working copy) % @@ -80,28 +80,39 @@ % #define FP_NORMAL 0x04 % #define FP_SUBNORMAL 0x08 % #define FP_ZERO 0x10 % + % +#if __STDC_VERSION__ >= 201112L % +#define __fp_type_select(x, f, d, ld) _Generic((x), \ % + float: f(x), \ % + double: d(x), \ % + long double: ld(x)) The normal formatting of this is unclear. Except for the tab after #define. math.h has only 1 other instance of a space after #define. % +#elif __GNUC_PREREQ__(5, 1) % +#define __fp_type_select(x, f, d, ld) __builtin_choose_expr( \ % + __builtin_types_compatible_p(__typeof (x), long double), ld(x), \ % + __builtin_choose_expr( \ % + __builtin_types_compatible_p(__typeof (x), double), d(x), \ % + __builtin_choose_expr( \ % + __builtin_types_compatible_p(__typeof (x), float), f(x), (void)0))) Extra space after __typeof. Normal formatting doesn't march to the right like this... % +#else % +#define __fp_type_select(x, f, d, ld) \ % + ((sizeof (x) == sizeof (float)) ? f(x) \ % + : (sizeof (x) == sizeof (double)) ? d(x) \ % + : ld(x)) ... or like this. Extra space after sizeof (bug copied from old code). % +#endif % + % + % + Extra blank lines. % #define fpclassify(x) \ % - ((sizeof (x) == sizeof (float)) ? __fpclassifyf(x) \ % - : (sizeof (x) == sizeof (double)) ? __fpclassifyd(x) \ % - : __fpclassifyl(x)) Example of normal style in old code (except for the space after sizeof(), and the backslashes aren't line up like they are in some other places in this file). % ... % @@ -119,10 +130,8 @@ % #define isunordered(x, y) (isnan(x) || isnan(y)) % #endif /* __MATH_BUILTIN_RELOPS */ % % -#define signbit(x) \ % - ((sizeof (x) == sizeof (float)) ? __signbitf(x) \ % - : (sizeof (x) == sizeof (double)) ? __signbit(x) \ % - : __signbitl(x)) % +#define signbit(x) \ % + __fp_type_select(x, __signbitf, __signbit, __signbitl) The tab lossage is especially obvious here. This macro definition fits on 1 line now. Similarly for others except __inline_isnan*, which takes 2 lines. __inline_isnan* should be named less verbosely, without __inline. I think this doesn't cause any significant conflicts with libm. Might need __always_inline. __fp_type_select is also verbose. % % typedef __double_t double_t; % typedef __float_t float_t; % @@ -175,6 +184,7 @@ % int __isfinite(double) __pure2; % int __isfinitel(long double) __pure2; % int __isinff(float) __pure2; % +int __isinf(double) __pure2; % int __isinfl(long double) __pure2; % int __isnanf(float) __pure2; % int __isnanl(long double) __pure2; % @@ -185,6 +195,23 @@ % int __signbitf(float) __pure2; % int __signbitl(long double) __pure2; The declarations of old extern functions can probably be removed too when they are replaced by inlines (only __isnan*() for now) . I think the declarations of __isnan*() are now only used to prevent warnings (at higher warning levels than have ever been used) in the file that implement the functions. % % +static __inline int % +__inline_isnanf(float __x) % +{ % + return (__x != __x); % +} % +static __inline int % +__inline_isnan(double __x) % +{ % + return (__x != __x); % +} % +static __inline int % +__inline_isnanl(long double __x) % +{ % + return (__x != __x); % +} % + % + Extra blank lines. Some insertion sort errors. In this file, APIs are mostly sorted in the order double, float, long double. All the inline functions except __inline_isnan*() only evaluate their args once, so they can be simpler shorter and less namespace-polluting as macros. catrig*.c uses the following macros for them (just showing the double precision ones here): @ #define isinf(x) (fabs(x) == INFINITY) @ #define isnan(x) ((x) != (x)) @ #define signbit(x) (__builtin_signbit(x)) Note that that these are not quite independent of the precision, so they don't all need multiple inline functions or macros. fabs() is not type-generic, but __builtin_fabs() might be. fabsl() for all precisions would work but be slower. __builtin_signbit() is even more likely to be type-generic, like __builtin_isnan(). But catrig[fl].c spell out the type suffixes for safety. I hoped to copy these to math.h after fixing any technical problems. isnan() needs a statement-expression to be safe. signbit() is more of the problem than the others, since it is hard to do without a builtin and the builtin doesn't exist in gcc-3.x. The builtin works fine with -current gcc and clang, at least on x86 . So the committed version just uses the builtin, and my local version uses libm internals that are unsuitable for the public signbit() (mainly due to namespace problems). The builtin might not work right in -curent for non-x86. When the extern function is used, it uses similar libm internals, but they take much longer when not inlined. Bruce