* [RFC PATCH 4/5] RFC: Two vfprintf implementations (IBM and IEEE 128)
2018-05-24 4:36 [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Gabriel F. T. Gomes
@ 2018-05-24 4:36 ` Gabriel F. T. Gomes
2018-05-24 20:38 ` Tulio Magno Quites Machado Filho
2018-05-24 4:36 ` [RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c Gabriel F. T. Gomes
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Gabriel F. T. Gomes @ 2018-05-24 4:36 UTC (permalink / raw)
To: libc-alpha
This RFC exemplifies what I have planned for the *printf and *scanf
family of functions, with respect to the second transition of the long
double format on powerpc64le...
In the light of what's been discussed in a previous thread [1], more
specifically, the explanation about the unnecessary increase in the
number of exported symbols [2] (quotes are indented):
[..] using new *l symbol versions and new exported names for ibm128
functions (which seems to me to increase the number of exported symbols
unnecessarily).
And taking into account that new exported symbols are needed for those
functions that are not part of the TS 18661-3 API:
[..] obsolescent functions with no public *f128 names which shouldn't
get such names, but are part of the *l API so need to be available for
long double in the -mabi=ieeelongdouble case, so you may actually want
to call new __*f128 names instead
I adjusted the implementation that I have been working on (for *printf
functions), so that there are no new exported functions for ibm128.
Instead, there are only new exported functions for binary128 and header
redirections (in stdio.h) from *l functions to these new exports (this
RFC only contains the rework for vfprintf and for the functions it
depends on, i.e.: __printf_fp and __printf_fphex. I haven't reworked
the other functions, that's why I omitted them from this RFC (I hope
that's OK and I believe it even eases the review)).
I tried to add as many comments as possible to the changes, so as to
avoid an overly long commit message. Anyhow, an overview:
* The redirections for user code are provided by the following files:
* libio/stdio.h
* sysdeps/ieee754/ldbl-128ibm-compat/bits/stdio-ieee128.h.
* The following files, which are compiled with -mabi=ieeelongdouble,
redefine function names before including the implementation from
stdio-common:
* sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fp.c
* sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fphex.c
* sysdeps/ieee754/ldbl-128ibm-compat/ieee128-vfprintf.c
* Exported symbols (Versions) and their correct versions
(math_ldbl_opt.h) are defined by these files:
* sysdeps/ieee754/ldbl-128ibm-compat/Versions
* sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
* sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h
* For the remaining files, I don't have any special comments.
Any feedback is highly appreciated.
Thanks,
Gabriel
[1] https://sourceware.org/ml/libc-alpha/2018-05/msg00509.html
[2] https://sourceware.org/ml/libc-alpha/2018-05/msg00519.html
---
libio/stdio.h | 9 ++++
sysdeps/ieee754/ldbl-128ibm-compat/Makefile | 20 +++++++++
sysdeps/ieee754/ldbl-128ibm-compat/Versions | 12 +++++
.../ldbl-128ibm-compat/bits/stdio-ieee128.h | 30 +++++++++++++
.../ieee754/ldbl-128ibm-compat/ieee128-printf_fp.c | 16 +++++++
.../ldbl-128ibm-compat/ieee128-printf_fphex.c | 16 +++++++
.../ieee754/ldbl-128ibm-compat/ieee128-vfprintf.c | 13 ++++++
.../ldbl-128ibm-compat/test-printf-ibm128.c | 1 +
.../ldbl-128ibm-compat/test-printf-ieee128.c | 1 +
.../ldbl-128ibm-compat/test-printf-ldbl-compat.c | 51 ++++++++++++++++++++++
sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h | 27 +++++++++++-
.../powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h | 8 ++++
12 files changed, 202 insertions(+), 2 deletions(-)
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/Makefile
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/Versions
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/bits/stdio-ieee128.h
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fp.c
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fphex.c
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/ieee128-vfprintf.c
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ibm128.c
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c
create mode 100644 sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ldbl-compat.c
create mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h
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
+
__END_DECLS
#endif /* <stdio.h> included. */
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))
+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))
+CFLAGS-test-printf-ibm128.c += -mabi=ibmlongdouble -Wno-psabi
+
+headers += bits/stdio-ieee128.h
+endif
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/Versions b/sysdeps/ieee754/ldbl-128ibm-compat/Versions
new file mode 100644
index 0000000000..ea4e096cce
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/Versions
@@ -0,0 +1,12 @@
+%include <ldbl-128ibm-compat-abi.h>
+%ifndef LDBL_IBM128_VERSION
+% error "ldbl-128ibm-compat-abi.h must define LDBL_IBM128_VERSION"
+%endif
+
+libc {
+ LDBL_IBM128_VERSION {
+ __ieee128___printf_fp;
+ __ieee128__IO_vfprintf;
+ __ieee128_vfprintf;
+ }
+}
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/bits/stdio-ieee128.h b/sysdeps/ieee754/ldbl-128ibm-compat/bits/stdio-ieee128.h
new file mode 100644
index 0000000000..a393c0b209
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/bits/stdio-ieee128.h
@@ -0,0 +1,30 @@
+/* Redirections for stdio functions for -mabi=ieeelongdouble.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _STDIO_H
+# error "Never include <bits/stdio-ibm128.h> directly; use <stdio.h> instead."
+#endif
+
+#include <stdio.h>
+
+#define __IBM128_REDIR(name) \
+ extern __typeof (name) __ieee128_##name; \
+ extern __typeof (name) name __asm (__ASMNAME ("__ieee128_" #name));
+
+__IBM128_REDIR (vfprintf)
+/* To be completed with the other functions. */
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fp.c b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fp.c
new file mode 100644
index 0000000000..813315f8b0
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fp.c
@@ -0,0 +1,16 @@
+/* Redefine the names of the functions in printf_fp.c before including it. */
+#define __printf_fp __ieee128___printf_fp
+#define __printf_fp_l __ieee128___printf_fp_l
+#define ___printf_fp __ieee128____printf_fp
+#define __guess_grouping __ieee128___guess_grouping
+
+/* Use __mpn_extract_float128 since it handles the floating-point
+ parameter as a floating-point value with binary128 format. */
+#define __mpn_extract_long_double __mpn_extract_float128
+
+/* 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
+
+#include <stdio-common/printf_fp.c>
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fphex.c b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fphex.c
new file mode 100644
index 0000000000..e3da570a16
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/ieee128-printf_fphex.c
@@ -0,0 +1,16 @@
+/* Redefine the name of the function in printf_fphex.c before including it. */
+#define __printf_fphex __ieee128___printf_fphex
+
+/* 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
+
+/* Explicitly include ieee754.h from the ldbl-128 directory, since this
+ file is building __printf_fphex for long double with IEEE binary128
+ format (implicitly, this header would be included from ldbl-128ibm,
+ which has definitions for long double with IBM Extended Precision
+ format for floating-point). */
+#include <sysdeps/ieee754/ldbl-128/ieee754.h>
+
+#include <sysdeps/ieee754/ldbl-128/printf_fphex.c>
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
+
+/* 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
+
+#include <stdio-common/vfprintf.c>
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ibm128.c b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ibm128.c
new file mode 100644
index 0000000000..5de4ea3e7f
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ibm128.c
@@ -0,0 +1 @@
+#include <test-printf-ldbl-compat.c>
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c
new file mode 100644
index 0000000000..5de4ea3e7f
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ieee128.c
@@ -0,0 +1 @@
+#include <test-printf-ldbl-compat.c>
diff --git a/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ldbl-compat.c b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ldbl-compat.c
new file mode 100644
index 0000000000..03014507b4
--- /dev/null
+++ b/sysdeps/ieee754/ldbl-128ibm-compat/test-printf-ldbl-compat.c
@@ -0,0 +1,51 @@
+/* Test for the long double variants of *printf functions.
+ Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdarg.h>
+#include <stdint.h>
+#include <stdio.h>
+
+#include <support/check.h>
+
+static void
+do_test_call_varg (FILE *stream, const char *format, ...)
+{
+ va_list args;
+
+ printf ("%20s", "vfprintf: ");
+ va_start (args, format);
+ vfprintf (stream, format, args);
+ va_end (args);
+ printf ("\n");
+}
+
+static int
+do_test (void)
+{
+ long double ld = -1;
+
+ /* Print in decimal notation. */
+ do_test_call_varg (stdout, "%.60Lf", ld);
+
+ /* Print in hexadecimal notation. */
+ do_test_call_varg (stdout, "%.60La", ld);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
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
+# include <ldbl-128ibm-compat-abi.h>
+# ifndef LDBL_IBM128_COMPAT_VERSION
+# error "ldbl-128ibm-compat-abi.h must define LDBL_IBM128_COMPAT_VERSION"
+# endif
+#endif
+
#include <shlib-compat.h>
#define LONG_DOUBLE_COMPAT(lib, introduced) \
SHLIB_COMPAT(lib, introduced, LONG_DOUBLE_COMPAT_VERSION)
-#define long_double_symbol(lib, local, symbol) \
+/* When building long double with IEEE long double format on
+ powerpc64le, exposed the symbols with the version defined in
+ ldbl-128ibm-compat-abi.h. */
+#if __HAVE_DISTINCT_FLOAT128 && __LDBL_MANT_DIG__ == 113
+# define long_double_symbol(lib, local, symbol) \
+ long_double_symbol_1 (lib, local, symbol, LDBL_IBM128_COMPAT_VERSION)
+/* Otherwise, expose the symbols with the version from nldbl-abi.h. */
+#else
+# define long_double_symbol(lib, local, symbol) \
long_double_symbol_1 (lib, local, symbol, LONG_DOUBLE_COMPAT_VERSION)
+#endif
#ifdef SHARED
# define ldbl_hidden_def(local, name) libc_hidden_ver (local, name)
# define ldbl_strong_alias(name, aliasname) \
+ ldbl_strong_alias_x (name, aliasname)
+# define ldbl_strong_alias_x(name, aliasname) \
strong_alias (name, __GL_##name##_##aliasname) \
long_double_symbol (libc, __GL_##name##_##aliasname, aliasname);
# define ldbl_weak_alias(name, aliasname) \
@@ -22,7 +43,9 @@
versioned_symbol (lib, local, symbol, version)
#else
# define ldbl_hidden_def(local, name) libc_hidden_def (name)
-# define ldbl_strong_alias(name, aliasname) strong_alias (name, aliasname)
+# define ldbl_strong_alias(name, aliasname) \
+ ldbl_strong_alias_x (name, aliasname)
+# define ldbl_strong_alias_x(name, aliasname) strong_alias (name, aliasname)
# define ldbl_weak_alias(name, aliasname) weak_alias (name, aliasname)
# ifndef __ASSEMBLER__
/* Note that weak_alias cannot be used - it is defined to nothing
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h
new file mode 100644
index 0000000000..6eb0e72b07
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h
@@ -0,0 +1,8 @@
+/* ABI version for long double switch to IEEE 128-bit floating point..
+ This is used by the Versions and math_ldbl_opt.h files in
+ sysdeps/ieee754/ldbl-128ibm-compat/. It gives the ABI version where
+ long double == ibm128 was replaced with long double == _Float128
+ for libm *l functions and libc functions using long double. */
+
+#define LDBL_IBM128_VERSION GLIBC_2.28
+#define LDBL_IBM128_COMPAT_VERSION GLIBC_2_28
--
2.14.3
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [RFC PATCH 4/5] RFC: Two vfprintf implementations (IBM and IEEE 128)
2018-05-24 4:36 ` [RFC PATCH 4/5] RFC: Two vfprintf implementations (IBM and IEEE 128) Gabriel F. T. Gomes
@ 2018-05-24 20:38 ` Tulio Magno Quites Machado Filho
0 siblings, 0 replies; 12+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-05-24 20:38 UTC (permalink / raw)
To: Gabriel F. T. Gomes, libc-alpha
"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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c
2018-05-24 4:36 [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Gabriel F. T. Gomes
2018-05-24 4:36 ` [RFC PATCH 4/5] RFC: Two vfprintf implementations (IBM and IEEE 128) Gabriel F. T. Gomes
@ 2018-05-24 4:36 ` Gabriel F. T. Gomes
2018-05-24 14:16 ` Tulio Magno Quites Machado Filho
2018-05-24 4:36 ` [RFC PATCH 5/5] RFC: powerpc64le: Convert default long double format to IEEE binary128 Gabriel F. T. Gomes
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Gabriel F. T. Gomes @ 2018-05-24 4:36 UTC (permalink / raw)
To: libc-alpha
In preparation for the transition of the long double format on
powerpc64le, this patch replaces the uses of 'vfprintf' with
'___vfprintf', thus avoiding the need to '#undef vfprintf' before
creating the externally visible symbols. In a future, powerpc64le
specific patch, a new file will define 'vfprintf' to an alternate name
to be used by the long double implementation with binary128 format.
Tested for powerpc64le.
* stdio-common/vfprintf.c (vfprintf): Rename function name to
___vfprintf, which is redefined to _IO_vfprintf or
_IO_vfprintf_internal, depending on whether support for wide or
regular characters is being built.
---
stdio-common/vfprintf.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index ae412e4b84..0ab7ade1d6 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -78,7 +78,7 @@
} while (0)
#ifndef COMPILE_WPRINTF
-# define vfprintf _IO_vfprintf_internal
+# define ___vfprintf _IO_vfprintf_internal
# define CHAR_T char
# define UCHAR_T unsigned char
# define INT_T int
@@ -105,7 +105,7 @@ typedef const char *THOUSANDS_SEP_T;
# define ORIENT if (_IO_vtable_offset (s) == 0 && _IO_fwide (s, -1) != -1)\
return -1
#else
-# define vfprintf _IO_vfwprintf
+# define ___vfprintf _IO_vfwprintf
# define CHAR_T wchar_t
/* This is a hack!!! There should be a type uwchar_t. */
# define UCHAR_T unsigned int /* uwchar_t */
@@ -1235,7 +1235,7 @@ static CHAR_T *group_number (CHAR_T *, CHAR_T *, CHAR_T *, const char *,
/* The function itself. */
int
-vfprintf (FILE *s, const CHAR_T *format, va_list ap)
+___vfprintf (FILE *s, const CHAR_T *format, va_list ap)
{
/* The character used as thousands separator. */
THOUSANDS_SEP_T thousands_sep = 0;
@@ -2321,7 +2321,7 @@ buffered_vfprintf (FILE *s, const CHAR_T *format, va_list args)
#ifndef COMPILE_WPRINTF
result = _IO_vfprintf (hp, format, args);
#else
- result = vfprintf (hp, format, args);
+ result = _IO_vfwprintf (hp, format, args);
#endif
/* Lock stream. */
@@ -2352,7 +2352,6 @@ buffered_vfprintf (FILE *s, const CHAR_T *format, va_list args)
return result;
}
-#undef vfprintf
#ifdef COMPILE_WPRINTF
strong_alias (_IO_vfwprintf, __vfwprintf);
ldbl_weak_alias (_IO_vfwprintf, vfwprintf);
--
2.14.3
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c
2018-05-24 4:36 ` [RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c Gabriel F. T. Gomes
@ 2018-05-24 14:16 ` Tulio Magno Quites Machado Filho
0 siblings, 0 replies; 12+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-05-24 14:16 UTC (permalink / raw)
To: Gabriel F. T. Gomes, libc-alpha
"Gabriel F. T. Gomes" <gabriel@inconstante.eti.br> writes:
> diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
> index ae412e4b84..0ab7ade1d6 100644
> --- a/stdio-common/vfprintf.c
> +++ b/stdio-common/vfprintf.c
> @@ -2321,7 +2321,7 @@ buffered_vfprintf (FILE *s, const CHAR_T *format, va_list args)
> #ifndef COMPILE_WPRINTF
> result = _IO_vfprintf (hp, format, args);
> #else
> - result = vfprintf (hp, format, args);
> + result = _IO_vfwprintf (hp, format, args);
This should be using ___vfprintf.
It won't change the binary, but it's going to reuse the macro defined earliear.
--
Tulio Magno
^ permalink raw reply [flat|nested] 12+ messages in thread
* [RFC PATCH 5/5] RFC: powerpc64le: Convert default long double format to IEEE binary128
2018-05-24 4:36 [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Gabriel F. T. Gomes
2018-05-24 4:36 ` [RFC PATCH 4/5] RFC: Two vfprintf implementations (IBM and IEEE 128) Gabriel F. T. Gomes
2018-05-24 4:36 ` [RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c Gabriel F. T. Gomes
@ 2018-05-24 4:36 ` Gabriel F. T. Gomes
2018-05-24 4:36 ` [RFC PATCH 1/5] powerpc: Move around math-related Implies Gabriel F. T. Gomes
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Gabriel F. T. Gomes @ 2018-05-24 4:36 UTC (permalink / raw)
To: libc-alpha
A commit with this change would be added only when all other changes are
ready. I'm sending this in the RFC in case someone wishes to test the
previous patches.
---
sysdeps/powerpc/powerpc64/le/Implies-before | 1 +
sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist | 3 +++
2 files changed, 4 insertions(+)
diff --git a/sysdeps/powerpc/powerpc64/le/Implies-before b/sysdeps/powerpc/powerpc64/le/Implies-before
index 7c20db4e97..2139f4dae8 100644
--- a/sysdeps/powerpc/powerpc64/le/Implies-before
+++ b/sysdeps/powerpc/powerpc64/le/Implies-before
@@ -1,4 +1,5 @@
# On PowerPC we use the IBM extended long double format.
+ieee754/ldbl-128ibm-compat
ieee754/ldbl-128ibm
ieee754/ldbl-opt
ieee754/dbl-64
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
index 9869feb56b..d495b5852f 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
@@ -2221,3 +2221,6 @@ GLIBC_2.27 wcstof64 F
GLIBC_2.27 wcstof64_l F
GLIBC_2.27 wcstof64x F
GLIBC_2.27 wcstof64x_l F
+GLIBC_2.28 __ieee128___printf_fp F
+GLIBC_2.28 __ieee128__IO_vfprintf F
+GLIBC_2.28 __ieee128_vfprintf F
--
2.14.3
^ permalink raw reply [flat|nested] 12+ messages in thread* [RFC PATCH 1/5] powerpc: Move around math-related Implies
2018-05-24 4:36 [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Gabriel F. T. Gomes
` (2 preceding siblings ...)
2018-05-24 4:36 ` [RFC PATCH 5/5] RFC: powerpc64le: Convert default long double format to IEEE binary128 Gabriel F. T. Gomes
@ 2018-05-24 4:36 ` Gabriel F. T. Gomes
2018-05-24 4:36 ` [RFC PATCH 3/5] Do not declare __mpn_extract_float128 when long double has binary128 format Gabriel F. T. Gomes
2018-05-24 22:01 ` [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Joseph Myers
5 siblings, 0 replies; 12+ messages in thread
From: Gabriel F. T. Gomes @ 2018-05-24 4:36 UTC (permalink / raw)
To: libc-alpha
From: Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
Currently, powerpc, powerpc64, and powerpc64le imply the same set of
subdirectories from sysdeps/ieee754: flt-32, dbl-64, ldbl-128ibm, and
ldbl-opt. In preparation for the transition of the long double format -
from IBM Extended Precision to IEEE 754 128-bits floating-point - on
powerpc64le, this patch splits the shared Implies file into three
separate files (one for each of the powerpc architectures), without
changing their contents. Future patches will modify powerpc64le.
2018-05-11 Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
Gabriel F. T. Gomes <gabriel@inconstante.eti.br>
* sysdeps/powerpc/Implies: Removed. Previous contents copied to...
* sysdeps/powerpc/powerpc32/Implies-after: ... here.
* sysdeps/powerpc/powerpc64/be/Implies-after: ... here.
* sysdeps/powerpc/powerpc64/le/Implies-before: ... and here.
---
sysdeps/powerpc/{Implies => powerpc32/Implies-after} | 0
sysdeps/powerpc/powerpc64/be/Implies-after | 5 +++++
sysdeps/powerpc/powerpc64/le/Implies-before | 5 +++++
3 files changed, 10 insertions(+)
rename sysdeps/powerpc/{Implies => powerpc32/Implies-after} (100%)
create mode 100644 sysdeps/powerpc/powerpc64/be/Implies-after
diff --git a/sysdeps/powerpc/Implies b/sysdeps/powerpc/powerpc32/Implies-after
similarity index 100%
rename from sysdeps/powerpc/Implies
rename to sysdeps/powerpc/powerpc32/Implies-after
diff --git a/sysdeps/powerpc/powerpc64/be/Implies-after b/sysdeps/powerpc/powerpc64/be/Implies-after
new file mode 100644
index 0000000000..78dba9510c
--- /dev/null
+++ b/sysdeps/powerpc/powerpc64/be/Implies-after
@@ -0,0 +1,5 @@
+# On PowerPC we use the IBM extended long double format.
+ieee754/ldbl-128ibm
+ieee754/ldbl-opt
+ieee754/dbl-64
+ieee754/flt-32
diff --git a/sysdeps/powerpc/powerpc64/le/Implies-before b/sysdeps/powerpc/powerpc64/le/Implies-before
index 48065141a9..7c20db4e97 100644
--- a/sysdeps/powerpc/powerpc64/le/Implies-before
+++ b/sysdeps/powerpc/powerpc64/le/Implies-before
@@ -1 +1,6 @@
+# On PowerPC we use the IBM extended long double format.
+ieee754/ldbl-128ibm
+ieee754/ldbl-opt
+ieee754/dbl-64
+ieee754/flt-32
ieee754/float128
--
2.14.3
^ permalink raw reply [flat|nested] 12+ messages in thread* [RFC PATCH 3/5] Do not declare __mpn_extract_float128 when long double has binary128 format
2018-05-24 4:36 [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Gabriel F. T. Gomes
` (3 preceding siblings ...)
2018-05-24 4:36 ` [RFC PATCH 1/5] powerpc: Move around math-related Implies Gabriel F. T. Gomes
@ 2018-05-24 4:36 ` Gabriel F. T. Gomes
2018-05-24 22:01 ` [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Joseph Myers
5 siblings, 0 replies; 12+ messages in thread
From: Gabriel F. T. Gomes @ 2018-05-24 4:36 UTC (permalink / raw)
To: libc-alpha
In the internal header include/gmp.h, __mpn_extract_float128 is declared
whenever the float128 API is being built, but only when long double is
not ABI-compatible with _Float128 (i.e.: when __HAVE_DISTINCT_FLOAT128).
Checking for __HAVE_DISTINCT_FLOAT128 is currently enough for building
glibc for all platforms. However, when powerpc64le adds support for
long double with a third format (binary128), __mpn_extract_float128 will
be used by the implementation of some long double functions, such as
vfprintf/__printf_fp (at a first glance, it could seem illogical to use
a float128 function for long double, but this design choice reduces the
changes to the ABI, by reusing functions that are already exported).
The reuse of this internal function will be achieve (in a later patch)
with the redefinition of __mpn_extract_long_double to
__mpn_extract_float128, similarly to what float128_private.h does.
However, since __mpn_extract_long_double is redefined, it would get
declared twice by include/gmp.h, with mismatching parameters. This
patch adds a check for __HAVE_FLOAT128_UNLIKE_LDBL, which avoids the
second declaration of __mpn_extract_float128.
Tested for powerpc64le.
* include/gmp.h (__mpn_extract_float128): Only declare when
__HAVE_DISTINCT_FLOAT128 and __HAVE_FLOAT128_UNLIKE_LDBL.
---
include/gmp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/gmp.h b/include/gmp.h
index 657c7a0148..aa9299c67e 100644
--- a/include/gmp.h
+++ b/include/gmp.h
@@ -19,7 +19,7 @@ extern mp_size_t __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size,
long double value)
attribute_hidden;
-#if __HAVE_DISTINCT_FLOAT128
+#if __HAVE_DISTINCT_FLOAT128 && __HAVE_FLOAT128_UNLIKE_LDBL
extern mp_size_t __mpn_extract_float128 (mp_ptr res_ptr, mp_size_t size,
int *expt, int *is_neg,
_Float128 value)
--
2.14.3
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le
2018-05-24 4:36 [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Gabriel F. T. Gomes
` (4 preceding siblings ...)
2018-05-24 4:36 ` [RFC PATCH 3/5] Do not declare __mpn_extract_float128 when long double has binary128 format Gabriel F. T. Gomes
@ 2018-05-24 22:01 ` Joseph Myers
2018-05-28 19:55 ` Gabriel F. T. Gomes
5 siblings, 1 reply; 12+ messages in thread
From: Joseph Myers @ 2018-05-24 22:01 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
I'll comment on general design questions around (1) what the new ABIs
should be for IEEE 128-bit long double, (2) how the installed headers
should arrange for those ABIs to be used and (3) how those ABIs should be
implemented.
The general principle is that public APIs that are supported for IBM long
double should be supported just the same for IEEE 128-bit long double.
This in turn requires corresponding ABIs to be included in glibc. Note
that this only applies to ABIs accessible from *public* APIs - generally,
ones used via calling functions or macros that are declared in public
headers with a name not starting with '_'.
For (1), there are several different groups of ABIs that involve long
double in some way: (a) ABIs parametrized by floating-point format, (b)
ABIs parametrized by floating-point type, (c) ABIs not parametrized by
floating-point type, but still involving long double in some way.
(a) has already been discussed - these are ABIs such as __issignaling* and
__*_finite, and corresponding *f128 versions already exist and are in the
implementation namespace, so no new ABI additions are needed to support
IEEE 128-bit long double (beyond maybe the odd special case such as
__scalf128_finite).
(b) has also already been discussed - most of these are libm functions,
and while the *f128 versions already exist, there is still a use for
adding new versions (such as __*ieee128 or __ieee128_*) as aliases of the
*f128 functions, because *f128 aren't in the C11 implementation namespace,
and a few such functions that are somewhat obsolescent (but still
supported as APIs for long double) so don't have *f128 API variants.
There are a few complications to consider there such as whether calls to
gammal for IEEE 128-bit long double can map to the same ABI as lgammal
(more complicated headers, fewer new ABIs), and similarly whether calls to
finitel (and other such legacy classification *functions*) for IEEE
128-bit long double can map to the existing __finitef128 (etc.). There
are also libc functions in group (b), some of which already have *f128
variants (e.g. strtof128) and some of which don't because of obsolescence
but are still supported APIs (e.g. qecvt).
Somewhere between (b) and (c) are the nexttoward functions - parametrized
on a floating-point type but with an argument with is always long double
(and thus needing special variants and redirection logic for IEEE 128-bit
long double - new variants of nexttowardf and nexttoward are needed, while
nexttowardl could be redirected to the same function as nextafterl).
The main point of the present patch series is functions in class (c),
involving long double only implicitly. These are functions in the printf,
scanf *and strfmon* families (including, for example, a few functions such
as printf_size - not just functions with "..." variable arguments or
taking a va_list for such arguments; and note that syslog, for example, is
in the printf family for this purpose). Every such public ABI needs a
corresponding new ABI variant for IEEE 128-bit long double.
Now, the set of such __nldbl_* ABIs for such functions for
-mlong-double-64 is a plausible starting point for identifying relevant
ABIs. But it's only a starting point: there are functions that are
missing __nldbl_* ABIs but should have them, and there are __nldbl_* ABIs
for which the need is questionable.
For the former, as previously discussed, the printf-like argp.h, err.h and
error.h functions, at least, are missing __nldbl_* variants for
-mlong-double-64. For reasons explained below, I think fixing this will
be needed as part of the preparation for adding IEEE 128-bit long double
variants of those functions.
For the latter, for example, your patch 4 adds public ABIs for
__ieee128___printf_fp, __ieee128__IO_vfprintf and __ieee128_vfprintf.
There is certainly no need for __ieee128__IO_vfprintf, since _IO_vfprintf
is not declared in any installed header since we stopped installing
libio.h. But even if _IO_vfprintf were a public API, it *still* wouldn't
need __ieee128__IO_vfprintf, because the semantics of _IO_vfprintf are
exactly those of vfprintf, so just having __ieee128_vfprintf suffices.
And __printf_fp isn't declared in any installed header, a strong
indication that it is not a public API or ABI and so no export of
__ieee128___printf_fp is needed either. (The comment in
stdio-common/Versions on __printf_fp says "functions used in other
libraries", but as far as I can tell it's *not* in fact now used in any
other glibc library - and in any case, being used in another glibc library
would only justify an export at GLIBC_PRIVATE under current practice.)
So the __nldbl_* exports need reviewing to identify which are actually
relevant for new long double formats, and my guess is that
__nldbl__IO_fprintf __nldbl__IO_printf __nldbl__IO_sprintf
__nldbl__IO_sscanf __nldbl__IO_vfprintf __nldbl__IO_vfscanf
__nldbl__IO_vsprintf __nldbl___asprintf __nldbl___printf_fp
__nldbl___strfmon_l __nldbl___vfscanf __nldbl___vsnprintf
__nldbl___vsscanf __nldbl___vstrfmon __nldbl___vstrfmon_l all in fact are
not needed for new formats - either because the non-__nldbl versions
aren't exported at all (__vstrfrmon, __vstrfmon_l - the __nldbl_* versions
exist only for use from libnldbl_nonshared.a, I think), or because the
non-__nldbl versions are not actually public APIs (__printf_fp), or
because the functions exactly alias variants without __ or _IO_ (and with
an __ieee128_ prefix you're automatically in the implementation namespace
and so don't need such aliases).
Now for (2), the installed headers. We have, for example,
libio/bits/stdio-ldbl.h (installed as bits/stdio-ldbl.h), with a series of
__LDBL_REDIR_DECL and __LDBL_REDIR1_DECL calls to redirect particular
functions to __nldbl_*. My claim is that *the same* headers should be
reused, installed under additional conditions, to handle redirections to
__ieee128_*.
* You'd need to have a different definition of __LDBL_REDIR_DECL that adds
the __ieee128_ prefix instead of __nldbl_.
* You'd need to change __LDBL_REDIR1_DECL, so that either it takes both
the -mlong-double-64 and -mabi=ieeelongdouble function names as arguments,
or so that there are two variants (one that gets called with arguments
e.g. (scanf, __isoc99_scanf), in the case where __nldbl_ and __ieee128_
are the appropriate prefixes; one that gets called with arguments e.g.
(strtold, strtod), as at present, where the second argument is the right
name for -mlong-double-64 but __ieee128_ plus the first argument is the
right name for -mabi=ieeelongdouble).
* And you'd need to change a few calls for cases where I've said above to
eliminate ABIs that exist for __nldbl_*, e.g. "__LDBL_REDIR_DECL
(__asprintf)", so that it uses the first new variant of __LDBL_REDIR1_DECL
to generate calls to __nldbl_asprintf / __ieee128_asprintf, because
__ieee128___asprintf wouldn't exist.
* Of course where the present includes of these bits/*ldbl.h headers are
conditional on __LDBL_COMPAT, you'd need separate header conditions for
"compat for long double = double", "long double = IEEE 128" and "*some*
remapping of long double names, which might be either of the above, is in
effect", and to update all the tests of __LDBL_COMPAT in the existing
installed headers.
* And you need a few new bits/*ldbl.h headers for argp.h, err.h and
error.h. Using a shared mechanism for both __nldbl_* and __ieee128_* is
why I said above that fixing the lack of __nldbl_* for those functions is
a prerequisite to be able to add __ieee128_* for them.
I think doing things that way - shared long double redirection headers,
with only the definitions of macros such as __LDBL_REDIR_DECL being
different in the new IEEE 128-bit case - is clearly much cleaner than
having a separate set of redirection headers for the new case. We've had
enough problems in the past with feature test macro tests in *-ldbl.h
getting out of sync with those in the including headers, we don't want to
add a third place that also needs feature test macro handling kept in
sync.
(Of course that would mean you can't have redirections for some but not
all functions in the installed headers as a transitional state, because
such redirections would automatically be present for all or none of the
relevant functions.)
Now for (3), how the __ieee128_* for printf / scanf / strfmon functions
are implemented. As previously discussed, the approach used for many of
the __nldbl_* functions, of setting a TLS variable __no_long_double, is
not a good one to emulate. But that does not necessarily mean the two
sets of functions should be implemented in different ways. The key
question to answer is: is the best way to implement __ieee128_* the same
as or different from the best way to implement __nldbl_*? If they are the
same, changing how __nldbl_* are implemented may well be good preparation
for implementing __ieee128_*.
Specifically, Zack sent a patch series to move away from __no_long_double
to explicit flags passed to internal functions. So if there's some reason
why you think such explicit flags are not the best approach for
implementing __ieee128_*, but are the best approach for __nldbl_* you need
to explain why you think that's the case. Otherwise, if explicit flags
would be appropriate for __ieee128_*, it would seem to make sense to work
on getting Zack's patches into glibc (for example, updating them for your
review comments and reposting them, if he hasn't had time to update them
himself) as preparation for __ieee128_* being able to use such flags.
(I'm presuming you do think the explicit flags are the right approach for
__nldbl_* since you didn't mention otherwise in your reviews of some of
Zack's patches.)
I note that, for example, __printf_fp *already* supports _Float128. So it
really ought not be necessary to build a second copy of it at all, if you
can make vfprintf for IEEE 128-bit long double (however implemented) pass
appropriate data structures for the existing __printf_fp to behave as
required.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le
2018-05-24 22:01 ` [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le Joseph Myers
@ 2018-05-28 19:55 ` Gabriel F. T. Gomes
2018-05-31 21:23 ` Joseph Myers
0 siblings, 1 reply; 12+ messages in thread
From: Gabriel F. T. Gomes @ 2018-05-28 19:55 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
Hi, Joseph,
Thanks for your detailed reply and explanations...
On Thu, 24 May 2018, Joseph Myers wrote:
>Now, the set of such __nldbl_* ABIs for such functions for
>-mlong-double-64 is a plausible starting point for identifying relevant
>ABIs. But it's only a starting point: there are functions that are
>missing __nldbl_* ABIs but should have them, and there are __nldbl_* ABIs
>for which the need is questionable.
>
>For the former, as previously discussed, the printf-like argp.h, err.h and
>error.h functions, at least, are missing __nldbl_* variants for
>-mlong-double-64. For reasons explained below, I think fixing this will
>be needed as part of the preparation for adding IEEE 128-bit long double
>variants of those functions.
>
>For the latter, for example, your patch 4 adds public ABIs for
>__ieee128___printf_fp, __ieee128__IO_vfprintf and __ieee128_vfprintf.
>There is certainly no need for __ieee128__IO_vfprintf, since _IO_vfprintf
>is not declared in any installed header since we stopped installing
>libio.h. But even if _IO_vfprintf were a public API, it *still* wouldn't
>need __ieee128__IO_vfprintf, because the semantics of _IO_vfprintf are
>exactly those of vfprintf, so just having __ieee128_vfprintf suffices.
Yes, indeed. And you have already explained that before, so, sorry about
that (I could try to explain myself saying that I induced myself to error,
because I was thinking of additions to the ABI for the *IBM* format, but
now I finally understood that even in that case there would be no need for
new *_IO_* symbols. Really sorry).
>And __printf_fp isn't declared in any installed header, a strong
>indication that it is not a public API or ABI and so no export of
>__ieee128___printf_fp is needed either. (The comment in
>stdio-common/Versions on __printf_fp says "functions used in other
>libraries", but as far as I can tell it's *not* in fact now used in any
>other glibc library - and in any case, being used in another glibc library
>would only justify an export at GLIBC_PRIVATE under current practice.)
Yes. Sorry about this, too.
>I think doing things that way - shared long double redirection headers,
>with only the definitions of macros such as __LDBL_REDIR_DECL being
>different in the new IEEE 128-bit case - is clearly much cleaner than
>having a separate set of redirection headers for the new case. We've had
>enough problems in the past with feature test macro tests in *-ldbl.h
>getting out of sync with those in the including headers, we don't want to
>add a third place that also needs feature test macro handling kept in
>sync.
>
>(Of course that would mean you can't have redirections for some but not
>all functions in the installed headers as a transitional state, because
>such redirections would automatically be present for all or none of the
>relevant functions.)
OK. I'll probably keep this header (stdio-ieee128.h) in a *local branch*,
because it allows us to progress in smaller sets of functions. In future
patch submissions (before all functions are ready), I'll not send it, but
I could make it available if someone wants to test the functions. Then,
when all functions are ready, we'll send a patch to do as you suggested
(i.e.: one which adjusts bits/stdio-ldbl.h with no extra headers).
>Now for (3), how the __ieee128_* for printf / scanf / strfmon functions
>are implemented. As previously discussed, the approach used for many of
>the __nldbl_* functions, of setting a TLS variable __no_long_double, is
>not a good one to emulate. But that does not necessarily mean the two
>sets of functions should be implemented in different ways. The key
>question to answer is: is the best way to implement __ieee128_* the same
>as or different from the best way to implement __nldbl_*? If they are the
>same, changing how __nldbl_* are implemented may well be good preparation
>for implementing __ieee128_*.
>
>Specifically, Zack sent a patch series to move away from __no_long_double
>to explicit flags passed to internal functions. So if there's some reason
>why you think such explicit flags are not the best approach for
>implementing __ieee128_*, but are the best approach for __nldbl_* you need
>to explain why you think that's the case. Otherwise, if explicit flags
>would be appropriate for __ieee128_*, it would seem to make sense to work
>on getting Zack's patches into glibc (for example, updating them for your
>review comments and reposting them, if he hasn't had time to update them
>himself) as preparation for __ieee128_* being able to use such flags.
>(I'm presuming you do think the explicit flags are the right approach for
>__nldbl_* since you didn't mention otherwise in your reviews of some of
>Zack's patches.)
You presumed correctly, indeed. I think that Zack's proposal is the right
thing to have in the end. However, I also expected that rebuilding these
files with -mabi=ieeelongdouble had the advantage of being potentially
easier to get right, because I wouldn't need to touch anything from the
current implementation. Besides that, I also expect that it will be
faster to get done, because it only adds files to the build, and since this
is useful for POWER9 users, getting it done sooner than later is something
that we need.
Taking this "timing" perspective into account and also that there seems to
be other problems that would need fixing before Zack's proposal can be
fully complete [1], would it be OK to have these functions added as new
implementations in a first step? Afterwards, with "timing" concerns out
of the way, we could work together (if Zack is OK with that) on the
cleanup (for nldbl and ieee128-on-powerpc).
[1] https://sourceware.org/ml/libc-alpha/2018-03/msg00554.html
>I note that, for example, __printf_fp *already* supports _Float128. So it
>really ought not be necessary to build a second copy of it at all, if you
>can make vfprintf for IEEE 128-bit long double (however implemented) pass
>appropriate data structures for the existing __printf_fp to behave as
>required.
For the same reason (getting the new format available sooner than later),
I thought of starting with a copy of vfprintf (adjusted to use a copy of
__printf_fp).
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le
2018-05-28 19:55 ` Gabriel F. T. Gomes
@ 2018-05-31 21:23 ` Joseph Myers
2018-06-01 0:46 ` Zack Weinberg
0 siblings, 1 reply; 12+ messages in thread
From: Joseph Myers @ 2018-05-31 21:23 UTC (permalink / raw)
To: Gabriel F. T. Gomes; +Cc: libc-alpha
On Mon, 28 May 2018, Gabriel F. T. Gomes wrote:
> You presumed correctly, indeed. I think that Zack's proposal is the right
> thing to have in the end. However, I also expected that rebuilding these
> files with -mabi=ieeelongdouble had the advantage of being potentially
> easier to get right, because I wouldn't need to touch anything from the
> current implementation. Besides that, I also expect that it will be
> faster to get done, because it only adds files to the build, and since this
> is useful for POWER9 users, getting it done sooner than later is something
> that we need.
>
> Taking this "timing" perspective into account and also that there seems to
> be other problems that would need fixing before Zack's proposal can be
> fully complete [1], would it be OK to have these functions added as new
> implementations in a first step? Afterwards, with "timing" concerns out
> of the way, we could work together (if Zack is OK with that) on the
> cleanup (for nldbl and ieee128-on-powerpc).
>
> [1] https://sourceware.org/ml/libc-alpha/2018-03/msg00554.html
I don't think tests for hidden annotation issues (or fixes for existing
such issues) are needed in order to make the move to explicit flags here;
the patches just need reviewing / updating to follow the rules described
at <https://sourceware.org/ml/libc-alpha/2018-03/msg00552.html> for any
new internal functions they add. I'm also not convinced that building
files twice, and making sure that all calls within those files end up
pointed to the new ieee128 internal functions when needed, is any easier
to get right than passing explicit flags, and making sure that calls
within those files call appropriate other internal functions that take
those flags - especially given that Zack's patches already do most of
what's required regarding passing those flags around (new work for argp.h
/ err.h / error.h printf-like functions being needed in any case).
So rather than going down the dead-end route of building the files twice
and so complicating the subsequent cleanup to use explicit flags, I think
the support for explicit flags (along the lines of Zack's patches) should
be added first. That is, first in the commit sequence on master. You
could of course maintain multiple public branches, one with the latest
version of Zack's patches and one with ieee128 patches built on top of
that, working on them in parallel and posting for review patches from each
branch as and when they are ready, with patches from the ieee128 branch
not actually being able to go on master until all the changes they depend
on are also in master.
Some changes could be independent of both of the above to some extent -
for example, the argp.h / err.h / error.h ldbl-opt support, or the header
changes to support calling appropriate ieee128 functions on the basis that
the actual changes defining new macros to activate the new header code for
powerpc64le would go in last of all along with the actual addition of all
the new functions to the ABI. So there could be rather more than two
branches involved while development is active, all being developed and
having patches posted, reviewed and revised in parallel.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le
2018-05-31 21:23 ` Joseph Myers
@ 2018-06-01 0:46 ` Zack Weinberg
0 siblings, 0 replies; 12+ messages in thread
From: Zack Weinberg @ 2018-06-01 0:46 UTC (permalink / raw)
To: Joseph Myers; +Cc: Gabriel F. T. Gomes, GNU C Library
On Thu, May 31, 2018 at 5:23 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 28 May 2018, Gabriel F. T. Gomes wrote:
>
>> You presumed correctly, indeed. I think that Zack's proposal is the right
>> thing to have in the end.
...
> I don't think tests for hidden annotation issues (or fixes for existing
> such issues) are needed in order to make the move to explicit flags here;
> the patches just need reviewing / updating to follow the rules described
> at <https://sourceware.org/ml/libc-alpha/2018-03/msg00552.html> for any
> new internal functions they add.
For the record, I might not get to it but it is my intention to update
my __no_long_double removal patchset this weekend, adding appropriate
hidden annotations and addressing any other review comments that are
still outstanding.
I did write some tests to expose existing issues with missing hidden
annotations; the problem was they exposed a _lot_ of missing hidden
annotations, and fixing them all, well, that rabbit hole bottomed out
at Adhemerval's thread cancellation patches!
zw
^ permalink raw reply [flat|nested] 12+ messages in thread