[PATCH v2 1/8] Use STRFMON_LDBL_IS_DBL instead of __ldbl_is_dbl.

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

From: Zack Weinberg <zackw@panix.com>

Changed since v1:

  - Added attribute hidden to __vstrfmon_l_internal.
    On powerpc, code in the call sites changed:
    From:
      $ objdump -d --reloc VISIBLE-glibc/libc.so.6 | grep __nldbl___vstrfmon_l -A 13
      001502a0 <__nldbl___vstrfmon_l>:
        1502a0:       94 21 ff f0     stwu    r1,-16(r1)
        1502a4:       39 00 00 01     li      r8,1
        1502a8:       7c 08 02 a6     mflr    r0
        1502ac:       42 9f 00 05     bcl     20,4*cr7+so,1502b0 <__nldbl___vstrfmon_l+0x10>
        1502b0:       93 c1 00 08     stw     r30,8(r1)
        1502b4:       90 01 00 14     stw     r0,20(r1)
        1502b8:       7f c8 02 a6     mflr    r30
        1502bc:       3f de 00 05     addis   r30,r30,5
        1502c0:       3b de fd 44     addi    r30,r30,-700
        1502c4:       4b ef b6 5d     bl      4b920 <__vstrfmon_l_internal>
        1502c8:       80 01 00 14     lwz     r0,20(r1)
        1502cc:       83 c1 00 08     lwz     r30,8(r1)
        1502d0:       38 21 00 10     addi    r1,r1,16
        1502d4:       7c 08 03 a6     mtlr    r0
        1502d8:       4e 80 00 20     blr
        1502dc:       60 00 00 00     nop
    To:
      $ objdump -d --reloc HIDDEN-glibc/libc.so.6 | grep __nldbl___vstrfmon_l -A 5
      00150260 <__nldbl___vstrfmon_l>:
        150260:       39 00 00 01     li      r8,1
        150264:       4b ef b6 bc     b       4b920 <__vstrfmon_l_internal>
        150268:       60 00 00 00     nop
        15026c:       60 00 00 00     nop
  - Replaced blocks of eight spaces with tabs.
  - Added signed-off-by field with Zack's and mine names.
  - Extended commit message and comments.

-- 8< --
On platforms where long double used to have the same format as double,
but later switched to a different format (alpha, s390, sparc, and
powerpc), accessing the older behavior is possible and it happens via
__nldbl_* functions (not on the API, but accessible from header
redirection and from compat symbols).  These functions write to the
global flag __ldbl_is_dbl, which tells other functions that long double
variables should be handled as double.  This patch takes the first step
towards removing this global flag and creates __vstrfmon_l_internal,
which takes an explicit flags parameter.

This change arguably makes the generated code slightly worse on
architectures where __ldbl_is_dbl is never true; right now, on those
architectures, it's a compile-time constant; after this change, the
compiler could theoretically prove that __vstrfmon_l_internal was
never called with a nonzero flags argument, but it would probably need
LTO to do it.  This is not performance critical code and I tend to
think that the maintainability benefits of removing action at a
distance are worth it.  However, we _could_ wrap the runtime flag
check with a macro that was defined to ignore its argument and always
return false on architectures where __ldbl_is_dbl is never true, if
people think the codegen benefits are important.

Tested for powerpc and powerpc64le.

2018-10-11  Zack Weinberg  <zackw@panix.com>
	    Gabriel F. T. Gomes  <gabriel@inconstante.eti.br>

	* include/monetary.h (STRFMON_LDBL_IS_DBL): New constant.
	(__vstrfmon_l): Rename to __vstrfmon_l_internal and add flags
	argument.
	* stdlib/strfmon_l.c (__vstrfmon_l): Rename to __vstrfmon_l_internal
	and add flags argument.	 Check flags instead of __ldbl_is_dbl when
	deciding whether to set is_long_double.
	(__strfmon_l): Call __vstrfmon_l_internal instead of __vstrfmon_l,
	passing zero for flags argument.
	* stdlib/strfmon.c (strfmon): Same change as made to __strfmon_l.

	* sysdeps/ieee754/ldbl-opt/nldbl-compat.c
	(__nldbl___vstrfmon, __nldbl___vstrfmon_l)
	(__nldbl_strfmon, __nldbl___strfmon_l): Call __vstrfmon_l_internal
	directly, passing STRFMON_LDBL_IS_DBL for flags argument.  Normalize
	variable names.  Remove libc_hidden_def/libc_hidden_proto from
	__nldbl___vstrfmon and __nldbl___vstrfmon_l, because they are no
	longer called from within the library.
	* sysdeps/ieee754/ldbl-opt/nldbl-compat.h: Don't use NLDBL_DECL
	for __nldbl___vstrfmon_l, declare it explicitly.

	* manual/locale.texi: Update a reference to vstrfmon_l in comments.

Signed-off-by: Zack Weinberg <zackw@panix.com>
Signed-off-by: Gabriel F. T. Gomes <gabriel@inconstante.eti.br>
---
 include/monetary.h                      | 12 +++++++++---
 manual/locale.texi                      |  9 +++++----
 stdlib/strfmon.c                        |  3 ++-
 stdlib/strfmon_l.c                      |  8 ++++----
 sysdeps/ieee754/ldbl-opt/nldbl-compat.c | 34 ++++++++++++---------------------
 sysdeps/ieee754/ldbl-opt/nldbl-compat.h |  8 +++++---
 6 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/include/monetary.h b/include/monetary.h
index c130ed56a3..a226305adf 100644
--- a/include/monetary.h
+++ b/include/monetary.h
@@ -2,7 +2,13 @@
 #ifndef _ISOMAC
 #include <stdarg.h>
 
-extern ssize_t __vstrfmon_l (char *s, size_t maxsize, locale_t loc,
-			     const char *format, va_list ap)
-     attribute_hidden;
+extern ssize_t
+__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
+		       const char *format, va_list ap,
+		       unsigned int flags)
+  attribute_hidden;
+
+/* Flags for __vstrfmon_l_internal.  */
+#define STRFMON_LDBL_IS_DBL 0x0001
+
 #endif
diff --git a/manual/locale.texi b/manual/locale.texi
index dabb959f9e..720e0ca952 100644
--- a/manual/locale.texi
+++ b/manual/locale.texi
@@ -1209,10 +1209,11 @@ numbers according to these rules.
 
 @deftypefun ssize_t strfmon (char *@var{s}, size_t @var{maxsize}, const char *@var{format}, @dots{})
 @safety{@prelim{}@mtsafe{@mtslocale{}}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
-@c It (and strfmon_l) both call vstrfmon_l, which, besides accessing the
-@c locale object passed to it, accesses the active locale through
-@c isdigit (but to_digit assumes ASCII digits only).  It may call
-@c __printf_fp (@mtslocale @ascuheap @acsmem) and guess_grouping (safe).
+@c It (and strfmon_l) both call __vstrfmon_l_internal, which, besides
+@c accessing the locale object passed to it, accesses the active
+@c locale through isdigit (but to_digit assumes ASCII digits only).
+@c It may call __printf_fp (@mtslocale @ascuheap @acsmem) and
+@c guess_grouping (safe).
 The @code{strfmon} function is similar to the @code{strftime} function
 in that it takes a buffer, its size, a format string,
 and values to write into the buffer as text in a form specified
diff --git a/stdlib/strfmon.c b/stdlib/strfmon.c
index 01980d3e15..3058b3eed1 100644
--- a/stdlib/strfmon.c
+++ b/stdlib/strfmon.c
@@ -30,7 +30,8 @@ __strfmon (char *s, size_t maxsize, const char *format, ...)
 
   va_start (ap, format);
 
-  ssize_t res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
+  ssize_t res = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE,
+				       format, ap, 0);
 
   va_end (ap);
 
diff --git a/stdlib/strfmon_l.c b/stdlib/strfmon_l.c
index cd3796ced9..5293104400 100644
--- a/stdlib/strfmon_l.c
+++ b/stdlib/strfmon_l.c
@@ -76,8 +76,8 @@
    too.  Some of the information contradicts the information which can
    be specified in format string.  */
 ssize_t
-__vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
-	      va_list ap)
+__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
+		       const char *format, va_list ap, unsigned int flags)
 {
   struct __locale_data *current = loc->__locales[LC_MONETARY];
   _IO_strfile f;
@@ -268,7 +268,7 @@ __vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
       if (*fmt == 'L')
 	{
 	  ++fmt;
-	  if (!__ldbl_is_dbl)
+	  if (__glibc_likely ((flags & STRFMON_LDBL_IS_DBL) == 0))
 	    is_long_double = 1;
 	}
 
@@ -608,7 +608,7 @@ ___strfmon_l (char *s, size_t maxsize, locale_t loc, const char *format, ...)
 
   va_start (ap, format);
 
-  ssize_t res = __vstrfmon_l (s, maxsize, loc, format, ap);
+  ssize_t res = __vstrfmon_l_internal (s, maxsize, loc, format, ap, 0);
 
   va_end (ap);
 
diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
index ffb5fabebe..7a1e89c1a3 100644
--- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
+++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
@@ -51,8 +51,6 @@ libc_hidden_proto (__nldbl___vswprintf_chk)
 libc_hidden_proto (__nldbl___vasprintf_chk)
 libc_hidden_proto (__nldbl___vdprintf_chk)
 libc_hidden_proto (__nldbl___obstack_vprintf_chk)
-libc_hidden_proto (__nldbl___vstrfmon)
-libc_hidden_proto (__nldbl___vstrfmon_l)
 libc_hidden_proto (__nldbl___isoc99_vsscanf)
 libc_hidden_proto (__nldbl___isoc99_vfscanf)
 libc_hidden_proto (__nldbl___isoc99_vswscanf)
@@ -780,12 +778,13 @@ attribute_compat_text_section
 __nldbl_strfmon (char *s, size_t maxsize, const char *format, ...)
 {
   va_list ap;
-  ssize_t res;
+  ssize_t ret;
 
   va_start (ap, format);
-  res = __nldbl___vstrfmon (s, maxsize, format, ap);
+  ret = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
+			       STRFMON_LDBL_IS_DBL);
   va_end (ap);
-  return res;
+  return ret;
 }
 
 ssize_t
@@ -794,12 +793,13 @@ __nldbl___strfmon_l (char *s, size_t maxsize, locale_t loc,
 		     const char *format, ...)
 {
   va_list ap;
-  ssize_t res;
+  ssize_t ret;
 
   va_start (ap, format);
-  res = __nldbl___vstrfmon_l (s, maxsize, loc, format, ap);
+  ret = __vstrfmon_l_internal (s, maxsize, loc, format, ap,
+			       STRFMON_LDBL_IS_DBL);
   va_end (ap);
-  return res;
+  return ret;
 }
 weak_alias (__nldbl___strfmon_l, __nldbl_strfmon_l)
 
@@ -807,28 +807,18 @@ ssize_t
 attribute_compat_text_section
 __nldbl___vstrfmon (char *s, size_t maxsize, const char *format, va_list ap)
 {
-  ssize_t res;
-  __no_long_double = 1;
-  res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
-  __no_long_double = 0;
-  va_end (ap);
-  return res;
+  return __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
+				STRFMON_LDBL_IS_DBL);
 }
-libc_hidden_def (__nldbl___vstrfmon)
 
 ssize_t
 attribute_compat_text_section
 __nldbl___vstrfmon_l (char *s, size_t maxsize, locale_t loc,
 		      const char *format, va_list ap)
 {
-  ssize_t res;
-  __no_long_double = 1;
-  res = __vstrfmon_l (s, maxsize, loc, format, ap);
-  __no_long_double = 0;
-  va_end (ap);
-  return res;
+  return __vstrfmon_l_internal (s, maxsize, loc, format, ap,
+				STRFMON_LDBL_IS_DBL);
 }
-libc_hidden_def (__nldbl___vstrfmon_l)
 
 void
 attribute_compat_text_section
diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.h b/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
index b7e938f785..b7606c3c2d 100644
--- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
+++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
@@ -64,7 +64,6 @@ NLDBL_DECL (vsyslog);
 NLDBL_DECL (qecvt);
 NLDBL_DECL (qfcvt);
 NLDBL_DECL (qgcvt);
-NLDBL_DECL (__vstrfmon_l);
 NLDBL_DECL (__isoc99_scanf);
 NLDBL_DECL (__isoc99_fscanf);
 NLDBL_DECL (__isoc99_sscanf);
@@ -78,10 +77,13 @@ NLDBL_DECL (__isoc99_vwscanf);
 NLDBL_DECL (__isoc99_vfwscanf);
 NLDBL_DECL (__isoc99_vswscanf);
 
-/* This one does not exist in the normal interface, only
-   __nldbl___vstrfmon really exists.  */
+/* These do not exist in the normal interface, but must exist in the
+   __nldbl interface so that they can be called from libnldbl.  */
 extern ssize_t __nldbl___vstrfmon (char *, size_t, const char *, va_list)
   __THROW;
+extern ssize_t __nldbl___vstrfmon_l (char *, size_t, locale_t, const char *,
+				     va_list)
+  __THROW;
 
 /* These don't use __typeof because they were not declared by the headers,
    since we don't compile with _FORTIFY_SOURCE.  */
-- 
2.14.5