[PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

---
 include/complex.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/complex.h b/include/complex.h
index 13a45c5..3e14e04 100644
--- a/include/complex.h
+++ b/include/complex.h
@@ -112,8 +112,7 @@ 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)
+#define __CMPLX(x, y, t) ((t)(x) + _Complex_I*(t)(y))
 
 #define CMPLX(x, y) __CMPLX(x, y, double)
 #define CMPLXF(x, y) __CMPLX(x, y, float)
-- 
1.9.1

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Szabolcs Nagy]

* Jens Gustedt <Jens.Gustedt@inria.fr> [2014-11-25 15:49:39 +0100]:
> diff --git a/include/complex.h b/include/complex.h
> index 13a45c5..3e14e04 100644
> --- a/include/complex.h
> +++ b/include/complex.h
> @@ -112,8 +112,7 @@ 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)

hm the compound literal is not a constant expression

that is sad

> +#define __CMPLX(x, y, t) ((t)(x) + _Complex_I*(t)(y))
>  

this is only correct if the compiler supports annex g kind of
imaginary type, otherwise it is incorrect for infinites

_Complex_I*INFINITY == (0 + i)*(inf + i*0) == nan + i*inf

so imaginary inf will turn into nan real part

this is what gcc (and i assume clang) does

>  #define CMPLX(x, y) __CMPLX(x, y, double)
>  #define CMPLXF(x, y) __CMPLX(x, y, float)
> -- 
> 1.9.1

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2014-11-25 16:25:00 +0100]:
> * Jens Gustedt <Jens.Gustedt@inria.fr> [2014-11-25 15:49:39 +0100]:
> > -#define __CMPLX(x, y, t) \
> > -	((union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)
> 
> hm the compound literal is not a constant expression
> 
> > +#define __CMPLX(x, y, t) ((t)(x) + _Complex_I*(t)(y))
> >  
> 
> this is only correct if the compiler supports annex g kind of
> imaginary type, otherwise it is incorrect for infinites

it seems gcc added __builtin_complex but there is no easy way to check
for the availability of such builtin (other than checking >= gcc 4.7)

http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01600.html

i guess we could use the builtin with gcc version check and fall back
to the compound literal version otherwise (it wont work as static
initializer then, but right now the biggest user of CMPLX is most
likely musl src/complex/* which dont need it to be constant expr
but does rely on proper semantics for infinites and signed zeros)

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

Am Dienstag, den 25.11.2014, 16:25 +0100 schrieb Szabolcs Nagy:
> * Jens Gustedt <Jens.Gustedt@inria.fr> [2014-11-25 15:49:39 +0100]:
> > +#define __CMPLX(x, y, t) ((t)(x) + _Complex_I*(t)(y))
> >  
> 
> this is only correct if the compiler supports annex g kind of
> imaginary type, otherwise it is incorrect for infinites

even then it would only work by using _Imaginary_I, I think

> _Complex_I*INFINITY == (0 + i)*(inf + i*0) == nan + i*inf
> 
> so imaginary inf will turn into nan real part

right

(but I think it is more import to get initialization of normal complex
constants going, than the border cases with INFINITY and NAN)

to support this it seems that gcc provides __builtin_complex, so we
could implement this subject to some case analysis according to
__builtin_const etc. But since the doc for __builtin_complex
explicitly mentions C11, this probably doesn't exist for older
versions of gcc.

I'll think of it and prepare a new version of this patch

Thanks

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 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

Am Dienstag, den 25.11.2014, 20:21 +0100 schrieb Szabolcs Nagy:
> it seems gcc added __builtin_complex but there is no easy way to check
> for the availability of such builtin (other than checking >= gcc 4.7)

Haven't they planned to implement a feature test tool, similar to
clang's ?

Basically this means that we only can use it in C11 mode, I guess. At
least that one has a feature test macro :)

> i guess we could use the builtin with gcc version check

This always has the possibility of false detection when using one of
the other compilers that simulate gcc behavior. Usually getting this
right for all such compilers is quite a pain. I'll see what clang
offers, here.

> and fall back
> to the compound literal version otherwise (it wont work as static
> initializer then, but right now the biggest user of CMPLX is most
> likely musl src/complex/* which dont need it to be constant expr
> but does rely on proper semantics for infinites and signed zeros)

If that is so, I'd be in favor of having and using an internal macro
that uses the compound literal. The compilation of the library
shouldn't depend too much on compiler versions.

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 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2014-11-25 22:23:03 +0100]:
> right for all such compilers is quite a pain. I'll see what clang
> offers, here.
> 

clang has

 static double _Complex z = (double _Complex){x, y};

which is problematic because it is a language extension that
gcc and other c11 compilers wont support most likely

(a compound literal (T){x,y} is a constraint violation for
scalar T, so a sensible c11 compiler will emit diagnostics)

> > and fall back
> > to the compound literal version otherwise (it wont work as static
> > initializer then, but right now the biggest user of CMPLX is most
> > likely musl src/complex/* which dont need it to be constant expr
> > but does rely on proper semantics for infinites and signed zeros)
> 
> If that is so, I'd be in favor of having and using an internal macro
> that uses the compound literal. The compilation of the library
> shouldn't depend too much on compiler versions.
> 

it used to be an internal macro, but i thought it makes sense
to use the new CMPLX instead when it was added to complex.h

but if CMPLX semantics cannot be guaranteed with c99 features
then probably we should go back to the internal macro (and not
define CMPLX at all in complex.h if the semantics is wrong)

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 3294 bytes --]

Hello,

Am Mittwoch, den 26.11.2014, 00:19 +0100 schrieb Szabolcs Nagy:
> * Jens Gustedt <jens.gustedt@inria.fr> [2014-11-25 22:23:03 +0100]:
> > right for all such compilers is quite a pain. I'll see what clang
> > offers, here.
> > 
> 
> clang has
> 
>  static double _Complex z = (double _Complex){x, y};

yes, I noticed that in the mean time, too

> which is problematic because it is a language extension that
> gcc and other c11 compilers wont support most likely

we'd have to do case analysis according to the compiler, anyhow, since
there will be some time that other compilers implement
__builtin_complex, or which would even be better, _Imaginary.

> (a compound literal (T){x,y} is a constraint violation for
> scalar T, so a sensible c11 compiler will emit diagnostics)
> 
> > > and fall back
> > > to the compound literal version otherwise (it wont work as static
> > > initializer then, but right now the biggest user of CMPLX is most
> > > likely musl src/complex/* which dont need it to be constant expr
> > > but does rely on proper semantics for infinites and signed zeros)
> > 
> > If that is so, I'd be in favor of having and using an internal macro
> > that uses the compound literal. The compilation of the library
> > shouldn't depend too much on compiler versions.
> > 
> 
> it used to be an internal macro, but i thought it makes sense
> to use the new CMPLX instead when it was added to complex.h
> 
> but if CMPLX semantics cannot be guaranteed with c99 features

I suppose that these were added to the standard, because such a
feature needs compiler extensions

> then probably we should go back to the internal macro (and not
> define CMPLX at all in complex.h if the semantics is wrong)

I would much be in favor of that, we shouldn't put the quality of the
library implementation in danger just because of some border cases for
this macro. So I prepare a patch in that sense?

Below you see what I have currently, but I don't like it at all.

Jens


#define __CMPLX_NC(x, y, t) (+(union { _Complex t __z; t __xy[2]; }){.__xy = {(x),(y)}}.__z)

#if defined(_Imaginary_I)
# define __CMPLX_I(x, y, t) ((t)(x) + _Imaginary_I*(t)(y))
#else
# define __CMPLX_I(x, y, t) ((t)(x) + _Complex_I*(t)(y))
#endif

#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 defined(__GNUC__)
# if (__STDC_VERSION__ >= 201112L)
#  define __CMPLX(x, y, t) __builtin_complex((t)(x), (t)(y))
# elif (__GNUC__ >= 3)
#  define __CMPLX(x, y, t) \
  (__builtin_constant_p(x+y) ? __CMPLX_I(x, y, t) : __CMPLX_NC(x, y, t))
# endif
#endif

#ifndef __CMPLX
# define __CMPLX(x, y, t) __CMPLX_I(x, y, t)
#endif

#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)



-- 
:: 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 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 5127 bytes --]

Hi,

Am Mittwoch, den 03.12.2014, 09:38 -0500 schrieb Rich Felker:
> On Wed, Dec 03, 2014 at 10:18:31AM +0100, Jens Gustedt wrote:
> > > #if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
> > > // use clang compound literal hack
> > > #else
> > > // use __builtin_complex
> > > #endif
> > > 
> > > I somewhat prefer the latter since the _Complex_I form is never really
> > > correct (INF/NAN issues) and it gives better fallback behavior
> > > assuming other compilers will implement __builtin_complex but might
> > > not identify themselves as "GNU C >= 4.7". It also produces an error
> > > on compilers that can't give the right behavior for INF/NAN rather
> > > than silently miscompiling the code.
> > 
> > I agree with your analysis, we should avoid using _Complex_I, here.
> > 
> > For the implementation I don't agree completely. Version number games
> > are not the right thing for clang, they don't support this, in a sense
> > in the same way that you don't want a macro that identifies musl.
> 
> OK. I had it commented just as a suggestion that we could do some
> detection for clang versions that need the clang-specific compound
> literal hack if/when there are versions that no longer need it. In any
> case right now it would just be #ifdef __clang__ I think.

so you want to see this changed explicitly, the day they support this?

on that day the correct version will most certainly look like

#ifndef __has_builtin
  #define __has_builtin(x) 0  // Compatibility with non-clang compilers.
#endif

#if __clang__ && !__has_builtin(__builtin_complex)
// use clang compound literal hack
#else
// use __builtin_complex
#endif

so we could have that from the start and not talk about it anymore :)

> > But still, I'd like to have a version with _Imaginary_I, see below.
> 
> I'd like to have it in mind for the future, but putting it in the file
> is confusing since it can't be used without further changes to the
> file.
> 
> > > > > > > _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,
> > > > 
> > > > it is non-conforming
> > > 
> > > Because of the _-prefixed namespace?
> > 
> > Yes, an implementation simply *mustn't* touch any identifier that
> > starts with underscore and capital letter. Period.
> 
> Or other reserved identifiers? I think you're just saying things that
> you're explicitly allowed to #undef are special, and otherwise
> reserved identifiers are not admissible for #undef.

most reserved identifiers are only so, provided you include this or
that header, e.g bool, false and true. The _Something identifiers are
always reserved, they are reserved for language specific features, not
library.

> In any case this isn't a really important matter.

agreed

> > > > But if you want to avoid that branch, I understand. _Imaginary_I would
> > > > be *the* way to solve all of this easily, sigh.
> > > 
> > > Yes, I really wish GCC would support it, especially since Annex G
> > > _requires_ _Imaginary_I, and glibc falsely advertises Annex G support
> > > via the __STDC_IEC_559_COMPLEX__ macro despite not having
> > > _Imaginary_I. But the glibc folks seem to think this is a non-issue
> > > and I haven't seen any interest in fixing it on the GCC side.
> > 
> > argh, I wasn't aware of that, what a mess. yes _Imaginary and all that
> > comes with it is really an important feature of that Annex, e.g
> > functions returning _Imaginary values when known to have such values.
> > 
> > So I keep my claim for a first case version with _Imaginary_I up :)
> > we should encourage compiler implementors to provide this feature.
> > 
> > I looked a bit around and I found that
> >  - Keil supports imaginary
> >  - for Cray compilers 1.0i is pure imaginary and not complex, but they
> >    don't explain exactly what happens if you use this in arithmetic
> 
> Perhaps we should redefine _Complex_I as (0.0f+1.0fi) so that it's
> clearly complex even if the compiler has 1.0fi as a pure-imaginary
> type.

yes, good idea, also it makes it explicit for the eye that this is a
complex value. For me, having a heavily biased math background,
reading 0.0i as complex is still counterintuitive.

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 ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Rich Felker]

On Wed, Dec 03, 2014 at 10:18:31AM +0100, Jens Gustedt wrote:
> Hello,
> 
> Am Dienstag, den 02.12.2014, 17:47 -0500 schrieb Rich Felker:
> > On Tue, Dec 02, 2014 at 11:00:12PM +0100, Jens Gustedt wrote:
> > Then what about something like
> > 
> > #if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
> > // use clang compound literal hack
> > #elif __GNUC__*100+__GNUC_MINOR__ >= 407
> > // use __builtin_complex
> > #else
> > // use _Complex_I with mult and add
> > #endif
> > 
> > or alternatively:
> > 
> > #if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
> > // use clang compound literal hack
> > #else
> > // use __builtin_complex
> > #endif
> > 
> > I somewhat prefer the latter since the _Complex_I form is never really
> > correct (INF/NAN issues) and it gives better fallback behavior
> > assuming other compilers will implement __builtin_complex but might
> > not identify themselves as "GNU C >= 4.7". It also produces an error
> > on compilers that can't give the right behavior for INF/NAN rather
> > than silently miscompiling the code.
> 
> I agree with your analysis, we should avoid using _Complex_I, here.
> 
> For the implementation I don't agree completely. Version number games
> are not the right thing for clang, they don't support this, in a sense
> in the same way that you don't want a macro that identifies musl.

OK. I had it commented just as a suggestion that we could do some
detection for clang versions that need the clang-specific compound
literal hack if/when there are versions that no longer need it. In any
case right now it would just be #ifdef __clang__ I think.

> But still, I'd like to have a version with _Imaginary_I, see below.

I'd like to have it in mind for the future, but putting it in the file
is confusing since it can't be used without further changes to the
file.

> > > > > > _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,
> > > 
> > > it is non-conforming
> > 
> > Because of the _-prefixed namespace?
> 
> Yes, an implementation simply *mustn't* touch any identifier that
> starts with underscore and capital letter. Period.

Or other reserved identifiers? I think you're just saying things that
you're explicitly allowed to #undef are special, and otherwise
reserved identifiers are not admissible for #undef.

In any case this isn't a really important matter.

> > > But if you want to avoid that branch, I understand. _Imaginary_I would
> > > be *the* way to solve all of this easily, sigh.
> > 
> > Yes, I really wish GCC would support it, especially since Annex G
> > _requires_ _Imaginary_I, and glibc falsely advertises Annex G support
> > via the __STDC_IEC_559_COMPLEX__ macro despite not having
> > _Imaginary_I. But the glibc folks seem to think this is a non-issue
> > and I haven't seen any interest in fixing it on the GCC side.
> 
> argh, I wasn't aware of that, what a mess. yes _Imaginary and all that
> comes with it is really an important feature of that Annex, e.g
> functions returning _Imaginary values when known to have such values.
> 
> So I keep my claim for a first case version with _Imaginary_I up :)
> we should encourage compiler implementors to provide this feature.
> 
> I looked a bit around and I found that
>  - Keil supports imaginary
>  - for Cray compilers 1.0i is pure imaginary and not complex, but they
>    don't explain exactly what happens if you use this in arithmetic

Perhaps we should redefine _Complex_I as (0.0f+1.0fi) so that it's
clearly complex even if the compiler has 1.0fi as a pure-imaginary
type.

Rich

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 4154 bytes --]

Hello,

Am Dienstag, den 02.12.2014, 17:47 -0500 schrieb Rich Felker:
> On Tue, Dec 02, 2014 at 11:00:12PM +0100, Jens Gustedt wrote:
> Then what about something like
> 
> #if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
> // use clang compound literal hack
> #elif __GNUC__*100+__GNUC_MINOR__ >= 407
> // use __builtin_complex
> #else
> // use _Complex_I with mult and add
> #endif
> 
> or alternatively:
> 
> #if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
> // use clang compound literal hack
> #else
> // use __builtin_complex
> #endif
> 
> I somewhat prefer the latter since the _Complex_I form is never really
> correct (INF/NAN issues) and it gives better fallback behavior
> assuming other compilers will implement __builtin_complex but might
> not identify themselves as "GNU C >= 4.7". It also produces an error
> on compilers that can't give the right behavior for INF/NAN rather
> than silently miscompiling the code.

I agree with your analysis, we should avoid using _Complex_I, here.

For the implementation I don't agree completely. Version number games
are not the right thing for clang, they don't support this, in a sense
in the same way that you don't want a macro that identifies musl.

They have their feature/builtin test framework, we should use
that. This then would kick in whenever they decide to implement that
builtin.

But still, I'd like to have a version with _Imaginary_I, see below.

> > > > > _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,
> > 
> > it is non-conforming
> 
> Because of the _-prefixed namespace?

Yes, an implementation simply *mustn't* touch any identifier that
starts with underscore and capital letter. Period.

> Per my understanding, it's
> conforming to #undef macros provided by the standard headers in many
> cases.

> Eliminating a function-like macro to get at the underlying
> function is one such usage mentioned in the standard, but I think
> there are others too.

I don't know of many, and these are explicitly named. And all of this
is *not* the same thing as the case above. This concerns e.g "complex"
and "I" which are otherwise not reserved, and not e.g "_Complex_I"
that falls under the same rule as _Imaginary_I.

> > But if you want to avoid that branch, I understand. _Imaginary_I would
> > be *the* way to solve all of this easily, sigh.
> 
> Yes, I really wish GCC would support it, especially since Annex G
> _requires_ _Imaginary_I, and glibc falsely advertises Annex G support
> via the __STDC_IEC_559_COMPLEX__ macro despite not having
> _Imaginary_I. But the glibc folks seem to think this is a non-issue
> and I haven't seen any interest in fixing it on the GCC side.

argh, I wasn't aware of that, what a mess. yes _Imaginary and all that
comes with it is really an important feature of that Annex, e.g
functions returning _Imaginary values when known to have such values.

So I keep my claim for a first case version with _Imaginary_I up :)
we should encourage compiler implementors to provide this feature.

I looked a bit around and I found that
 - Keil supports imaginary
 - for Cray compilers 1.0i is pure imaginary and not complex, but they
   don't explain exactly what happens if you use this in arithmetic

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 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Rich Felker]

On Tue, Dec 02, 2014 at 11:00:12PM +0100, Jens Gustedt wrote:
> Am Dienstag, den 02.12.2014, 14:42 -0500 schrieb Rich Felker:
> > On Tue, Dec 02, 2014 at 08:10:32PM +0100, Jens Gustedt wrote:
> > > > 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.
> 
> you mean it is a bug because the two-value-initializer is a constraint
> violation? It only is a bug that has to be diagnosed if user code is
> using it. Internals as long they are not visible to the user isn't
> covered by this, I think. And this is exactly the reason why the
> macros have been introduced in C11, to hide such cruft from the user's
> eye :)

Well, maybe, if they handle it via warning suppression in the system
header path. But this seems fragile.

> > > > 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 that's then one of the points where they aren't C11 conforming
> when compiled with other compilers

Then what about something like

#if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
// use clang compound literal hack
#elif __GNUC__*100+__GNUC_MINOR__ >= 407
// use __builtin_complex
#else
// use _Complex_I with mult and add
#endif

or alternatively:

#if __clang__ // && __clang_major__*100+__clang_minor__ <= xxxxx
// use clang compound literal hack
#else
// use __builtin_complex
#endif

I somewhat prefer the latter since the _Complex_I form is never really
correct (INF/NAN issues) and it gives better fallback behavior
assuming other compilers will implement __builtin_complex but might
not identify themselves as "GNU C >= 4.7". It also produces an error
on compilers that can't give the right behavior for INF/NAN rather
than silently miscompiling the code.

> > > > _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,
> 
> it is non-conforming

Because of the _-prefixed namespace? Per my understanding, it's
conforming to #undef macros provided by the standard headers in many
cases. Eliminating a function-like macro to get at the underlying
function is one such usage mentioned in the standard, but I think
there are others too.

> > but the standard
> > is fairly direct in saying that complex.h defines the _Imaginary_I
> > macro rather than just having it predefined.
> 
> but it also says
> 
>  » Implementations that define the macro __STDC_NO_COMPLEX__ need not provide
>  » this header nor support any of its facilities.
> 
> so any of the facilities *may* be provided anyhow. (and since
> _Imaginary_I is reserved it is allowed to do that in any case.)
> 
> But if you want to avoid that branch, I understand. _Imaginary_I would
> be *the* way to solve all of this easily, sigh.

Yes, I really wish GCC would support it, especially since Annex G
_requires_ _Imaginary_I, and glibc falsely advertises Annex G support
via the __STDC_IEC_559_COMPLEX__ macro despite not having
_Imaginary_I. But the glibc folks seem to think this is a non-issue
and I haven't seen any interest in fixing it on the GCC side.

Rich

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 6194 bytes --]

Am Dienstag, den 02.12.2014, 14:42 -0500 schrieb Rich Felker:
> On Tue, Dec 02, 2014 at 08:10:32PM +0100, Jens Gustedt wrote:
> > > 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.

you mean it is a bug because the two-value-initializer is a constraint
violation? It only is a bug that has to be diagnosed if user code is
using it. Internals as long they are not visible to the user isn't
covered by this, I think. And this is exactly the reason why the
macros have been introduced in C11, to hide such cruft from the user's
eye :)

> > > 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 that's then one of the points where they aren't C11 conforming
when compiled with other compilers

> > > 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?)

yes

> 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.

ok

> > > _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,

it is non-conforming

> but the standard
> is fairly direct in saying that complex.h defines the _Imaginary_I
> macro rather than just having it predefined.

but it also says

 » Implementations that define the macro __STDC_NO_COMPLEX__ need not provide
 » this header nor support any of its facilities.

so any of the facilities *may* be provided anyhow. (and since
_Imaginary_I is reserved it is allowed to do that in any case.)

But if you want to avoid that branch, I understand. _Imaginary_I would
be *the* way to solve all of this easily, sigh.

> > > 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.

ok, I'll prepare a new version according to that

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 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Rich Felker]

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

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 7729 bytes --]

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 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[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 and now it's
truncated by cast. But without the cast it would not be truncated.
Which behavior is right?

> +#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? I would highly prefer not to
use the clang form since it's a compiler bug that should be fixed in
future clang.

> +#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. 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.

My preference would be if we could just define CMPLX either in terms
of __builtin_complex, and only exposing it in C11 mode. _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. 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. 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.)

Rich

[PATCH 1/4] the CMPLX macros must be usable in initializations of static variables

[Jens Gustedt]

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
+
+#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
 
+#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
 
 #ifdef __cplusplus
 }
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
-- 
1.9.1