Re: [PATCH v2 4/8] float128: Add public _Float128 declarations to libm.

[Joseph Myers <joseph at codesourcery dot com>]

On Fri, 28 Apr 2017, Gabriel F. T. Gomes wrote:

> diff --git a/bits/floatn.h b/bits/floatn.h
> new file mode 100644
> index 0000000..abc39cb
> --- /dev/null
> +++ b/bits/floatn.h
> @@ -0,0 +1,30 @@
> +/* Macros to control TS 18661-3 glibc features.
> +   Copyright (C) 2016 Free Software Foundation, Inc.

Everything in any of these patches needs to include 2017 in copyright 
dates.

> +/* Defined if the current compiler invocation supports _Float128 or
> +   __float128 as an additional floating-point type.  */
> +#define __HAVE_FLOAT128 0
> +
> +/* Defined if support for _Float128 is present.  This implies
> +   support for ISO/IEC TS-18661-3, meaning that architectures that
> +   define this macro use __GLIBC_USE (IEC_60559_TYPES_EXT).  */
> +#define __USE_FLOAT128 0

I still don't follow what the exact semantics of the macros are meant to 
be.

What's "additional" for types?  Is the case of _Float128 being 
ABI-compatible with long double included or not?  And what does "support 
for _Float128 is present" mean?

I think the following questions might be answered by such macros, and it 
must be completely unambiguous from the comments on the default 
definitions exactly which question or questions each macro relates to.

(a) Does the compiler support a particular type name (such as _Float128)?

(b) Does the compiler support some name for a type that is ABI-compatible 
with _Float128 (the name possibly being __float128 or long double)?

(c) Does this glibc build include support for _Float128 (that is, *f128 
functions / aliases, and all the associated header pieces)?  If 
ABI-distinct, the headers *should* care about this even when the functions 
aren't going to be declared, because of the type-generic <math.h> macros 
that might need to call __isinff128 etc.

(d) Is such a type, given that it is supported, ABI-distinct from the 
default float, double and long double in the glibc ABI?  Headers care 
about this because __isinff128 won't exist if it's not ABI-distinct.  But 
for your purposes you could hardcode the answer that this is "yes" 
whenever (c) is true.

(e) Should this compilation expose declarations for the *f128 interfaces?  
I think it's best for this to be represented as ((b) && (c)) && 
__GLIBC_USE (IEC_60559_TYPES_EXT), rather than having its own macro.

It looks rather like you're using __USE_FLOAT128 as (e), which as noted I 
don't think should have its own macro at all, and __HAVE_FLOAT128 as (d) 
or (b) && (d).  ((b) && (d) is the condition for including 
bits/mathcalls-helper-functions.h, (b) && (e) is the condition for 
including bits/mathcalls.h.)

So define macros for compiler+library support for _Float128 ((b) && (c)), 
and compiler+library support for ABI-distinct _Float128 ((b) && (d)), both 
of those macros admitting the possibility that the name might be different 
- the macro for ((b) && (c)) would be used in conjunction with __GLIBC_USE 
(IEC_60559_TYPES_EXT) to determine whether to expose public declarations, 
the macro for ((b) && (d)) would determine whether to include 
bits/mathcalls-helper-functions.h and how __MATH_TG is defined.  The 
compiler+ part is relevant because you don't want to try declaring the 
functions etc. if the compiler doesn't support the types at all.  (The 
compiler used to build glibc must support them, possibly under another 
name, but the user's compiler may be older.)

> diff --git a/bits/huge_val_flt128.h b/bits/huge_val_flt128.h

> +#ifdef __USE_FLOAT128
> +# define HUGE_VAL_F128 (__builtin_huge_valf128 ())
> +#endif

I'd expect exactly one of math.h and this header to have the ((b) && (c)) 
&& __GLIBC_USE (IEC_60559_TYPES_EXT) conditional.  Probably math.h as 
that's the existing practice for bits/huge_vall.h.  You currently have 
conditionals in both places.

> +#if __USE_FLOAT128 && __USE_GNU

__USE_GNU needs to be tested with "defined", it's a defined/undefined 
macro not 1/0.

> diff --git a/sysdeps/powerpc/powerpc64le/bits/floatn-compat.h b/sysdeps/powerpc/powerpc64le/bits/floatn-compat.h
> new file mode 100644
> index 0000000..5ecadec
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64le/bits/floatn-compat.h

> +#  define L(x) x##Q

"L" is in the user's namespace and should not go in a public header.

> +/* Builtin __builtin_huge_val is only type-generic since GCC 7.0.  */

It's never type-generic.  Type-generic built-in functions are only 
type-generic on argument types (they all have int return type).  You mean 
something else, such as that __builtin_huge_valf128 doesn't exist before 
GCC 7.

-- 
Joseph S. Myers
joseph@codesourcery.com