From patchwork Mon Oct 29 12:16:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Gabriel F. T. Gomes" X-Patchwork-Id: 29939 Received: (qmail 17215 invoked by alias); 29 Oct 2018 12:17:13 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 17125 invoked by uid 89); 29 Oct 2018 12:17:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=contradicts, eight, mflr, switched X-HELO: mo20.mail-out.ovh.net From: "Gabriel F. T. Gomes" To: Subject: [PATCH v2 1/8] Use STRFMON_LDBL_IS_DBL instead of __ldbl_is_dbl. Date: Mon, 29 Oct 2018 09:16:43 -0300 Message-ID: <20181029121650.24544-2-gabriel@inconstante.eti.br> In-Reply-To: <20181029121650.24544-1-gabriel@inconstante.eti.br> References: <20181029121650.24544-1-gabriel@inconstante.eti.br> MIME-Version: 1.0 X-Ovh-Tracer-Id: 18033257337046748867 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: 0 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedtkedrheekgdefjecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemucehtddtnecu From: Zack Weinberg 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 Gabriel F. T. Gomes * 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 Signed-off-by: Gabriel F. T. Gomes --- 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 -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. */