This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 5/10] Tilera (and Linux asm-generic) support for glibc

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

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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]