powerpc: Fix the compiler mode used with C++ when -mabi=ieeelongdouble

  Joseph Myers <joseph@codesourcery.com> writes:

> On Fri, 27 Apr 2018, Tulio Magno Quites Machado Filho wrote:
>
>> When compiling C++  code with -mabi=ieeelongdouble, KCtype is
>> unavailable and TCtype should be used instead.
>
> In that case (-mabi=ieeelongdouble), I'd expect you to use "typedef long 
> double _Float128;" for C++, and _Complex long double as the definition of 
> __CFLOAT128 for C++, and x##l as the definition of __f128 for C++, as it's 
> essentially the case implemented by 
> sysdeps/ieee754/ldbl-128/bits/floatn.h.  You shouldn't need to use mode 
> attributes at all.

Agreed.

> (Uses of __HAVE_DISTINCT_FLOAT128 and __HAVE_FLOAT64X_LONG_DOUBLE will 
> need careful review to see if additional macros are needed to cover this 
> -mabi=ieeelongdouble case, but the general rule is that they relate to the 
> *default* long double type for that glibc binary - meaning the one with 
> __*l symbols, as selection of such implementation-namespace one-per-format 
> symbols is generally what they are used for in installed headers - meaning 
> the existing definitions for powerpc remain correct even with 
> -mabi=ieeelongdouble and you should not copy the 
> sysdeps/ieee754/ldbl-128/bits/floatn.h definitions of those macros.)

Ack.

I haven't seen requirement for additional macros yet, but when _Float128 is
typedef'ed to long double, the following changes are also necessary:


Is it architecturally-agnostic OK?
  

Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com> writes:

> [ text/plain ]
> Joseph Myers <joseph@codesourcery.com> writes:
>
>> On Fri, 27 Apr 2018, Tulio Magno Quites Machado Filho wrote:
>>
>>> When compiling C++  code with -mabi=ieeelongdouble, KCtype is
>>> unavailable and TCtype should be used instead.
>>
>> (Uses of __HAVE_DISTINCT_FLOAT128 and __HAVE_FLOAT64X_LONG_DOUBLE will 
>> need careful review to see if additional macros are needed to cover this 
>> -mabi=ieeelongdouble case, but the general rule is that they relate to the 
>> *default* long double type for that glibc binary - meaning the one with 
>> __*l symbols, as selection of such implementation-namespace one-per-format 
>> symbols is generally what they are used for in installed headers - meaning 
>> the existing definitions for powerpc remain correct even with 
>> -mabi=ieeelongdouble and you should not copy the 
>> sysdeps/ieee754/ldbl-128/bits/floatn.h definitions of those macros.)
>
> Ack.
>
> I haven't seen requirement for additional macros yet, but when _Float128 is
> typedef'ed to long double, the following changes are also necessary:

I should have said "kind of changes".  We have to make the same changes to a
few other places in math/math.h.

> diff --git a/math/math.h b/math/math.h
> index 9b6cdce431..1f1ae6014f 100644
> --- a/math/math.h
> +++ b/math/math.h
> @@ -1025,7 +1025,11 @@ issignaling (long double __val)
>    return __issignalingl (__val);
>  #  endif
>  }
> -#  if __HAVE_DISTINCT_FLOAT128
> +#  if __HAVE_DISTINCT_FLOAT128 \
> +      && (!defined __cplusplus \
> +	  || defined __cplusplus && __LDBL_MANT_DIG__ != 113)
> +/* When using an IEEE 128-bit long double, _Float128 is defined as long double
> +   in C++.  */
>  inline int issignaling (_Float128 __val) { return __issignalingf128 (__val); }
>  #  endif
>  } /* extern C++ */
>
> Is it architecturally-agnostic OK?
  

On Mon, 30 Apr 2018, Tulio Magno Quites Machado Filho wrote:

> I haven't seen requirement for additional macros yet, but when _Float128 is
> typedef'ed to long double, the following changes are also necessary:
> 
> diff --git a/math/math.h b/math/math.h
> index 9b6cdce431..1f1ae6014f 100644
> --- a/math/math.h
> +++ b/math/math.h
> @@ -1025,7 +1025,11 @@ issignaling (long double __val)
>    return __issignalingl (__val);
>  #  endif
>  }
> -#  if __HAVE_DISTINCT_FLOAT128
> +#  if __HAVE_DISTINCT_FLOAT128 \
> +      && (!defined __cplusplus \
> +	  || defined __cplusplus && __LDBL_MANT_DIG__ != 113)
> +/* When using an IEEE 128-bit long double, _Float128 is defined as long double
> +   in C++.  */
>  inline int issignaling (_Float128 __val) { return __issignalingf128 (__val); }
>  #  endif
>  } /* extern C++ */
> 
> Is it architecturally-agnostic OK?

I think that suggests the need for a new macro.

__HAVE_DISTINCT_FLOAT128 means that _Float128 has a different format from 
the *default* long double in this glibc - so that it's necessary to call 
__issignalingf128 rather than __issignalingl for a _Float128 argument, for 
example (if the format is the same as the default long double, 
__issignalingf128 doesn't exist, on the principle of having such 
implementation-namespace functions only exported once per floating-point 
format, not once per floating-point type).

In this C++ case (and this whole piece of code is only for C++, so 
__cplusplus conditionals certainly aren't needed within there), what 
you've found is that you want a conditional for whether _Float128 is 
different from long double for the present compilation - not from the 
default long double.

So that indicates having a new macro meaning that _Float128 exists and is 
different in format from long double for the present compilation, which 
could be defined in bits/floatn-common.h (based on 
__HAVE_DISTINCT_FLOAT128 and __LDBL_MANT_DIG__) rather than needing 
duplicating in different bits/floatn.h files.

Then, this would need using for issignaling and iszero and iseqsig in 
math.h.  Those C++ definitions would *also* need changes to the inlines 
for long double, because those inlines call either the unsuffixed or 
l-suffixed functions depending on __NO_LONG_DOUBLE_MATH, but would need to 
be able to call the f128-suffixed functions in the new case where long 
double uses such functions.  So maybe some new macro would be needed to 
indicate that case (and would then no doubt be used in other cases such as 
to control which functions are used by bits/math-finite.h for long 
double).

Something would also need doing for iscanonical, where 
sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h would need to know that in 
the case where long double is binary128, the same trivial definition used 
for __NO_LONG_DOUBLE_MATH suffices (presuming you don't try to support 
__ibm128 arguments to these macros in the case where long double is 
binary128).

The definitions of __MATH_TG using _Generic may also need updating to 
handle this case, since the uses in math.h are to select such 
once-per-format functions and thus long double selecting l-suffixed 
functions is inappropriate for -mabi=ieeelongdouble.  (The definitions 
using __builtin_types_compatible_p should only be applicate with old 
compilers with insufficient -mabi=ieeelongdouble support, so you probably 
don't need to change those.)  Note however that __MATH_TG is used in 
math_private.h in a way for which the types used in the current 
compilation are the relevant ones - so if any bits of glibc using 
math_private.h get built with -mabi=ieeelongdouble, you have an issue with 
different __MATH_TG definitions being appropriate for the different uses.
  

Joseph Myers <joseph@codesourcery.com> writes:

> On Mon, 30 Apr 2018, Tulio Magno Quites Machado Filho wrote:
>
>> I haven't seen requirement for additional macros yet, but when _Float128 is
>> typedef'ed to long double, the following changes are also necessary:
>> 
>> diff --git a/math/math.h b/math/math.h
>> index 9b6cdce431..1f1ae6014f 100644
>> --- a/math/math.h
>> +++ b/math/math.h
>> @@ -1025,7 +1025,11 @@ issignaling (long double __val)
>>    return __issignalingl (__val);
>>  #  endif
>>  }
>> -#  if __HAVE_DISTINCT_FLOAT128
>> +#  if __HAVE_DISTINCT_FLOAT128 \
>> +      && (!defined __cplusplus \
>> +	  || defined __cplusplus && __LDBL_MANT_DIG__ != 113)
>> +/* When using an IEEE 128-bit long double, _Float128 is defined as long double
>> +   in C++.  */
>>  inline int issignaling (_Float128 __val) { return __issignalingf128 (__val); }
>>  #  endif
>>  } /* extern C++ */
>> 
>> Is it architecturally-agnostic OK?
>
> I think that suggests the need for a new macro.
>
> __HAVE_DISTINCT_FLOAT128 means that _Float128 has a different format from 
> the *default* long double in this glibc - so that it's necessary to call 
> __issignalingf128 rather than __issignalingl for a _Float128 argument, for 
> example (if the format is the same as the default long double, 
> __issignalingf128 doesn't exist, on the principle of having such 
> implementation-namespace functions only exported once per floating-point 
> format, not once per floating-point type).

OK.

> In this C++ case (and this whole piece of code is only for C++, so 
> __cplusplus conditionals certainly aren't needed within there),

Aaargh!

> what you've found is that you want a conditional for whether _Float128 is 
> different from long double for the present compilation - not from the 
> default long double.
>
> So that indicates having a new macro meaning that _Float128 exists and is 
> different in format from long double for the present compilation, which 
> could be defined in bits/floatn-common.h (based on 
> __HAVE_DISTINCT_FLOAT128 and __LDBL_MANT_DIG__) rather than needing 
> duplicating in different bits/floatn.h files.

Agreed.
We need a good name and explanation to avoid the confusion with
__HAVE_DISTINCT_FLOAT*.

What about this?

/* Defined to 1 if the corresponding _FloatN type is not binary compatible
   with the corresponding ISO C type.  */
#define __HAVE_FLOAT128_UNLIKE_LDBL __HAVE_DISTINCT_FLOAT128 \
				    && __LDBL_MANT_DIG__ != 113

> Then, this would need using for issignaling and iszero and iseqsig in 
> math.h.  Those C++ definitions would *also* need changes to the inlines 
> for long double, because those inlines call either the unsuffixed or 
> l-suffixed functions depending on __NO_LONG_DOUBLE_MATH, but would need to 
> be able to call the f128-suffixed functions in the new case where long 
> double uses such functions.  So maybe some new macro would be needed to 
> indicate that case (and would then no doubt be used in other cases such as 
> to control which functions are used by bits/math-finite.h for long 
> double).

Ack.

> Something would also need doing for iscanonical, where 
> sysdeps/ieee754/ldbl-128ibm/bits/iscanonical.h would need to know that in 
> the case where long double is binary128, the same trivial definition used 
> for __NO_LONG_DOUBLE_MATH suffices (presuming you don't try to support 
> __ibm128 arguments to these macros in the case where long double is 
> binary128).

OK.

> The definitions of __MATH_TG using _Generic may also need updating to 
> handle this case, since the uses in math.h are to select such 
> once-per-format functions and thus long double selecting l-suffixed 
> functions is inappropriate for -mabi=ieeelongdouble.  (The definitions 
> using __builtin_types_compatible_p should only be applicate with old 
> compilers with insufficient -mabi=ieeelongdouble support, so you probably 
> don't need to change those.)  Note however that __MATH_TG is used in 
> math_private.h in a way for which the types used in the current 
> compilation are the relevant ones - so if any bits of glibc using 
> math_private.h get built with -mabi=ieeelongdouble, you have an issue with 
> different __MATH_TG definitions being appropriate for the different uses.

Ack.
  

On Fri, 4 May 2018, Tulio Magno Quites Machado Filho wrote:

> /* Defined to 1 if the corresponding _FloatN type is not binary compatible
>    with the corresponding ISO C type.  */
> #define __HAVE_FLOAT128_UNLIKE_LDBL __HAVE_DISTINCT_FLOAT128 \
> 				    && __LDBL_MANT_DIG__ != 113

Yes, something like that (but the comment should say explicitly this is 
about the types in the current compilation rather than the default types 
for this glibc, and that that's how this is distinguished from 
__HAVE_DISTINCT_FLOAT128 - and the expansion should be in parentheses).