[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 02 Dec 2014 20:10:32 +0100
From: Jens Gustedt <jens.gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/4] the CMPLX macros must be usable in
initializations of static variables
Hello,
Am Dienstag, den 02.12.2014, 12:49 -0500 schrieb Rich Felker:
> On Wed, Nov 26, 2014 at 02:07:55PM +0100, Jens Gustedt wrote:
> > Because of some boundary cases for infinities and negative zeros, doing
> > this properly is only possible with either support for _Imaginary or some
> > compiler magic.
> >
> > For the moment, we only know such magic for clang and gcc. There it is
> > only available for newer compilers. Therefore we make the CMPLX macros
> > only available when in C11 mode (or actually even in C1X mode).
> >
> > Internally for the compilation of the complex functions of the math
> > library we use such a macro, but that doesn't have the constraint of
> > being usable for static initializations. So if we are not in C11, we
> > provide such a macro as __CMPLX_NC in complex.h and map the CMPLX macros
> > internally to that.
> >
> > As an effect this reverts commit faea4c9937d36b17e53fdc7d5a254d7e936e1755.
> > ---
> > include/complex.h | 30 ++++++++++++++++++++++++++++--
> > src/internal/libm.h | 8 ++++++++
> > 2 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/complex.h b/include/complex.h
> > index 13a45c5..e88cf13 100644
> > --- a/include/complex.h
> > +++ b/include/complex.h
> > @@ -112,12 +112,38 @@ long double creall(long double complex);
> > #define cimagf(x) __CIMAG(x, float)
> > #define cimagl(x) __CIMAG(x, long double)
> >
> > -#define __CMPLX(x, y, t) \
> > - ((union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
> > +#ifdef _Imaginary_I
> > +# define __CMPLX(x, y, t) ((t)(x) + _Imaginary_I*(t)(y))
> > +#else
> > +# define __CMPLX_I(x, y, t) ((t)(x) + _Complex_I*(t)(y))
> > +#endif
>
> I was wondering about the purpose of the casts here, and I got to
> thinking: what is the behavior supposed to be with regards to excess
> precision?
> Before it was truncated by type punning
no, before the conversion was implicit in the initializer, I think
> and now it's
> truncated by cast. But without the cast it would not be truncated.
> Which behavior is right?
The reason for the cast is simple, the returned type must be
`_Complex t` and nothing else.
This could otherwise be achived by casting the result to `_Complex t`,
but the requirements for the macros are expressed as those of function
interfaces, so a separate conversion of each of the arguments is in
order.
> > +#ifndef __CMPLX
> > +# if defined(__clang__)
> > + /* Clang allows initializer lists for complex numbers and compound
> > + literals for the initialization of static variables. */
> > +# define __CMPLX(x, y, t) (+(_Complex t){ (x), (y) })
> > +# elif 100*__GNUC__+__GNUC_MINOR__ >= 407
> > +# define __CMPLX(x, y, t) __builtin_complex((t)(x), (t)(y))
> > +# endif
> > +#endif
>
> Does clang not support the GCC builtin?
no, and I don't have the impression they will. They invented their own
internals for this, as you can see here.
> I would highly prefer not to
> use the clang form since it's a compiler bug that should be fixed in
> future clang.
clang only claims to support gcc builtins up to 4.2, I think. For the
others they do it "à la carte", and __builtin_complex is quite recent,
so it may take a while, if ever.
We could use their feature test macros for that, but for the moment we
don't have anything like that, I think.
> > +#ifndef __CMPLX
> > +# if __STDC_VERSION__ >= 201000L
> > +# warning for this compiler, macros CMPLX, CMPLXF and CMPLXL are not standard
> > +# warning conforming for infinities and signed zeros
> > +# define __CMPLX(x, y, t) __CMPLX_I(x, y, t)
> > +# endif
> > +# define __CMPLX_NC(x, y, t) (+(union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
> > +#else
> > +# define __CMPLX_NC(x, y, t) __CMPLX(x, y, t)
> > +#endif
> > +
> > +#if __STDC_VERSION__ >= 201000L
> > #define CMPLX(x, y) __CMPLX(x, y, double)
> > #define CMPLXF(x, y) __CMPLX(x, y, float)
> > #define CMPLXL(x, y) __CMPLX(x, y, long double)
> > +#endif
>
> __CMPLX_NC is for internal use only and has no reason to be in the
> public header.
right, no problem, I'll push it into libm.h
> Also I'd like to keep the behavior in the public header
> a lot more explicit rather than having lots of nested conditionals and
> #ifndef __CMPLX where it's necessary to track back through all the
> conditions to reason about which case gets used.
yes, me too, as I already said earlier
(but in case of errors both, gcc and clang are quite good now in
tracking down to the real definition)
> My preference would be if we could just define CMPLX either in terms
> of __builtin_complex,
not possible, I think
> and only exposing it in C11 mode.
this is already the case. (Or is it C1x that bothers you?)
> _Imaginary_I
> is not supported yet at all and would require further changes to this
> header to add anyway (this header is responsible for defining it), so
> conditioning on its definition is not meaningful.
I am not sure of that. It could also come directly from the compiler
just as it defines some __SOMETHING__ macros before any include
> And pre-4.7 versions
> of GCC don't support C11 anyway (and even 4.7 and 4.8 are highly
> incomplete) so I don't think much is lost if a C11 feature fails to
> work on older compilers.
as said, there is no intention to support it for older compilers. So I
read it that you want me to pull the definition of the __CMPLX
versions into the C1x conditional?
> The big question is how this fares for clang
> compatibility though. I expect pcc and cparser/firm will provide a
> compatible __builtin_complex (if they don't already) as they improve
> C11 support.
>
> > diff --git a/src/internal/libm.h b/src/internal/libm.h
> > index ebcd784..f916e2e 100644
> > --- a/src/internal/libm.h
> > +++ b/src/internal/libm.h
> > @@ -155,4 +155,12 @@ long double __tanl(long double, long double, int);
> > long double __polevll(long double, const long double *, int);
> > long double __p1evll(long double, const long double *, int);
> >
> > +/* complex */
> > +
> > +#ifndef CMPLX
> > +#define CMPLX(x, y) __CMPLX_NC(x, y, double)
> > +#define CMPLXF(x, y) __CMPLX_NC(x, y, float)
> > +#define CMPLXL(x, y) __CMPLX_NC(x, y, long double)
> > +#endif
> > +
> > #endif
>
> This should probably use the unions unconditionally, after including
> complex.h and #undef'ing CMPLX[FL], since musl does not require a C11
> compiler to build it. Depending on whether a fallback is provided for
> old compilers or not, I think your approach could lead to musl being
> miscompiled. It probably doesn't happen since CMPLX is no longer
> exposed except in C11 mode, and musl is compiled with -std=c99, but
> this looks fragile and could cause breakage to go unnoticed if we
> switch to preferring -std=c11 some time later. (There's been talk of
> preferring -std=c11 if it works.)
Hm, clearly not my preference. I think we should use the right tool
(which is __builtin_complex for gcc or the bogus initializer for
clang) as soon as we have it.
This is why I wanted to have the detection of the features as early as
possible where it belongs to my opinion, complex.h.
They have more chances to optimize things correctly without
temporaries even when compiled without optimization.
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
