Re: [RFC PATCH 4/5] RFC: Two vfprintf implementations (IBM and IEEE 128)

[Tulio Magno Quites Machado Filho <tuliom at linux dot ibm dot com>]

"Gabriel F. T. Gomes" <gabriel@inconstante.eti.br> writes:

> diff --git a/libio/stdio.h b/libio/stdio.h
> index 731f8e56f4..6d6b3e6062 100644
> --- a/libio/stdio.h
> +++ b/libio/stdio.h
> @@ -864,6 +864,15 @@ extern int __overflow (FILE *, int);
>  # include <bits/stdio-ldbl.h>
>  #endif
>
> +/* For platforms where long double had double format, then was converted
> +   to some non-IEEE format, then finally to IEEE binary128 format, add
> +   redirections to the correct implementation of stdio.h functions.  */
> +#include <bits/floatn.h>
> +#if __HAVE_DISTINCT_FLOAT128 && __LDBL_MANT_DIG__ == 113 && \
> +    ! defined __BUILDING_EXTRA_LDBL_FORMAT
> +# include <bits/stdio-ieee128.h>
> +#endif

I think you have to replace '__LDBL_MANT_DIG__ == 113' with
'!__HAVE_FLOAT128_UNLIKE_LDBL'.

If you make that change would you still need to use
__BUILDING_EXTRA_LDBL_FORMAT here?

> diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/Makefile b/sysdeps/ieee754/ldbl-128ibm-compat/Makefile
> new file mode 100644
> index 0000000000..ef04cbb67f
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-128ibm-compat/Makefile
> @@ -0,0 +1,20 @@
> +ifeq ($(subdir),stdio-common)
> +routines += ieee128-printf_fp ieee128-printf_fphex
> +CFLAGS-ieee128-printf_fp.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi
> +CFLAGS-ieee128-printf_fphex.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi
> +
> +routines += ieee128-vfprintf
> +CFLAGS-ieee128-vfprintf.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi
> +
> +tests-internal += test-printf-ieee128
> +CFLAGS-test-printf-ieee128.c := $(filter-out -mlong-double-128, \
> +    $(CFLAGS-test-printf-ieee128.c))

I don't think this is mandatory, but helps someone analyzing the compiler
flags.  Could you add a source comment clarifying this, please?

> +CFLAGS-test-printf-ieee128.c += -mfloat128 -mabi=ieeelongdouble -Wno-psabi
> +
> +tests-internal += test-printf-ibm128
> +CFLAGS-test-printf-ibm128.c := $(filter-out -mlong-double-128, \
> +    $(CFLAGS-test-printf-ibm128.c))

Likewise.

> +__IBM128_REDIR (vfprintf)

Outdated name for the macro?  :-D
Notice there is already __LDBL_REDIR_DECL from misc/sys/cdefs.h to help you
with this.

> diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-vfprintf.c b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-vfprintf.c
> new file mode 100644
> index 0000000000..db9cd79b24
> --- /dev/null
> +++ b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-vfprintf.c
> @@ -0,0 +1,13 @@
> +/* Redefine the names of the functions in vfprintf.c before including it.  */
> +#define _IO_vfprintf_internal	__ieee128__IO_vfprintf_internal
> +#define __printf_fp		__ieee128___printf_fp
> +#define __printf_fphex		__ieee128___printf_fphex
> +#define vfprintf		__ieee128_vfprintf
> +#define _IO_vfprintf		__ieee128__IO_vfprintf

Don't you need to include stdio.h before defining these macros?

> +/* Skip the inclusion of the redirections of *l functions to __ieee128_*
> +   functions during the build of glibc (these redirections would
> +   conflict with the internal redirections to __GL_* symbols).  */
> +#define __BUILDING_EXTRA_LDBL_FORMAT

If you include stdio.h, would you still need this macro?

> diff --git a/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h b/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> index 61ba784f86..78e5d4b496 100644
> --- a/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> +++ b/sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> @@ -5,14 +5,35 @@
>  # error "nldbl-abi.h must define LONG_DOUBLE_COMPAT_VERSION"
>  #endif
>
> +/* On powerpc64le, a third format for long double is supported since the
> +   version described in ldbl-128ibm-compat-abi.h.  */
> +#include <bits/floatn.h>
> +#if __HAVE_DISTINCT_FLOAT128 && __LDBL_MANT_DIG__ == 113

Use !__HAVE_FLOAT128_UNLIKE_LDBL again.

-- 
Tulio Magno