[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141202194210.GF29621@brightrain.aerifal.cx>
Date: Tue, 2 Dec 2014 14:42:10 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/4] the CMPLX macros must be usable in
initializations of static variables
On Tue, Dec 02, 2014 at 08:10:32PM +0100, Jens Gustedt wrote:
> 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
Yes, sorry I was imprecise.
> > 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.
I see.
> 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.
Perhaps it could also be achieved via some other means of coercing an
appropriate type, but given the paragraph 4 note in the standard, I'm
happy with the casts.
> > > +#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.
Yes, but their form is a bug that needs to be removed, so they'll have
to add something in its place, and __builtin_complex is the natural
choice. For what it's worth, glibc only supports __builtin_complex and
just does not define these macros at all if it's not supported.
> > 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)
In the case of reporting warnings/errors, yes. But in case a user is
trying to figure out which definition is getting used because it's
relevant to a bug they're facing in their program, #ifdef mazes are a
pain.
> > My preference would be if we could just define CMPLX either in terms
> > of __builtin_complex,
>
> not possible, I think
Well, it's what glibc does. That's not to say it's a good choice, but
it suggests there could be pressure to get other compilers to support
it.
> > and only exposing it in C11 mode.
>
> this is already the case. (Or is it C1x that bothers you?)
I did find the 201000L comparison a bit odd (is that the "C1x" thing?)
but my comment here wasn't in reference to your proposal being
different from the ideal I'd like to have, just a statement of the
latter.
> > _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
I don't think so. Consider:
#undef _Imaginary_I
#include <complex.h>
Perhaps this is non-conforming for technical reasons, but the standard
is fairly direct in saying that complex.h defines the _Imaginary_I
macro rather than just having it predefined.
> > 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?
I'm not sure I follow what you're saying here. The text you quoted was
in regards to my hope of just having a single definition, not a
specific request for any particular changes to your proposed patch.
> > 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.
As stated in another thread, my preference is highly towards not
having multiple versions of the same code, especially when one or more
are unlikely to get good testing. The "more chance to optimize"
argument does not apply unless there really are compilers which
compile the unions poorly (not counting intentional non-optimizing
modes) and I'm not aware of any.
Rich
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.
