Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-29 Thread David Chisnall
On 28 Jul 2013, at 22:27, Raphael Kubo da Costa rak...@freebsd.org wrote:

 This seems to have been committed in r253321, and broke some code that
 was working with r253320; namely, some code in x11/kde4-workspace
 includes math.h and calls isnan() with a const double.

Please provide a test case.  Specifically, I need to know what language dialect 
this is using, because I have tested including math.h and calling isnan(double) 
with c89, gnu89, c99, c11, c++03 and c++11 on gcc (for the modes that it 
supports) and clang.

David

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-29 Thread Raphael Kubo da Costa
David Chisnall thera...@freebsd.org writes:

 On 28 Jul 2013, at 22:27, Raphael Kubo da Costa rak...@freebsd.org wrote:

 This seems to have been committed in r253321, and broke some code that
 was working with r253320; namely, some code in x11/kde4-workspace
 includes math.h and calls isnan() with a const double.

 Please provide a test case.  Specifically, I need to know what
 language dialect this is using, because I have tested including math.h
 and calling isnan(double) with c89, gnu89, c99, c11, c++03 and c++11
 on gcc (for the modes that it supports) and clang.

I get the following results with and without -std=c++11 and/or
-stdlib=libc++:

% cat isnan.cc
#include math.h
int main() {
  const double d = 42.0;
  return isnan(d) ? 1 : 0;
}

% clang++ isnan.cc
isnan.cc:4:10: error: controlling expression type 'const double' not
compatible with any generic association type
  return isnan(d) ? 1 : 0;
 ^~~~
/usr/include/math.h:109:2: note: expanded from macro 'isnan'
__fp_type_select(x, __inline_isnanf, __inline_isnan,
__inline_isnanl)
^
/usr/include/math.h:86:49: note: expanded from macro '__fp_type_select'
#define __fp_type_select(x, f, d, ld) _Generic((0,(x)),
\
^
1 error generated.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-29 Thread Pasi Parviainen

On 29.7.2013 14:27, Raphael Kubo da Costa wrote:

David Chisnall thera...@freebsd.org writes:


On 28 Jul 2013, at 22:27, Raphael Kubo da Costa rak...@freebsd.org wrote:


This seems to have been committed in r253321, and broke some code that
was working with r253320; namely, some code in x11/kde4-workspace
includes math.h and calls isnan() with a const double.


Please provide a test case.  Specifically, I need to know what
language dialect this is using, because I have tested including math.h
and calling isnan(double) with c89, gnu89, c99, c11, c++03 and c++11
on gcc (for the modes that it supports) and clang.


I get the following results with and without -std=c++11 and/or
-stdlib=libc++:

% cat isnan.cc
#include math.h
int main() {
   const double d = 42.0;
   return isnan(d) ? 1 : 0;
}

% clang++ isnan.cc
isnan.cc:4:10: error: controlling expression type 'const double' not
compatible with any generic association type
   return isnan(d) ? 1 : 0;
  ^~~~
/usr/include/math.h:109:2: note: expanded from macro 'isnan'
 __fp_type_select(x, __inline_isnanf, __inline_isnan,
 __inline_isnanl)
 ^
/usr/include/math.h:86:49: note: expanded from macro '__fp_type_select'
#define __fp_type_select(x, f, d, ld) _Generic((0,(x)),
\
 ^
1 error generated.



In the first place C++ code should be using cmath instead of math.h. 
That said, this is a result of just an another wonderful little 
difference between C and C++. C standard explicitly states that comma 
operator does not yield an lvalue but in C++ it can if the right most 
operand is an lvalue. If C++ compatibility is desired then unary + 
operator (e.g. _Generic(+(x),...) could be used for the same effect, 
only downside with this is that the integer arguments are promoted, but 
that doesn't matter in this case.


Pasi
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-29 Thread Raphael Kubo da Costa
Raphael Kubo da Costa rak...@freebsd.org writes:

 This seems to have been committed in r253321, and broke some code that
 was working with r253320; namely, some code in x11/kde4-workspace
 includes math.h and calls isnan() with a const double.

For posterity, David has reverted the code back to the previous
behaviour in r253766.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-28 Thread Raphael Kubo da Costa
Pasi Parviainen pasi.parviai...@iki.fi writes:

 On 13.7.2013 13:12, David Chisnall wrote:
 On 12 Jul 2013, at 22:47, O. Hartmann ohart...@zedat.fu-berlin.de wrote:

 Obviously not really fixed, but even worse:

 if I use in C code (C99, using clang 3.3 on FreeBSD 10.0-CURRENT/amd64
 revision 253287) isnan(x) where x is a const double, I receive now
 the following error (which doesn't appear on previous versions):

 Thanks.  This is now fixed, however the _Generic() usage that we had there 
 is also present in tgmath.h, and so this file will also need to be fixed in 
 the same way.

 I've now tested the macros with clang/c99, clang/c11, clang/c++98 and 
 clang/c++11, and gcc/c89 and they all seem to work for unqualified, const, 
 volatile, and const-volatile qualified types.

 I've added Ed to the cc: list, as he wrote this code in tgmath.h.

 David


 Instead of listing all possible type qualifier combinations (like in
 r253319), how about using a comma operator to strip away type
 qualifiers? Since only the result type of the expression matters and
 it isn't evaluated at all.

 like:

 #define __fp_type_select(x, f, d, ld) _Generic((0,(x)),

This seems to have been committed in r253321, and broke some code that
was working with r253320; namely, some code in x11/kde4-workspace
includes math.h and calls isnan() with a const double.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-13 Thread David Chisnall
On 12 Jul 2013, at 22:47, O. Hartmann ohart...@zedat.fu-berlin.de wrote:

 Obviously not really fixed, but even worse:
 
 if I use in C code (C99, using clang 3.3 on FreeBSD 10.0-CURRENT/amd64
 revision 253287) isnan(x) where x is a const double, I receive now
 the following error (which doesn't appear on previous versions):

Thanks.  This is now fixed, however the _Generic() usage that we had there is 
also present in tgmath.h, and so this file will also need to be fixed in the 
same way.

I've now tested the macros with clang/c99, clang/c11, clang/c++98 and 
clang/c++11, and gcc/c89 and they all seem to work for unqualified, const, 
volatile, and const-volatile qualified types.

I've added Ed to the cc: list, as he wrote this code in tgmath.h.

David



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-13 Thread Pasi Parviainen

On 13.7.2013 13:12, David Chisnall wrote:

On 12 Jul 2013, at 22:47, O. Hartmann ohart...@zedat.fu-berlin.de wrote:


Obviously not really fixed, but even worse:

if I use in C code (C99, using clang 3.3 on FreeBSD 10.0-CURRENT/amd64
revision 253287) isnan(x) where x is a const double, I receive now
the following error (which doesn't appear on previous versions):


Thanks.  This is now fixed, however the _Generic() usage that we had there is 
also present in tgmath.h, and so this file will also need to be fixed in the 
same way.

I've now tested the macros with clang/c99, clang/c11, clang/c++98 and 
clang/c++11, and gcc/c89 and they all seem to work for unqualified, const, 
volatile, and const-volatile qualified types.

I've added Ed to the cc: list, as he wrote this code in tgmath.h.

David



Instead of listing all possible type qualifier combinations (like in 
r253319), how about using a comma operator to strip away type 
qualifiers? Since only the result type of the expression matters and it 
isn't evaluated at all.


like:

#define __fp_type_select(x, f, d, ld) _Generic((0,(x)),

Pasi
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-12 Thread Scot Hetzel
On Thu, Jul 11, 2013 at 9:33 AM, David Chisnall thera...@freebsd.org wrote:
 On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au 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.


 #definefpclassify(x) \
-((sizeof (x) == sizeof (float)) ? __fpclassifyf(x) \
-: (sizeof (x) == sizeof (double)) ? __fpclassifyd(x) \
-: __fpclassifyl(x))
+__fp_type_select(x, __fpclassifyf, __fpclassifyd, __fpclassifyd)

The last __fpclassifyd should be __fpclassifyl.

-- 
DISCLAIMER:

No electrons were maimed while sending this message. Only slightly bruised.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-12 Thread Scot Hetzel
On Fri, Jul 12, 2013 at 11:16 AM, Scot Hetzel swhet...@gmail.com wrote:
 On Thu, Jul 11, 2013 at 9:33 AM, David Chisnall thera...@freebsd.org wrote:
 On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au 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.


  #definefpclassify(x) \
 -((sizeof (x) == sizeof (float)) ? __fpclassifyf(x) \
 -: (sizeof (x) == sizeof (double)) ? __fpclassifyd(x) \
 -: __fpclassifyl(x))
 +__fp_type_select(x, __fpclassifyf, __fpclassifyd, __fpclassifyd)

 The last __fpclassifyd should be __fpclassifyl.

I see it has already been fixed.


-- 
DISCLAIMER:

No electrons were maimed while sending this message. Only slightly bruised.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-12 Thread O. Hartmann
On Fri, 12 Jul 2013 12:13:58 -0500
Scot Hetzel swhet...@gmail.com wrote:

 On Fri, Jul 12, 2013 at 11:16 AM, Scot Hetzel swhet...@gmail.com
 wrote:
  On Thu, Jul 11, 2013 at 9:33 AM, David Chisnall
  thera...@freebsd.org wrote:
  On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au 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.
 
 
   #definefpclassify(x) \
  -((sizeof (x) == sizeof (float)) ? __fpclassifyf(x) \
  -: (sizeof (x) == sizeof (double)) ? __fpclassifyd(x) \
  -: __fpclassifyl(x))
  +__fp_type_select(x, __fpclassifyf, __fpclassifyd,
  __fpclassifyd)
 
  The last __fpclassifyd should be __fpclassifyl.
 
 I see it has already been fixed.
 
 

Obviously not really fixed, but even worse:

if I use in C code (C99, using clang 3.3 on FreeBSD 10.0-CURRENT/amd64
revision 253287) isnan(x) where x is a const double, I receive now
the following error (which doesn't appear on previous versions):

[...]
Making all in scaling
/bin/sh ../../libtool  --tag=CC   --mode=compile cc -DHAVE_CONFIG_H -I.
-I../..  -I. -I../  -I/usr/local/include  -O0 -march=native -g -pipe
-DHAVE_INLINE -g -O2 -MT libscaling_la-scalingTransientCroft.lo -MD -MP
-MF .deps/libscaling_la-scalingTransientCroft.Tpo -c -o
libscaling_la-scalingTransientCroft.lo `test -f
'scalingTransientCroft.c' || echo './'`scalingTransientCroft.c libtool:
compile:  cc -DHAVE_CONFIG_H -I. -I../.. -I. -I../ -I/usr/local/include
-O0 -march=native -g -pipe -DHAVE_INLINE -g -O2 -MT
libscaling_la-scalingTransientCroft.lo -MD -MP
-MF .deps/libscaling_la-scalingTransientCroft.Tpo -c
scalingTransientCroft.c -o libscaling_la-scalingTransientCroft.o
scalingTransientCroft.c:48:12: error: controlling expression type
'const double' not compatible with any generic association type if
(isnan(Dsg) || isnan(Dsc)) ^~~ /usr/include/math.h:109:19: note:
expanded from macro 'isnan' __fp_type_select(x, __inline_isnanf,
__inline_isnan, __inline_isnanl) ^ /usr/include/math.h:86:49: note:
expanded from macro '__fp_type_select' #define __fp_type_select(x, f,
d, ld) _Generic((x), 
[...]

The variables in question (Dsg and Dsc) are of type const double.


signature.asc
Description: PGP signature


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread David Chisnall
Hi Bruce,

You're joining in this discussion starting in the middle, so you probably 
missed the earlier explanation.

On 11 Jul 2013, at 05:21, Bruce Evans b...@optusnet.com.au 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 math.h 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 cmath.

 The FreeBSD isnan() implementation would be broken by removing the
 isnan() function from libm.a or ifdefing it in math.h.  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.  

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)
#define isnan(x) __builtin_isnan(x)
#else
static __inline int
__isnanf(float __x)
{
return (__x != __x);
}
static __inline int
__isnand(double __x)
{
return (__x != __x);
}
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))
#else
#define isnan(x)\
((sizeof (x) == sizeof (float)) ? __isnanf(x)   \
 : (sizeof (x) == sizeof (double)) ? __isnand(x)\
 : __isnanl(x))
#endif
#endif

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);
 ^



David



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread Tijl Coosemans
On 2013-07-11 06:21, Bruce Evans wrote:
 On Wed, 10 Jul 2013, Garrett Wollman wrote:
 On Wed, 10 Jul 2013 22:12:59 +0200, Tijl Coosemans t...@freebsd.org said:
 I think isnan(double) and isinf(double) in math.h should only be
 visible if (_BSD_VISIBLE || _XSI_VISIBLE)  __ISO_C_VISIBLE  1999.
 For C99 and higher there should only be the isnan/isinf macros.

 I believe you are correct.  POSIX.1-2008 (which is aligned with C99)
 consistently calls isnan() a macro, and gives a pseudo-prototype of

 int isnan(real-floating x);
 
 Almost any macro may be implemented as a function, if no conforming
 program can tell the difference.  It is impossible for technical reasons
 to implement isnan() as a macro (except on weird implementations where
 all real-floating types are physically the same).  In the FreeBSD
 implementation, isnan() is a macro, but it is also a function, and
 the macro expands to the function in double precision:
 
 % #defineisnan(x)\
 % ((sizeof (x) == sizeof (float)) ? __isnanf(x)\
 % : (sizeof (x) == sizeof (double)) ? isnan(x)\
 % : __isnanl(x))

The C99 standard says isnan is a macro. I would say that only means
defined(isnan) is true. Whether that macro then expands to function
calls or not is not important.

 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.

I don't think the standard allows a function to be declared with the same
name as a standard macro (it does allow the reverse: define a macro with
the same name as a standard function). I believe the following code is
C99 conforming but it currently does not compile with our math.h:

--
#include math.h

int (isnan)(int a, int b, int c) {
return (a + b + c);
}
--




signature.asc
Description: OpenPGP digital signature


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread Bruce Evans

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 b...@optusnet.com.au 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 math.h 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 cmath.


math.h 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 math.h.  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 

Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread David Chisnall
On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au wrote:

 math.h 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.

Working with the C++ implementation is the problem that we are trying to solve.

 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.

I don't see a problem with changing the name of the function in the header and 
leaving the old symbol in libm for legacy code.  

 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.

Please look in cdefs.h, which defines __has_builtin(x) to 0 if we the compiler 
does not support it.  It is therefore safe to use __has_builtin() in any 
FreeBSD header.

 It also doesn't even work.  clang has squillions of builtins that
 aren't really builtines so they reduce to libcalls.

Which, again, is not a problem for code outside of libm.  If libm needs 
different definitions of these macros then that's fine, but they should be 
private to libm, not installed as public headers.

 The msun implementation knows that isnan() and other classification
 macros are too slow to actually use, and rarely uses them.  

Which makes any concerns that only apply to msun internals irrelevant from the 
perspective of discussing what goes into this header.

 #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.

They will generate the same code.  Clang expands the builtin in the LLVM IR to 
a fcmp uno, so will generate the correct code even when doing fast math 
optimisations.

 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().

That's fine.

 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()?

Yes.

 #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).  

I'm not sure what you were testing:

$ cat isnan2.c 

int test(float f, double d, long double l)
{
return __builtin_isnan(f) |
__builtin_isnan(d) |
__builtin_isnan(l);
}
$ clang isnan2.c -S -emit-llvm -o - -O1
...
  %cmp = fcmp uno float %f, 0.00e+00
  %cmp1 = fcmp uno double %d, 0.00e+00
  %or4 = or i1 %cmp, %cmp1
  %cmp2 = fcmp uno x86_fp80 %l, 0xK
...

As you can see, it parses them as generics and generates different IR for each. 
 I don't believe that there's a way that these would be translated back into 
libcalls in the back end.

 For a trivial example of why this is an improvement in terms of error 
 reporting, consider this trivial piece of code:

Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread Bruce Evans

On Thu, 11 Jul 2013, Tijl Coosemans wrote:


On 2013-07-11 06:21, Bruce Evans wrote:

On Wed, 10 Jul 2013, Garrett Wollman wrote:

On Wed, 10 Jul 2013 22:12:59 +0200, Tijl Coosemans t...@freebsd.org said:

I think isnan(double) and isinf(double) in math.h should only be
visible if (_BSD_VISIBLE || _XSI_VISIBLE)  __ISO_C_VISIBLE  1999.
For C99 and higher there should only be the isnan/isinf macros.


I believe you are correct.  POSIX.1-2008 (which is aligned with C99)
consistently calls isnan() a macro, and gives a pseudo-prototype of

int isnan(real-floating x);


Almost any macro may be implemented as a function, if no conforming
program can tell the difference.  It is impossible for technical reasons
to implement isnan() as a macro (except on weird implementations where
all real-floating types are physically the same).  In the FreeBSD
implementation, isnan() is a macro, but it is also a function, and
the macro expands to the function in double precision:

% #defineisnan(x)\
% ((sizeof (x) == sizeof (float)) ? __isnanf(x)\
% : (sizeof (x) == sizeof (double)) ? isnan(x)\
% : __isnanl(x))


The C99 standard says isnan is a macro. I would say that only means
defined(isnan) is true. Whether that macro then expands to function
calls or not is not important.


I think it means only that defined(isnan) is true.  isnan() can still be
a function (declared or just in the compile-time namespace somewhere,
or in a library object).  It is reserved in the compile-time namespace,
and the standard doesn't cover library objects, so conforming applications
can't reference either except via the isnan() macro (if that has its
strange historical implementation).


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.


I don't think the standard allows a function to be declared with the same
name as a standard macro (it does allow the reverse: define a macro with
the same name as a standard function). I believe the following code is
C99 conforming but it currently does not compile with our math.h:

--
#include math.h

int (isnan)(int a, int b, int c) {
   return (a + b + c);
}
--


I think isnan is just reserved, so you can't redefine it an any way.  I
think the reverse is even less allowed.  Almost any standard function may
be implemented as a macro, and then any macro definition of it would
conflict with the previous macro even more than with a previous prototype.
E.g.:

/* Header. */
void exit(int);
#define exit(x) __exit(x)

/* Application. */
#undef exit /* non-conforming */
#define exit(x) my_exit(x)  /* conflicts without the #undef */

Now suppose the header doesn't define exit().

#define exit(x) my_exit(x)

This hides the protoype but doesn't automatically cause problems, especially
if exit() is not used after this point.  But this is still non-conforming,
since exit() is reserved.

Here are some relevant parts of C99 (n869.txt):

%%%
 -- Each  identifier  with  file scope listed in any of the
following  subclauses  (including  the  future  library
directions)  is  reserved  for  use  as macro and as an
identifier with file scope in the same  name  space  if
any of its associated headers is included.

   [#2]  No  other  identifiers  are  reserved.  If the program
   declares or defines an identifier in a context in  which  it
   is  reserved  (other than as allowed by 7.1.4), or defines a
   reserved  identifier  as  a  macro  name,  the  behavior  is
   undefined.

   [#3]   If  the  program  removes  (with  #undef)  any  macro
   definition of an identifier in the first group listed above,
   the behavior is undefined.
%%%

Without any include of a header that is specified to declare exit(),
file scope things are permitted for it, including defining it and
making it a static function, but not making it an extern function.

isnan is reserved for use as a macro and as an identifier with file
scope by the first clause above.  Thus (isnan) cannot even be defined
as a static function.  But (isnan) is not reserved in inner scopes.
I thought that declarations like int (isnan); are impossible since
they look like syntax errors, but this syntax seems to be allowed an
actually work with gcc-3.3.3 and TenDRA-5.0.0.  So you can have
variables with silly names like (isnan) and (getchar) :-).  However,
(NULL) for a variable name doesn't work, and (isnan) is a syntax error
for struct member names.  The compilers may be correct in allowing
(isnan) but not (NULL) for variables.  isnan happens to be function-like,
so the parentheses are special for (isnan), but the parentheses are not
special for 

Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread David Chisnall
On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au 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.

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().  

David




isnan.diff
Description: Binary data


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread Bruce Evans

On Thu, 11 Jul 2013, David Chisnall wrote:


On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au wrote:


math.h 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.


Working with the C++ implementation is the problem that we are trying to solve.


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
...


I don't see a problem with changing the name of the function in the header and 
leaving the old symbol in libm for legacy code.


I don't even see why old code needs the symbol.  Old code should link to
old compat libraries that still have it.




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.


Please look in cdefs.h, which defines __has_builtin(x) to 0 if we the compiler 
does not support it.  It is therefore safe to use __has_builtin() in any 
FreeBSD header.


The old compilers run on old systems that don't have that in cdefs.h
(though I sometimes edit it to add compatibility cruft like that).  msun
sources are otherwise portable to these systems.  Well, not quite.  They
are not fully modular and also depend on stuff in libc/include and
libc/${ARCH}.  I have to update or edit headers there.

This hack also doesn't work with gcc in -current.  gcc has __builtin_isnan
but not __has_builtin(), so __has_builtin(__builtin_isnan) gives the wrong
result 0.


It also doesn't even work.  clang has squillions of builtins that
aren't really builtines so they reduce to libcalls.


Which, again, is not a problem for code outside of libm.  If libm needs 
different definitions of these macros then that's fine, but they should be 
private to libm, not installed as public headers.


Yes it is.  It means that nothing should use isnan() or FP_FAST_FMA* outside
of libm either, since isnan() is too slow and FP_FAST_FMA* can't be trusted.
Even the implementation can't reliably tell if __builtin_isnan is usuable
or better than alternatives.


The msun implementation knows that isnan() and other classification
macros are too slow to actually use, and rarely uses them.


Which makes any concerns that only apply to msun internals irrelevant from the 
perspective of discussing what goes into this header.


No, the efficiency of isnan() is more important for externals, because the
internals already have work-arounds.


#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.


They will generate the same code.  Clang expands the builtin in the LLVM IR to 
a fcmp uno, so will generate the correct code even when doing fast math 
optimisations.


On some arches the same, and not affected by -ffast-math.  But this
is not necessarily the fastest code, so it is a performance bug if clang
akways generates the same code for the builtin.  Bit tests are faster in
some cases, and may be required to prevent exceptions for signaling NaNs.
-ffast-math could reasonably optimize x != x to false.  It already assumes
that things like overflow and NaN results can't happen, so why not optimize
further by assuming that NaN inputs can't happen?


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).


I'm not sure what you were testing:


Mostly isnan() without including math.h, and gcc.  I was confused by
gcc converting floats to doubles.


$ cat isnan2.c

int test(float f, double d, long double l)
{
   return __builtin_isnan(f) |
   __builtin_isnan(d) |
   __builtin_isnan(l);
}
$ clang isnan2.c -S -emit-llvm -o - -O1
...
 %cmp = fcmp uno float %f, 0.00e+00
 %cmp1 = fcmp uno double %d, 0.00e+00
 %or4 = or i1 %cmp, %cmp1
 %cmp2 = fcmp uno x86_fp80 %l, 0xK
...

As you can see, it parses them as generics and generates different IR for each. 
 I don't believe that there's a way that these would be translated back into 
libcalls in the back end.


Yes, most cases work right.  gcc converts f to double and compares the
result, but that mostly works.  It would be just a pessimization except
te conversion gives an 

Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-11 Thread Bruce Evans

On Thu, 11 Jul 2013, David Chisnall wrote:


On 11 Jul 2013, at 13:11, Bruce Evans b...@optusnet.com.au 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_SUBNORMAL0x08
%  #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 

CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread O. Hartmann

Whe I try to compile the sources of a port in spe (devel/pocl), which
is now out as RC6, I receive this error shown below:

[...]
../vecmathlib/pocl/../vec_sse_double1.h:451:38: error:
conversion from 'int' to 'boolvec_t' (aka 'boolvecreal_t, size') is
ambiguous boolvec_t isinf() const { return std::isinf(v); }
^ ../vecmathlib/pocl/../vec_sse_double1.h:75:5: note:
candidate constructor boolvec(bvector_t x): v(x) {}
^
../vecmathlib/pocl/../vec_sse_double1.h:76:5: note: candidate
constructor boolvec(bool a): v(a) {}
[...]

Compilation is performed on the most recent CURRENT with CLANG 3.3 and
devel/llvm (which is obviously stuck with 3.2 for now) and option
switches -std=c++11 -stdlib=libc++.

As I was told, in standard C++11, isnan(), isinf() and fellows now
should return bool, not int as this seems obviously the case as the
error documents and I was able to check with a small program.

Is this a bug in FreeBSD's implementation of libc++? Or am I doing
something wrong?

I'm new to C++/C++11.


Some advice or explanation could be helpful.

regards,

Oliver


signature.asc
Description: PGP signature


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread David Chisnall
Hi,

On 10 Jul 2013, at 14:58, O. Hartmann ohart...@zedat.fu-berlin.de wrote:

 
 Whe I try to compile the sources of a port in spe (devel/pocl), which
 is now out as RC6, I receive this error shown below:
 
 [...]
 ../vecmathlib/pocl/../vec_sse_double1.h:451:38: error:
 conversion from 'int' to 'boolvec_t' (aka 'boolvecreal_t, size') is
 ambiguous boolvec_t isinf() const { return std::isinf(v); }
 ^ ../vecmathlib/pocl/../vec_sse_double1.h:75:5: note:
 candidate constructor boolvec(bvector_t x): v(x) {}
^
 ../vecmathlib/pocl/../vec_sse_double1.h:76:5: note: candidate
 constructor boolvec(bool a): v(a) {}
 [...]
 
 Compilation is performed on the most recent CURRENT with CLANG 3.3 and
 devel/llvm (which is obviously stuck with 3.2 for now) and option
 switches -std=c++11 -stdlib=libc++.
 
 As I was told, in standard C++11, isnan(), isinf() and fellows now
 should return bool, not int as this seems obviously the case as the
 error documents and I was able to check with a small program.
 
 Is this a bug in FreeBSD's implementation of libc++? Or am I doing
 something wrong?
 
 I'm new to C++/C++11.
 
 
 Some advice or explanation could be helpful.

I believe that this is also causing some failures in the libc++ test suite and 
is due to some interaction between our headers and the libc++ headers, but I 
don't see where it is.

Our isnan implementation is a really ugly macro that looks like this:

#defineisnan(x)\
((sizeof (x) == sizeof (float)) ? __isnanf(x)   \
: (sizeof (x) == sizeof (double)) ? isnan(x)\
: __isnanl(x))


The definition in the libc++ cmath header is:

#ifdef isnan

template class _A1
_LIBCPP_ALWAYS_INLINE
bool
__libcpp_isnan(_A1 __x) _NOEXCEPT
{
return isnan(__x);
}

#undef isnan

This should work correctly.

However...

I wonder if you are including math.h instead of cmath?  That would show the 
result that you appear to be seeing, which looks like the result of using the 
isnan() macro rather than the isnan() function.  If you have included cmath 
then the isnan() macro will have been defined.

David



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread O. Hartmann
On Wed, 10 Jul 2013 15:22:58 +0100
David Chisnall thera...@freebsd.org wrote:

 Hi,
 
 On 10 Jul 2013, at 14:58, O. Hartmann ohart...@zedat.fu-berlin.de
 wrote:
 
  
  Whe I try to compile the sources of a port in spe (devel/pocl),
  which is now out as RC6, I receive this error shown below:
  
  [...]
  ../vecmathlib/pocl/../vec_sse_double1.h:451:38: error:
  conversion from 'int' to 'boolvec_t' (aka 'boolvecreal_t, size')
  is ambiguous boolvec_t isinf() const { return std::isinf(v); }
  ^ ../vecmathlib/pocl/../vec_sse_double1.h:75:5: note:
  candidate constructor boolvec(bvector_t x): v(x) {}
 ^
  ../vecmathlib/pocl/../vec_sse_double1.h:76:5: note: candidate
  constructor boolvec(bool a): v(a) {}
  [...]
  
  Compilation is performed on the most recent CURRENT with CLANG 3.3
  and devel/llvm (which is obviously stuck with 3.2 for now) and
  option switches -std=c++11 -stdlib=libc++.
  
  As I was told, in standard C++11, isnan(), isinf() and fellows now
  should return bool, not int as this seems obviously the case as
  the error documents and I was able to check with a small program.
  
  Is this a bug in FreeBSD's implementation of libc++? Or am I doing
  something wrong?
  
  I'm new to C++/C++11.
  
  
  Some advice or explanation could be helpful.
 
 I believe that this is also causing some failures in the libc++ test
 suite and is due to some interaction between our headers and the
 libc++ headers, but I don't see where it is.
 
 Our isnan implementation is a really ugly macro that looks like this:
 
 #defineisnan(x)  \
 ((sizeof (x) == sizeof (float)) ? __isnanf(x) \
 : (sizeof (x) == sizeof (double)) ? isnan(x)  \
 : __isnanl(x))
 
 
 The definition in the libc++ cmath header is:
 
 #ifdef isnan
 
 template class _A1
 _LIBCPP_ALWAYS_INLINE
 bool
 __libcpp_isnan(_A1 __x) _NOEXCEPT
 {
 return isnan(__x);
 }
 
 #undef isnan
 
 This should work correctly.
 
 However...
 
 I wonder if you are including math.h instead of cmath?  That would
 show the result that you appear to be seeing, which looks like the
 result of using the isnan() macro rather than the isnan() function.
 If you have included cmath then the isnan() macro will have been
 defined.
 
 David
 

Hi David,

thanks for the fast response.

The code I was told to check with is this:

#include iostream
#include typeinfo
#include cmath

int
main(void)
{

std::cout  typeid(isnan(1.0)).name()  \n;

}


If I compile it with 

c++ -o testme -std=c++11 -stdlib=libc++ source.cc

and run the binary, the result is i which I interpret as INT.

My OS is at 

FreeBSD 10.0-CURRENT #4 r253138: Wed Jul 10 09:52:16 CEST 2013

Oliver


signature.asc
Description: PGP signature


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread David Chisnall
On 10 Jul 2013, at 17:33, O. Hartmann ohart...@zedat.fu-berlin.de wrote:

 Hi David,
 
 thanks for the fast response.
 
 The code I was told to check with is this:
 
 #include iostream
 #include typeinfo
 #include cmath
 
 int
 main(void)
 {
 
std::cout  typeid(isnan(1.0)).name()  \n;
 
 }
 
 
 If I compile it with 
 
 c++ -o testme -std=c++11 -stdlib=libc++ source.cc
 
 and run the binary, the result is i which I interpret as INT.

I believe there is a bug, which is that the math.h things are being exposed but 
shouldn't be, however it is not the bug that you think it is.  Try this line 
instead:

   std::cout  typeid(std::isnan(1.0)).name()  \n;

We have a libm function, isnan(), and a libc++ function, std::isnan().  The 
former is detected if you do not specify a namespace.  I am not sure what will 
happen if you do:

#include iostream
#include typeinfo
#include cmath
using namespace std;

int
main(void)
{

   cout  typeid(isnan(1.0)).name()  \n;

}

This is considered bad form, but does happen in some code.  I am not certain 
what the precedence rules are in this case and so I don't know what happens.

To properly fix this, we'd need to namespace the libm functions when including 
math.h in C++.  This would also include fixing tweaking the macros.  

A fix for your code is to ensure isnan() and isinf() are explicitly namespaced. 
 Potentially, this may also work:

using std::isinf;
using std::isnan;

David



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread O. Hartmann
On Wed, 10 Jul 2013 18:04:16 +0100
David Chisnall thera...@freebsd.org wrote:

 On 10 Jul 2013, at 17:33, O. Hartmann ohart...@zedat.fu-berlin.de
 wrote:
 
  Hi David,
  
  thanks for the fast response.
  
  The code I was told to check with is this:
  
  #include iostream
  #include typeinfo
  #include cmath
  
  int
  main(void)
  {
  
 std::cout  typeid(isnan(1.0)).name()  \n;
  
  }
  
  
  If I compile it with 
  
  c++ -o testme -std=c++11 -stdlib=libc++ source.cc
  
  and run the binary, the result is i which I interpret as INT.
 
 I believe there is a bug, which is that the math.h things are being
 exposed but shouldn't be, however it is not the bug that you think it
 is.  Try this line instead:
 
std::cout  typeid(std::isnan(1.0)).name()  \n;
 
 We have a libm function, isnan(), and a libc++ function,
 std::isnan().  The former is detected if you do not specify a
 namespace.  I am not sure what will happen if you do:
 
 #include iostream
 #include typeinfo
 #include cmath
 using namespace std;
 
 int
 main(void)
 {
 
cout  typeid(isnan(1.0)).name()  \n;
 
 }
 
 This is considered bad form, but does happen in some code.  I am not
 certain what the precedence rules are in this case and so I don't
 know what happens.
 
 To properly fix this, we'd need to namespace the libm functions when
 including math.h in C++.  This would also include fixing tweaking the
 macros.  
 
 A fix for your code is to ensure isnan() and isinf() are explicitly
 namespaced.  Potentially, this may also work:
 
 using std::isinf;
 using std::isnan;
 
 David
 

I tried in the test code I provided using 


#include iostream
#include typeinfo
#include cmath

int
main(void)
{

std::cout  typeid(std::isnan(1.0)).name()  \n;

}

now std::isnan().

The result is the same, it flags INT.

Using 

#include iostream
#include typeinfo
#include cmath

using namespace std;

int
main(void)
{

std::cout  typeid(std::isnan(1.0)).name()  \n;

}

which is considered bad coding also results in INT (it gives i).

So, is this woth a PR?

Oliver


signature.asc
Description: PGP signature


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread Tijl Coosemans
On 2013-07-10 20:32, O. Hartmann wrote:
 On Wed, 10 Jul 2013 18:04:16 +0100
 David Chisnall thera...@freebsd.org wrote:
 
 On 10 Jul 2013, at 17:33, O. Hartmann ohart...@zedat.fu-berlin.de
 wrote:

 Hi David,

 thanks for the fast response.

 The code I was told to check with is this:

 #include iostream
 #include typeinfo
 #include cmath

 int
 main(void)
 {

std::cout  typeid(isnan(1.0)).name()  \n;

 }


 If I compile it with 

 c++ -o testme -std=c++11 -stdlib=libc++ source.cc

 and run the binary, the result is i which I interpret as INT.

 I believe there is a bug, which is that the math.h things are being
 exposed but shouldn't be, however it is not the bug that you think it
 is.  Try this line instead:

std::cout  typeid(std::isnan(1.0)).name()  \n;

 We have a libm function, isnan(), and a libc++ function,
 std::isnan().  The former is detected if you do not specify a
 namespace.  I am not sure what will happen if you do:

 #include iostream
 #include typeinfo
 #include cmath
 using namespace std;

 int
 main(void)
 {

cout  typeid(isnan(1.0)).name()  \n;

 }

 This is considered bad form, but does happen in some code.  I am not
 certain what the precedence rules are in this case and so I don't
 know what happens.

 To properly fix this, we'd need to namespace the libm functions when
 including math.h in C++.  This would also include fixing tweaking the
 macros.  

 A fix for your code is to ensure isnan() and isinf() are explicitly
 namespaced.  Potentially, this may also work:

 using std::isinf;
 using std::isnan;

 David

 
 I tried in the test code I provided using 
 
 
 #include iostream
 #include typeinfo
 #include cmath
 
 int
 main(void)
 {
 
 std::cout  typeid(std::isnan(1.0)).name()  \n;
 
 }
 
 now std::isnan().
 
 The result is the same, it flags INT.
 
 Using 
 
 #include iostream
 #include typeinfo
 #include cmath
 
 using namespace std;
 
 int
 main(void)
 {
 
 std::cout  typeid(std::isnan(1.0)).name()  \n;
 
 }
 
 which is considered bad coding also results in INT (it gives i).
 
 So, is this woth a PR?

isnan is overloaded. There's int isnan(double) in math.h and
bool isnan(arithmetic) in cmath. When you call isnan(1.0),
isnan(double) is selected.

I think isnan(double) and isinf(double) in math.h should only be
visible if (_BSD_VISIBLE || _XSI_VISIBLE)  __ISO_C_VISIBLE  1999.
For C99 and higher there should only be the isnan/isinf macros.

CCed standards@.



signature.asc
Description: OpenPGP digital signature


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread Garrett Wollman
On Wed, 10 Jul 2013 22:12:59 +0200, Tijl Coosemans t...@freebsd.org said:

 I think isnan(double) and isinf(double) in math.h should only be
 visible if (_BSD_VISIBLE || _XSI_VISIBLE)  __ISO_C_VISIBLE  1999.
 For C99 and higher there should only be the isnan/isinf macros.

I believe you are correct.  POSIX.1-2008 (which is aligned with C99)
consistently calls isnan() a macro, and gives a pseudo-prototype of

int isnan(real-floating x);

-GAWollman

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: CURRENT: CLANG 3.3 and -stad=c++11 and -stdlib=libc++: isnan()/isninf() oddity

2013-07-10 Thread Bruce Evans

On Wed, 10 Jul 2013, Garrett Wollman wrote:


On Wed, 10 Jul 2013 22:12:59 +0200, Tijl Coosemans t...@freebsd.org said:


I think isnan(double) and isinf(double) in math.h should only be
visible if (_BSD_VISIBLE || _XSI_VISIBLE)  __ISO_C_VISIBLE  1999.
For C99 and higher there should only be the isnan/isinf macros.


I believe you are correct.  POSIX.1-2008 (which is aligned with C99)
consistently calls isnan() a macro, and gives a pseudo-prototype of

int isnan(real-floating x);


Almost any macro may be implemented as a function, if no conforming
program can tell the difference.  It is impossible for technical reasons
to implement isnan() as a macro (except on weird implementations where
all real-floating types are physically the same).  In the FreeBSD
implementation, isnan() is a macro, but it is also a function, and
the macro expands to the function in double precision:

% #define   isnan(x)\
% ((sizeof (x) == sizeof (float)) ? __isnanf(x) \
% : (sizeof (x) == sizeof (double)) ? isnan(x)  \
% : __isnanl(x))

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 math.h or libm.a and
creates a bug for C++.

The FreeBSD isnan() implementation would be broken by removing the
isnan() function from libm.a or ifdefing it in math.h.  Changing the
function to __isnan() would cause compatibility problems.  The function
is intentionally named isnan() to reduce compatibility problems.

OTOH, the all of the extern sub-functions that are currently used should
bever never be used, since using them gives a very low quality of
implementation:
- the functions are very slow
- the functions have names that confuse compilers and thus prevent
  compilers from replacing them by builtins.  Currently, only gcc
  automatically replaces isnan() by __builtin_isnan().  This only
  works in double precision.  So the FreeBSD implementation only
  works right in double precision too, only with gcc, __because__
  it replaces the macro isnan(x) by the function isnan(x).  The
  result is inline expansion, the same as if the macro isnan()
  is replaced by __builtin_isnan().  clang never does this automatic
  replacement, so it generates calls to the slow library functions.
  Other things go wrong for gcc in other precisions:
  - if math.h is not included, then isnan(x) gives
__builtin_isnan((double)x).  This sort of works on x86, but is
low quality since it is broken for signaling NaNs (see below).
One of the main reasons reason for the existence of the
classification macros is that simply converting the arg to a common
type and classifying the result doesn't always work.
  - if math.h is not included, then spelling the API isnanf() or
isnanl() gives correct results but a warning about these APIs
not being declared.  These APIs are nonstandard but are converted
to __builtin_isnan[fl] by gcc.
  - if math.h is included, then:
- if the API is spelled isnan(), then the macro converts to
  __isnanf() or __isnanl().  gcc doesn't understand these, and
  the slow extern functions are used.
- if the API is spelled isnanf() or isnanl(), then the result is
  correct and the warning magically goes away.  math.h declares
  isnanf(), but gcc apparently declares both iff math.h is included.
  gcc also optimizes isnanl() on a float arg to __builtin_isnanf().
- no function version can work in some cases, because any function version
  may  have unwanted side effects.  This is another of the main reason
  for the existence of these and other macros.  The main unwanted side
  effect is signaling for signaling NaNs.  C99 doesn't really support
  signaling NaNs, even with the IEC 60559 extensions, so almost anything
  is allowed for them.  But IEEE 854 is fairly clear that isnan() and
  classification macros shouldn't raise any exceptions.  IEEE 854 is
  even clearer that copying values without changing their representation
  should (shall?) not cause exceptions.  But on i387, just loading a float
  or double value changes its representation and generates an exception
  for signaling NaNs, while just loading a long double value conforms to
  IEEE 854 and doesn't change its representation or generate an exception.
  Passing of args to functions may or may not load the values.  ABIs may
  require a change of representation.  On i387, passing of double args
  should go through the FPU for efficiency reasons, and this changes the
  representation twice to not even get back to the original (for signaling
  NaNs, it generates an exception and sets the quiet bit in the result;
  thus a classification function can never see a signaling NaN in double
  precision).  So