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

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

[Gabriel F. T. Gomes]

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

[RFC PATCH 1/5] powerpc: Move around math-related Implies

[Gabriel F. T. Gomes]

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

[RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c

[Gabriel F. T. Gomes]

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

[RFC PATCH 3/5] Do not declare __mpn_extract_float128 when long double has binary128 format

[Gabriel F. T. Gomes]

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

[RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le

[Gabriel F. T. Gomes]

Each RFC patch in this thread has its own comments, but I'd like to
point out that:

  * Patch 1/5 is currently under review on libc-alpha, but I added it to
    this series, because it makes the series usable (testable).

  * Patches 2/5 and 3/5 are new cleanups (required by the next patch).

  * Patch 4/5 is the bulk of the RFC.

  * Patch 5/5 is there just to allow people to test the series.



Gabriel F. T. Gomes (4):
  Do not define and undefine vfprintf in vfprintf.c
  Do not declare __mpn_extract_float128 when long double has binary128
    format
  RFC: Two vfprintf implementations (IBM and IEEE 128)
  RFC: powerpc64le: Convert default long double format to IEEE binary128

Tulio Magno Quites Machado Filho (1):
  powerpc: Move around math-related Implies

 include/gmp.h                                      |  2 +-
 libio/stdio.h                                      |  9 ++++
 stdio-common/vfprintf.c                            |  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/{Implies => powerpc32/Implies-after}   |  0
 sysdeps/powerpc/powerpc64/be/Implies-after         |  5 +++
 sysdeps/powerpc/powerpc64/le/Implies-before        |  6 +++
 .../powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h  |  8 ++++
 .../sysv/linux/powerpc/powerpc64/libc-le.abilist   |  3 ++
 18 files changed, 221 insertions(+), 8 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
 rename sysdeps/powerpc/{Implies => powerpc32/Implies-after} (100%)
 create mode 100644 sysdeps/powerpc/powerpc64/be/Implies-after
 create mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc64/le/ldbl-128ibm-compat-abi.h

-- 
2.14.3

[RFC PATCH 5/5] RFC: powerpc64le: Convert default long double format to IEEE binary128

[Gabriel F. T. Gomes]

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

Re: [RFC PATCH 2/5] Do not define and undefine vfprintf in vfprintf.c

[Tulio Magno Quites Machado Filho]

"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

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

[Tulio Magno Quites Machado Filho]

"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

Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le

[Joseph Myers]

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

Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le

[Gabriel F. T. Gomes]

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

Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le

[Joseph Myers]

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

Re: [RFC PATCH 0/5] *printf/*scanf functions for the long double migration on powerpc64le

[Zack Weinberg]

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