On Sun, Mar 13, 2011 at 9:14 AM, Brad <[email protected]> wrote:
> On 08/03/11 2:50 PM, Matthew Dempsky wrote:
>> The diff below adds support for C99's<fenv.h> to libm. It's based on
>> NetBSD's implementation with minimal changes to work on OpenBSD.
>>
>> Currently, the diff only supports amd64, i386, and sparc64 (the only
>> arches that NetBSD supports fenv.h on), but I've only tested on amd64
>> so far. Using this diff, I was able to successfully build and run
>> OpenSCAD.
Hmm, any test programs that are smaller and preferably not need X11?
My little V100 takes forever with a build as is...
...
> 2 things I noticed right away...
>
> You should be using __sparc64__ instead of __sparc__ and _NETBSD_SOURCE
> should be replaced with __BSD_VISIBLE.
I'll second/confirm both of those. Note that it'll need to pull in
<sys/cdefs.h> for the latter to work.
Other thoughts:
The platform check in <fenv.h> should be moved inside the _FENV_H_
inclusion guard. Or maybe it can simply be removed: if the platform
doesn't support it, then it won't have <machine/fenv.h> and the
compilation will fail on the #include.
Might want to wrap the function names in the declarations in <fenv.h>
in parens, ala:
int (feclearexcept)(int);
so that possible future work to provide macros for those in
<machine/fenv.h> don't have to go back and tweak them.
These headers violate POSIX namespace rules in various ways:
- pulling in <sys/stdint.h>, <machine/fpu.h>, and <i386/npx.h>
defines names that aren't permitted.
(seems unnecessary too; just use unsigned short/int/long long
instead of uint16_t/uint32_t/uint64_t)
- the members of the fenv_t structures in the i386 and amd64 versions
must have names in the implementation
namespace, ala __control, __status, __x87, etc.
Philip Guenther
