[PATCH v2 5/10] Tilera (and Linux asm-generic) support for glibc
Chris Metcalf
cmetcalf@tilera.com
Fri Nov 11 18:48:00 GMT 2011
On 11/11/2011 12:59 PM, Roland McGrath wrote:
>> diff --git a/include/fenv.h b/include/fenv.h
>> index 3605f81..c061eb9 100644
>> --- a/include/fenv.h
>> +++ b/include/fenv.h
>> @@ -20,4 +20,10 @@ libm_hidden_proto (feholdexcept)
>> libm_hidden_proto (feupdateenv)
>> libm_hidden_proto (fetestexcept)
>>
>> +/* Allow coding feraiseexcept() without guarding the call with an
>> + ifdef of the argument, to suport platforms without FP exceptions. */
> There's a typo in this comment.
Oops, so there is. I had to re-read it twice to notice it, for some reason.
>> +#if FE_ALL_EXCEPT == 0
>> +# define feraiseexcept(e) ({ 1; })
>> +#endif
> This is just generically bad macro practice: it fails to evaluate its argument.
Except in this case, that's exactly why this macro is defined the way
it is. The issue is that commit 77425c63e72b (last month) removed a
lot of the guards around calls to feraiseexcept(), with diffs that
looked like this:
@@ -112,10 +107,8 @@ __cexp (__complex__ double x)
__real__ retval = __nan ("");
__imag__ retval = __nan ("");
-#ifdef FE_INVALID
if (rcls != FP_NAN || icls != FP_NAN)
feraiseexcept (FE_INVALID);
-#endif
}
return retval;
The removal of the guards wasn't mentioned in the commit message,
which just said it was for adding branch prediction; I assume it was
viewed as just normal cleanup that didn't require mentioning. A
number of other minor cleanups (variable scope, whitespace, etc.) also
happened in this commit. There are still some files that test with an
"#ifdef FE_INVALID", for example (s_ccoshf.c, etc.) but the majority
of them no longer do.
The problem is that if some platform does not support FE_INVALID,
there will be no definition of it in<fenv.h>, and this code then
can't compile. So my workaround is to observe that if a given
platform has no exceptions at all, then feraiseexcept() will be a
no-op no matter what its argument is. By ignoring the argument we
then allow the math code to compile. It's a bit of a hack, but it
does mean we don't have to put back all the "#ifdef FE_INVALID", etc.,
lines (though from a standards point of view that's probably still the
best solution).
I would have preferred to put the feraiseexcept() hack in some
tile-specific header, but I couldn't see an obvious one.
<bits/mathdef.h> is of course installed, so although I could add an
"#if _LIBC" stanza there, it seemed inappropriate. The alternative
would be a new file, e.g., sysdeps/generic/libc-fenv.h, which just
includes<fenv.h> by default, but overrideable by platform, which the
math source files would be switched over to use. But that seemed
pretty heavyweight, so I didn't go down that route.
Sorry not to have included more rationale in the original submissions;
in retrospect it was an obvious oversight.
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
More information about the Libc-alpha
mailing list