From owner-freebsd-toolchain@FreeBSD.ORG Thu Jul 11 12:11:08 2013 Return-Path: Delivered-To: freebsd-toolchain@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 EB7E34C5; Thu, 11 Jul 2013 12:11:08 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 35E6718C5; Thu, 11 Jul 2013 12:11:07 +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 mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 90CC93C2C4C; Thu, 11 Jul 2013 22:11:01 +1000 (EST) Date: Thu, 11 Jul 2013 22:11:00 +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: <20130711202908.L84170@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> 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=K8x6hFqI c=1 sm=1 a=GSpoo125mhwA:10 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=JzwRw_2MAAAA:8 a=mlUDQFikY8EA:10 a=AJ7DxV0eBxafINBosPIA:9 a=CjuIK1q_8ugA:10 a=ebeQFi2P/qHVC0Yw9JDJ4g==:117 Cc: freebsd-toolchain@FreeBSD.org, Garrett Wollman , FreeBSD CURRENT , Bruce Evans , Tijl Coosemans , freebsd-standards@FreeBSD.org X-BeenThere: freebsd-toolchain@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Maintenance of FreeBSD's integrated toolchain List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Jul 2013 12:11:09 -0000 On Thu, 11 Jul 2013, David Chisnall wrote: > You're joining in this discussion starting in the middle, so you probably missed the earlier explanation. I was mainly addressing a C99 point. I know little about C++ or C11. > On 11 Jul 2013, at 05:21, Bruce Evans wrote: > >> I don't see how any conforming program can access the isnan() function >> directly. It is just as protected as __isnan() would be. (isnan)() >> gives the function (the function prototype uses this), but conforming >> programs can't do that since the function might not exist. Maybe some >> non-conforming program like autoconfig reads or libm.a and >> creates a bug for C++. > > The cmath header defines a template function isnan that invokes the isnan macro, but then undefines the isnan macro. This causes a problem because when someone does something along the lines of using namespace std then they end up with two functions called isnan and the compiler gets to pick the one to use. Unfortunately, std::isnan() returns a bool, whereas isnan() returns an int. > > The C++ headers are not required to be conforming C code, because they are not C, and our math.h causes namespace pollution in C++ when included from . is also not required to be conforming C code, let alone C++ code, so there is only a practical requirement that it works when included in the C++ implementation. >> The FreeBSD isnan() implementation would be broken by removing the >> isnan() function from libm.a or ifdefing it in . Changing the >> function to __isnan() would cause compatibility problems. The function >> is intentionally named isnan() to reduce compatibility problems. > > On OS X this is avoided because their isnan() macro expands to call one of the __-prefixed inline functions (which adopt your suggestion of being implemented as x != x, for all types). I am not sure that this is required for standards conformance, but it is certainly cleaner. Your statement that having the function not called isnan() causes compatibility problems is demonstrably false, as neither OS X nor glibc has a function called isnan() and, unlike us, they do not experience problems with this macro. The compatibility that I'm talking about is with old versions of FreeBSD. isnan() is still in libc as a function since that was part of the FreeBSD ABI and too many things depended on getting it from there. It was recently removed from libc.so, but is still in libm.a. This causes some implementation problems in libm that are still not completely solved. I keep having to edit msun/src/s_isnan.c the msun sources are more portable. Mostly I need to kill the isnan() there so that it doesn't get in the way of the one in libc. This mostly works even if there is none in libc, since the builtins result in neither being used. isnanf() is more of a problem, since it is mapped to __isnanf() and there is no builtin for __isnanf(). The old functions have actually been removed from libc.a too. They only in libc_pic.a. libc.a still has isnan.o, but that is bogus since isnan.o is now empty. > It would also be nice to implement these macros using _Generic when compiling in C11 mode, as it will allow the compiler to produce more helpful warning messages. I would propose this implementation: > #if __has_builtin(__builtin_isnan) This won't work for me, since I develop and test msun with old compilers that don't support __has_builtin(). Much the same set of compilers also don't have enough FP builtins. It also doesn't even work. clang has squillions of builtins that aren't really builtines so they reduce to libcalls. gcc has fewer builtins, but still many that reduce to libcalls. An example is fma(). __has_builtin(__builtin_fma) is true for clang on amd64 (freefall), but at least freefalls's CPU doesn't support fma in hardware, so the builtin can't really work, and in fact it doesn't -- it reduces to a libcall. This might change if the hardware supports fma, but then __has_builtin(__builtin_fma) would be even more useless for telling if fma is worth using. C99 has macros FP_FAST_FMA[FL] whose implementation makes them almost equally useless. For example, ia64 has fma in hardware and the implementation defines all of FP_FAST_FMA[FL] for ia64. But fma is implemented as an extern function, partly because there is no way to tell if __builtin_fma is any good (but IIRC, __builtin_fma is no good on ia64 either, since it reduces to the same extern function). The extern function is slow (something like 20 cycles instead of 1 for the fma operation). But if you ignore the existence of the C99 fma API and just write expressions of the form (a*x + b), then gcc on ia64 will automatically use the hardware fma, although this is technically wrong in some fenv environments. For gcc-4.2.1, __has_builtin(__builtin_fma) is a syntax error. I test with gcc-3.x. It is also missing __builtin_isnan(). The msun implementation knows that isnan() and other classification macros are too slow to actually use, and rarely uses them. Sometimes it is inherent that it can do better, because it already knows the bits in the macros and can test the bits directly. I have had limited tests with arranging access macros so that loads of the bits can be shared. > #define isnan(x) __builtin_isnan(x) > #else > static __inline int > __isnanf(float __x) > { > return (__x != __x); > } Here we can do better in most cases by hard-coding this without the ifdef. > static __inline int > __isnand(double __x) > { > return (__x != __x); > } __isnand() is a strange name, and doesn't match compiler conventions for builtins. Compilers use __builtin_isnan() and map this to the libcall __isnan(). > static __inline int > __isnanl(long double __x) > { > return (__x != __x); > } > #if __STDC_VERSION__ >= 201112L > #define isnan(x) _Generic((x), \ > float: __isnanf(x), \ > double: __isnand(x), \ > long double: __isnanl(x)) Does _Generic() have no side effects, like sizeof()? > #else > #define isnan(x) \ > ((sizeof (x) == sizeof (float)) ? __isnanf(x) \ > : (sizeof (x) == sizeof (double)) ? __isnand(x) \ > : __isnanl(x)) > #endif > #endif Both cases need to use __builtin_isnan[fl]() and depend on compiler magic to have any chance of avoiding side effects from loads and parameter passing. Generic stuff doesn't seem to work right for either isnan() or __builtin_isnan(), though it could for at least the latter. According to a quick grep of strings $(which clang), __builtin_classify() is generic but __builtin_isnan*() isn't (the former has no type suffixes but the latter does, and testing shows that the latter doesn't work without the suffices). It is the most useful FP builtins like __builtin_fabs*() that aren't generic. C99 probably prevents fabs*() being generic, but I think any builtin can be generic, and the isnan() macro is specified to be generic, so if compilers understood it directly their builtin for it needs to be and can be generic. > For a trivial example of why this is an improvement in terms of error reporting, consider this trivial piece of code: > > int is(int x) > { > return isnan(x); > } > > With our current implementation, this compiles and links without any warnings, although it will have somewhat interesting results at run time. With the __builtin_isnan() version, clang reports this error: > > isnan.c:35:15: error: floating point classification requires argument of > floating point type (passed in 'int') > return isnan(x); > ^ > (and then some more about the macro expansion) > > With the C11 version, it reports this error: > > isnan.c:35:15: error: controlling expression type 'int' not compatible with any > generic association type > return isnan(x); > ^ 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(). isnan(integer) = 0 with no warnings is quite good though. Integers are never NaNs, and converting an integer to a floating point type should never give a NaN. Bruce