Hi Francois,
On 01/10/14 15:29, Francois Gouget wrote:
The INFINITY and NAN definitions have been added to math.h in recent versions of msvcrt (i.e. they were not present in Visual C++ 2008 but are in Visual C++ 2013). This means we have to provide them too otherwise code that compiles with Visual C++ could fail to compile with Winelib.
To do so I'm just reusing the definition we have in wine/port.h.
This definition bothers me a bit in that it clearly depends on the in-memory representation of these floating point constants, which might not be very portable (depending on whether the system uses IEEE 754 or not, etc). In d3dx9_36 we used to define NAN as '0.0f / 0.0f'. Other definitions would be variants like 'INFINITY * 0.0f', etc. I can't really claim that those are really more portable and the compiler may (and some do) complain about them. Hence why I went with the wine/port.h definitions in the end. If they're good enough for port.h they should be good for math.h too.
include/msvcrt/math.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/include/msvcrt/math.h b/include/msvcrt/math.h index 34aa268..42d6bfe 100644 --- a/include/msvcrt/math.h +++ b/include/msvcrt/math.h @@ -146,6 +146,26 @@ static const union { # endif #endif
+#ifndef INFINITY +/* From wine/port.h */ +static inline float __port_infinity(void) +{
- static const unsigned __inf_bytes = 0x7f800000;
- return *(const float *)&__inf_bytes;
+} +#define INFINITY __port_infinity() +#endif
+#ifndef NAN +/* From wine/port.h */ +static inline float __port_nan(void) +{
- static const unsigned __nan_bytes = 0x7fc00000;
- return *(const float *)&__nan_bytes;
+} +#define NAN __port_nan() +#endif
#ifdef __cplusplus } #endif
In wine/port.h, we implement worst-case scenario where math.h doesn't define NAN/INFINITY. In this case, since glibc/system math.h won't be used and our msvcrt math.h will be included instead, we should try to provide better NAN/INFINITY versions when possible. Looking at how it's done in my Linux headers, there is __builtin_nanf() and __builtin_inff() guarded by GCC >= 3.3 that we could use. How is it defined on MSVC?
Jacek
On Fri, 10 Jan 2014, Jacek Caban wrote: [...]
Looking at how it's done in my Linux headers, there is __builtin_nanf() and __builtin_inff() guarded by GCC >= 3.3 that we could use.
This would work on Linux. But the Solaris headers don't define NAN which is what uncovered this problem in the first place... In other words the msvcrt headers cannot rely on the system headers to define INFINITY and NAN.
How is it defined on MSVC?
They define INFINITY by multiplying two very large floats that they know will overflow (which makes assumptions on the maximum exponant and causes a Visual C++ compiler warning).
Then they define NAN as INFINITY * 0.
On 1/10/14 5:44 PM, Francois Gouget wrote:
On Fri, 10 Jan 2014, Jacek Caban wrote: [...]
Looking at how it's done in my Linux headers, there is __builtin_nanf() and __builtin_inff() guarded by GCC >= 3.3 that we could use.
This would work on Linux. But the Solaris headers don't define NAN which is what uncovered this problem in the first place... In other words the msvcrt headers cannot rely on the system headers to define INFINITY and NAN.
__builtin_* stuff is compiler, not OS, dependent, so this should work on Solaris when GCC is used. We still have to use __port_nan()-like solution as a fallback for compilers that we can't support other way, but IMO we should try harder to provide a better definition.
Jacek
On Sat, 11 Jan 2014, Jacek Caban wrote: [...]
__builtin_* stuff is compiler, not OS, dependent, so this should work on Solaris when GCC is used. We still have to use __port_nan()-like solution as a fallback for compilers that we can't support other way, but IMO we should try harder to provide a better definition.
The only NAN definitions I see on Solaris are in /usr/include/iso/math_c99.h:
#if defined(_STDC_C99) || _XOPEN_SOURCE - 0 >= 600 || defined(__C99FEATURES__) # if defined(__GNUC__) [...] # undef NAN # define NAN (__builtin_nanf("")) # else [...] # undef NAN # define NAN __builtin_nan # endif
The problem is: * Not all systems have iso/math_c99.h so we cannot unconditionally include it. But autoconf macros checks like HAVE_XXX are not allowed in the Winelib headers. This means we cannot include math_c99.h from our math.h header.
* The system math.h header includes math_c99.h unconditionally. But including the system math.h from our own does not seem possible (I don't think forcing the Winelib users to use weird -I/symlink/to/usr/include would be acceptable).
* __builtin_nanf() is not a macro so we cannot check for it. So we'd have to duplicate either Solaris' or Linux's #ifdefs but that looks pretty fragile.
So I'm not sure there is much more we can do.
-
Francois Gouget -
Jacek Caban